Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to reflect CR definition to its spec in the code #6346

Closed
cornBuddy opened this issue Mar 4, 2023 · 8 comments
Closed

How to reflect CR definition to its spec in the code #6346

cornBuddy opened this issue Mar 4, 2023 · 8 comments
Assignees
Labels
language/go Issue is related to a Go operator project lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/support Indicates an issue that is a support question.
Milestone

Comments

@cornBuddy
Copy link

Type of question

How to implement a specific feature

Question

What did you do?

I followed the go operator tutorial for my custom resource, and ended up with this API so far

type ExternalDNS struct {
	// +kubebuilder:default=true
	// +operator-sdk:csv:customresourcedefinitions:type=spec
	Enabled bool `json:"enabled,omitempty"`

	// +kubebuilder:default="docker.io/klutchell/unbound:v1.17.1"
	// +operator-sdk:csv:customresourcedefinitions:type=spec
	Image string `json:"image,omitempty"`
}

type WireguardSpec struct {
	// +kubebuilder:validation:Minimum=1
	// +kubebuilder:validation:Maximum=3
	// +kubebuilder:validation:ExclusiveMaximum=false
	// +kubebuilder:default=1
	// +operator-sdk:csv:customresourcedefinitions:type=spec
	Replicas int32 `json:"replicas,omitempty"`

	// +kubebuilder:default=51820
	// +operator-sdk:csv:customresourcedefinitions:type=spec
	ContainerPort int32 `json:"containerPort,omitempty"`

	// +kubebuilder:default="192.168.1.1/24"
	// +operator-sdk:csv:customresourcedefinitions:type=spec
	Network string `json:"network,omitempty"`

	// +operator-sdk:csv:customresourcedefinitions:type=spec
	ExternalDNS ExternalDNS `json:"externalDns,omitempty"`
}

I added some logic which relies on the spec values in my Reconcile function and it seems like the .Spec field of the Wireguard resource always has default values, eg

...
spec:
  externalDns:
    enabled: false

in the Reconcile function

if wireguard.Spec.ExternalDNS.Enabled { // this field is always true
	// code
}

What did you expect to see?

I expect that .Spec of the CR is up to date with resource definition

What did you see instead? Under which circumstances?

.Spec always has default values of the CR

Environment

Operator type:
/language go

Kubernetes cluster type:
minikube

$ operator-sdk version
operator-sdk version: "v1.27.0", commit: "5cbdad9209332043b7c730856b6302edc8996faf", kubernetes version: "v1.25.0", go version: "go1.19.5", GOOS: "darwin", GOARCH: "amd64"

$ go version (if language is Go)
go version go1.19.6 darwin/amd64

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.1", GitCommit:"8f94681cd294aa8cfd3407b8191f6c70214973a4", GitTreeState:"clean", BuildDate:"2023-01-18T15:51:24Z", GoVersion:"go1.19.5", Compiler:"gc", Platform:"darwin/amd64"} Kustomize Version: v4.5.7 Server Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.1", GitCommit:"8f94681cd294aa8cfd3407b8191f6c70214973a4", GitTreeState:"clean", BuildDate:"2023-01-18T15:51:25Z", GoVersion:"go1.19.5", Compiler:"gc", Platform:"linux/amd64"}

@openshift-ci openshift-ci bot added the language/go Issue is related to a Go operator project label Mar 4, 2023
@varshaprasad96
Copy link
Member

@cornBuddy I created a very quick test project with the API spec which you have provided and tested it locally: https://github.com/varshaprasad96/wireguard-operator.

It works as expected. When I change the enabled field of the CR spec, it reflects the expected value in the reconciler. You can see the logs below:

test -s /Users/varshaprasadnarsing/go/src/github.com/varshaprasad96/wideguard-operator/bin/controller-gen || GOBIN=/Users/varshaprasadnarsing/go/src/github.com/varshaprasad96/wideguard-operator/bin go install sigs.k8s.io/controller-tools/cmd/[email protected]
/Users/varshaprasadnarsing/go/src/github.com/varshaprasad96/wideguard-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
/Users/varshaprasadnarsing/go/src/github.com/varshaprasad96/wideguard-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
go fmt ./...
go vet ./...
go run ./main.go
1.678126532680626e+09	INFO	controller-runtime.metrics	Metrics server is starting to listen	{"addr": ":8080"}
1.678126532680875e+09	INFO	setup	starting manager
1.678126532681273e+09	INFO	Starting server	{"kind": "health probe", "addr": "[::]:8081"}
1.6781265326813e+09	INFO	Starting server	{"path": "/metrics", "kind": "metrics", "addr": "[::]:8080"}
1.678126532681396e+09	INFO	Starting EventSource	{"controller": "wideguard", "controllerGroup": "cache.example.com", "controllerKind": "Wideguard", "source": "kind source: *v1alpha1.Wideguard"}
1.678126532681484e+09	INFO	Starting Controller	{"controller": "wideguard", "controllerGroup": "cache.example.com", "controllerKind": "Wideguard"}
1.6781265327828412e+09	INFO	Starting workers	{"controller": "wideguard", "controllerGroup": "cache.example.com", "controllerKind": "Wideguard", "worker count": 1}
1.678126554203628e+09	INFO	spec values	{"controller": "wideguard", "controllerGroup": "cache.example.com", "controllerKind": "Wideguard", "Wideguard": {"name":"wideguard-sample","namespace":"default"}, "namespace": "default", "name": "wideguard-sample", "reconcileID": "5c3e2e75-e088-464b-b25f-251b1518149a", "wideguard.Spec.ExternalDNS.Enabled": false}
1.67812658070607e+09	INFO	spec values	{"controller": "wideguard", "controllerGroup": "cache.example.com", "controllerKind": "Wideguard", "Wideguard": {"name":"wideguard-sample","namespace":"default"}, "namespace": "default", "name": "wideguard-sample", "reconcileID": "6659d964-7d7d-4f64-afd9-be6f359c523d", "wideguard.Spec.ExternalDNS.Enabled": true}
^C1.678126587099207e+09	INFO	Stopping and waiting for non leader election runnables
1.678126587099252e+09	INFO	Stopping and waiting for leader election runnables
1.6781265870992792e+09	INFO	Shutdown signal received, waiting for all workers to finish	{"controller": "wideguard", "controllerGroup": "cache.example.com", "controllerKind": "Wideguard"}
1.678126587099381e+09	INFO	All workers finished	{"controller": "wideguard", "controllerGroup": "cache.example.com", "controllerKind": "Wideguard"}
1.678126587099394e+09	INFO	Stopping and waiting for caches
1.678126587099547e+09	INFO	Stopping and waiting for webhooks
1.67812658709957e+09	INFO	Wait completed, proceeding to shutdown the manager

Could you verify if your CR looks like this. You could also check the operator (and even api server) logs in your cluster to check if there is anything suspicious happening.

@varshaprasad96 varshaprasad96 added the triage/support Indicates an issue that is a support question. label Mar 6, 2023
@varshaprasad96 varshaprasad96 added this to the Backlog milestone Mar 6, 2023
@cornBuddy
Copy link
Author

Thank for your response @varshaprasad96!
My CRs look very similar to yours, except that I don't have some much labels there

---
apiVersion: vpn.ahova.com/v1alpha1
kind: Wireguard
metadata:
  name: wireguard-default
spec:
  replicas: 1

---
apiVersion: vpn.ahova.com/v1alpha1
kind: Wireguard
metadata:
  name: wireguard-internal-dns
spec:
  replicas: 1
  externalDns:
    enabled: false

It seems like I fugured things out by myself :) It looks very similar to the kubernetes-sigs/controller-tools#622 issue. I managed to workaround it by providing defaults to the nested structure at the top level

// WireguardSpec defines the desired state of Wireguard
type WireguardSpec struct {
         // ...

	// Provides configuration of the dns sidecar
        // +kubebuilder:default={enabled: true, image: "docker.io/klutchell/unbound:v1.17.1"}
	ExternalDNS ExternalDNS `json:"externalDns,omitempty"`
        // ...
}

type ExternalDNS struct {
	// no defaults here
	Enabled bool `json:"enabled,omitempty"`

	// no defaults here
	Image string `json:"image,omitempty"`
}

Yet it still feels like a bug because I expect defaults of nested structure to be populated in the spec after make manifests is run

@OchiengEd
Copy link
Contributor

Thank you for sharing your workaround.

Defaults in boolean types can be elusive since there are only two options (true and false) and the default option is similarly false.

However, if the issue recurs, I would consider changing the Enabled option to be a pointer of type bool which adds a third possible value; nil. Thereby allowing you to easily tell whether the option was unset or set to either true or false.

In your specific scenario, since you have a default value, at the end of the day you should still be working with either true or false.

@EraYaN
Copy link

EraYaN commented Mar 24, 2023

Seem like setting the default to an empty object manually (like default: {}) seems to work in getting the child default values set. But sadly using +kubebuilder:default={} does nothing.

As a workaround set it to the string "{empty}" and that use sed -i "s/'{empty}'/{}/g" config/crd/bases/*.yaml in the makefile

EDIT: This only works for all optional fields in the child objects. Otherwise you get .default.<field>: Required value errors

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 23, 2023
@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 23, 2023
@openshift-bot
Copy link

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci
Copy link

openshift-ci bot commented Aug 23, 2023

@openshift-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot closed this as completed Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language/go Issue is related to a Go operator project lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/support Indicates an issue that is a support question.
Projects
None yet
Development

No branches or pull requests

6 participants