Defaulting functions do not get called

[baichinger]

Bug Report

What did you do?
The generated defaulting functions for the CRD do not get called. Code generation was done via

./vendor/k8s.io/code-generator/generate-internal-groups.sh defaulter github.com/Dynatrace/dynatrace-oneagent-operator/pkg/generated github.com/Dynatrace/dynatrace-oneagent-operator/pkg/apis
 github.com/Dynatrace/dynatrace-oneagent-operator/pkg/apis dynatrace:v1alpha1

RegisterDefaults gets invoked during startup. The registered function SetObjectDefaults_OneAgent does not.

What did you expect to see?
Defaulting function SetObjectDefaults_OneAgent gets called.

What did you see instead? Under which circumstances?
Functions do not get called. This causes a panic later on during reconciliation as fields are not setup as expected.

Environment

• operator-sdk version:
  v0.4.0

• Kubernetes version information:
  v1.9.11, v1.13.1

• Kubernetes cluster kind:
  kubeadm

• Are you writing your operator in ansible, helm, or go?
  Golang

Additional context
Defaulters did work in previous version of operator-sdk (v0.0.5).

[theishshah]

This should fix the issue, please re-open if not.

[baichinger]

The actual problem is not the missing defaulter functions generated. Already did this manually. The problem is that those functions don't get called by the client. The current workaround is to call them explicitly: r.schema.Default(instance)

https://github.com/Dynatrace/dynatrace-oneagent-operator/blob/defaulters/pkg/controller/oneagent/oneagent_controller.go#L108

Is this the way it is supposed to be used?

[AlexNPavel]

@baichinger Hello. I've looked into this and I think I found what might be causing the issues. I found some examples of defaulters here: https://github.com/kubernetes/gengo/tree/master/examples/defaulter-gen/_output_tests

All of the projects that have defaulters include this function in their defaults.go file:

func addDefaultingFuncs(scheme *runtime.Scheme) error {
	return RegisterDefaults(scheme)
}

I imported your project, added that function to your v1alpha1 package's default.go file, and then ran operator-sdk generate k8s. That generated a new zz_generated.defaults.go file. I don't know how to test this, but try this out and see if that fixes the problem.

[baichinger]

It does not make any difference if the functions gets registered either by RegisterDefaults or by addDefaultingFuncs. In either way the functions get registered. The problem is that those functions don't get called. IMO the client github.com/kubernetes-sigs/controller-runtime/pkg/client is in charge of calling https://github.com/kubernetes/apimachinery/blob/master/pkg/runtime/scheme.go#L389

[hasbro17]

@baichinger Just weighing in on this, I think the correct behavior for the controller-runtime client is to return exactly what is there on the server, without adding defaults to the CR.
I don't know if that will be changing anytime soon so you might have to explicitly call the Defaulting function on the object yourself for now.

The recommended way to default CRDs is via mutating admission webhooks.

The controller-runtime has APIs to help setup admission webhooks so I would defer to that for now.
https://github.com/kubernetes-sigs/controller-runtime/blob/master/example/mutatingwebhook.go
https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg#hdr-Webhook
https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/webhook

We don't have a workflow for webhooks in the SDK right now but we'll most likely utilize the same APIs to help users setup webhooks for defaulting.

@theishshah @AlexNPavel With that being said I don't see a use case for running the defaulter-gen in the SDK so I think we might need to revert #1050

Related: #269

[openshift-bot]

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

[baichinger]

/close

[openshift-ci-robot]

@baichinger: Closing this issue.

In response to this:

/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.