controller-gen feature request: support for generating DeepCopy of ApplyConfigrations

[zoetrope]

In Kubernetes v1.21, client-go supports typesafe server-side apply.
kubernetes/enhancements#2155

I believe this feature will be very useful in controller-runtime as well.
When our CRDs have a PodTemplate or ServiceTemplate, the difference calculation will be very easy.

I have written sample code for it.
https://github.com/zoetrope/reconcile-tips/blob/typed-ssa/controllers/myapp_controller.go

However, current controller-gen(v0.5.0) cannot generate deepCopy function for XxxxxApplyConfiguration.
Could you please support this feature in controller-gen?

[zoetrope]

kubernetes-sigs/controller-runtime#1365

[Jefftree]

Hi, can you elaborate on your use case for deepcopy? If it's to satisfy the client.Apply patch interface, there's a fix coming in kubernetes-sigs/controller-runtime#1365 to remove the need to convert to unstructured.

[zoetrope]

@Jefftree
To remove the need to convert to unstructured is really awesome.

In my use case, I want CRD to have PodTemplateApplyConfiguration as follows.
https://github.com/zoetrope/reconcile-tips/blob/typed-ssa/api/v1/myapp_types.go#L32

Then, the generated deepcopy will be compile error. (Instead, I used github.com/getlantern/deepcopy)
https://github.com/zoetrope/reconcile-tips/blob/typed-ssa/api/v1/zz_generated.deepcopy.go#L94

[Jefftree]

Hi @zoetrope. We will drop the requirement for applyconfig Patch requests to require DeepCopy to be implemented. The proposed interface looks something like this:

cl.Patch(context.TODO(), returnObj, client.ApplyFrom(applyConfig), owner);

I believe in your use case it's just an interface problem? The DeepCopy function doesn't seem to be used in any way.

[zoetrope]

@Jefftree
Understood. Thank you.

[zoetrope]

This is exactly what I want!
#536

[armsnyder]

I'm still curious how using ApplyConfiguration fields in a CRD can work, since it's something I need as well.

It looks like #536 removes the need to convert to unstructured, but we would still need the DeepCopy methods on the ApplyConfiguration types so that the code generation doesn't cause type has no field or method DeepCopyInto compiler errors, right?

Looking at @zoetrope 's example, they had to make a manual edit to the generated code:

https://github.com/zoetrope/reconcile-tips/blob/64114db1882d1eb76b496771cb51a931ce162470/api/v1/zz_generated.deepcopy.go#L94-L95

 //(*in).DeepCopyInto(*out) 
 _ = deepcopy.Copy(out, in) // Instead of generated deepcopy 

Or is was a programmatic way of skipping deepcopy configuration here?

[Jefftree]

I haven't had the time to polish up the PR but we modify the client Patch method to be able to take in an object without the DeepCopy methods. kubernetes-sigs/controller-runtime#1365

That way everything should be independent of the kubernetes object interface and we can use applyconfigurations in the Patch directly.

[armsnyder]

Thanks but to clarify, I mean that when you use a *ApplyConfiguraion type in your CRD, to represent some sort of template, then controller-gen object (or make generate in a kubebuilder project) generates non compiling code because of these missing methods.

[Jefftree]

The goal is to turn the CRD into an ApplyConfiguration object itself,, and have controller-runtime operate on that ApplyConfiguration instead of the kubernetes CRD type. The ApplyConfiguration will have all fields present in the CRD, and have the option to embed additional ApplyConfigurations (eg for built in types) as fields if needed.

[armsnyder]

Got it. So that would require changing the controller-runtime client.Get as well then, to accept the *ApplyConfiguraion type, for the start of your Reconcile function.

[armsnyder]

I still believe this would be useful, at least in the medium term, since there are probably implications like the client.Get case I mentioned that will make breaking the runtime.Object dependency take a while. Do you have a better sense of this @Jefftree?

I am currently working on a CRD that embeds a deployment spec, and using runtime.RawExtension in order to have access to the raw JSON payload, which my controller can unmarshal into a DeploymentSpecApplyConfiguration. The experience has been difficult since controller-gen will not generate an OpenAPI schema for that field.

For example most recently I have found I cannot target my CRD from an HPA because the .spec.deploymentTemplate.spec.replicas field does not exist on the OpenAPI schema. 😢

EDIT: The HPA issue turned out not to be related. However needing to manually marshal/unmarshal JSON in the controller and especially in tests, as well as the need to use the unsafe x-kubernetes-preserve-unknown-fields: true extension are both concerns which could be resolved with DeepCopy methods on ApplyConfigurations.

[zoetrope]

I have found a way to solve this problem.
First, prepare a type that wraps the ApplyConfiguration type and implement its deepcopy.

package types

import (
	corev1apply "k8s.io/client-go/applyconfigurations/core/v1"
)

type PodTemplateApplyConfiguration struct {
	corev1apply.PodTemplateApplyConfiguration `json:",inline"`
}

func (ac *PodTemplateApplyConfiguration) DeepCopy() *PodTemplateApplyConfiguration {
	out := new(PodTemplateApplyConfiguration)
	// implement deepcoy
	return out
}

I can use that wrapped type in my own custom resource type.

// MyAppSpec defines the desired state of MyApp
type MyAppSpec struct {
	PodTemplate *types.PodTemplateApplyConfiguration `json:"podTemplate"`
}

// MyAppStatus defines the observed state of MyApp
type MyAppStatus struct {
}

//+kubebuilder:object:root=true
//+kubebuilder:subresource:status

// MyApp is the Schema for the myapps API
type MyApp struct {
	metav1.TypeMeta   `json:",inline"`
	metav1.ObjectMeta `json:"metadata,omitempty"`

	Spec   MyAppSpec   `json:"spec,omitempty"`
	Status MyAppStatus `json:"status,omitempty"`
}

No need to modify controller-tools, this will work fine.

[zoetrope]

type PodTemplateApplyConfiguration corev1apply.PodTemplateApplyConfiguration is sufficient.

I updated my sample code.
https://github.com/zoetrope/reconcile-tips/blob/typed-ssa/api/v1/myapp_types.go

[fbozic]

I've run into the same issue. Are there any updates regarding this?
It feels hackish to implement DeepCopy for ApplyConfigurations.

@zoetrope 2 years have passed since you've opened this issue. Did you manage to get some experience running this code in production?