CreateOrUpdate ergonomics in presence of defaults

[dtantsur]

Hi folks.

I'm working on an operator that, absolutely unsurprisingly, creates deployments. Following what seems to be the good controller style, I tried using CreateOrUpdate in roughly this way (simplified):

	deploy := &appsv1.Deployment{
		ObjectMeta: metav1.ObjectMeta{Name: databaseDeploymentName(db), Namespace: db.Namespace},
	}
	result, err := controllerutil.CreateOrUpdate(cctx.Context, cctx.Client, deploy, func() error {
		deploy.Spec.Template = newDatabasePodTemplate(db)
		return controllerutil.SetControllerReference(db, deploy, cctx.Scheme)
	})

As you can guess from the title, this naive approach does not actually work. Because there are certain defaults in the schema (e.g. ImagePullPolicy), the deep comparison inside CreateOrUpdate always fails, and my controller gets stuck in an infinite reconcile loop trying to update the "defaulted" fields to their empty values.

What is a possible way out of this problem?

1. I've tried only changing fields that I want to set to non-default values. This quickly becomes really tedious.
2. Apply defaults before comparisons. So something like

	deploy := &appsv1.Deployment{
		ObjectMeta: metav1.ObjectMeta{Name: databaseDeploymentName(db), Namespace: db.Namespace},
	}
	result, err := controllerutil.CreateOrUpdate(cctx.Context, cctx.Client, deploy, func() error {
		deploy.Spec.Template = newDatabasePodTemplate(db)
                schema.Default(deploy)
		return controllerutil.SetControllerReference(db, deploy, cctx.Scheme)
	})

I've seen this solution mentioned on stackoverflow, but I see a large problem here. The local copy of Kubernetes code may not match the server side causing one of three issues:

1. If the server is older, my code may try sending fields that do not exist in it.
2. If the server is newer, it may have fields my code doesn't know defaults for, causing the infinite reconcile loop again.
3. If the server changes any defaults (I hope they don't but who knows), I'm now sending non-default values.

The only possible solution I can imaging is to run comparison on copies with defaults applied. So, change CreateOrUpdate to something like

	existing := obj.DeepCopyObject()
        schema.Default(existing)
	if err := mutate(f, key, obj); err != nil {
		return OperationResultNone, err
	}

        copyObj := obj.DeepCopyObject()
        schema.Default(copyObj)
	if equality.Semantic.DeepEqual(existing, copyObj) {
		return OperationResultNone, nil
	}

but obviously

1. Another deep copy may be expensive
2. The function does not have access to Schema now
3. Does not solve problem (2) with newer servers

No idea if this issue even has solutions but recording it here for any potential ideas.

[dtantsur]

If the server is newer, it may have fields my code doesn't know defaults for, causing the infinite reconcile loop again.

Hmm, this has to be false: if my client is older than the server, I will not see new fields at all. So at least this is not an issue. Point 3 (changing defaults) is probably a rare case. Now I'm just curious how much of a problem point 1 (older servers) is.

In any case, this stuff could use a lot of documentation.

[dtantsur]

schema.Default(deploy)

This does not work in my testing :( I guess I'm stuck with carefully updating only the fields I need set.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale