request: provide a way to set labels on the machines created by the controlplane provider

[lukebond]

when creating a machine deployment in cluster-api one can set labels on the created machines by specifying them in .spec.template.metadata. it would be nice if we could do something similar for the controlplane machines created by the k3s controlplane provider.

it would be nice if controlplane machines could be created as MachineDeployments in order to benefit from MachineHealthChecks, however that's a separate discussion. given they are now created as Machines i propose we add a MachineLabels map[string]string optional field to the KThreesControlPlaneSpec and set them onto the created machine in generateMachine().

@zawachte if you prefer a different approach let me know, otherwise i'll get started on a PR to do this.

[zawachte]

high level looks go to me. I just want to double check how its being done for kubeadmcontrolplane.

On the control plane machine as machine deployment topic, I don't think we want to go that route as it diverges too much from how the kubeadmcontrolplane works. We probably need to just implement the proper logic in this controlplane provider to get MHCs working.

[zawachte]

Maybe we should follow their pattern of using a ObjectMeta as the struct to pass in the labels? https://github.com/kubernetes-sigs/cluster-api/blob/8ba3f47b053da8bbf63cf407c930a2ee10bfd754/controlplane/kubeadm/api/v1alpha4/kubeadm_control_plane_types.go#L93C2-L93C67

[richardcase]

Maybe we should follow their pattern of using a ObjectMeta as the struct to pass in the labels? https://github.com/kubernetes-sigs/cluster-api/blob/8ba3f47b053da8bbf63cf407c930a2ee10bfd754/controlplane/kubeadm/api/v1alpha4/kubeadm_control_plane_types.go#L93C2-L93C67

I agree, following the conventions setup out by KCP would be a good approach. However, this would require a new API version (i.e. v1beta2) as it would require changing KThreesControlPlaneSpec so that InfrastructureTemplate is moved into a new struct. With a bump in API version thsi would enable conversion webhooks so that the old version can be converted automatically.

[zawachte]

Maybe we should follow their pattern of using a ObjectMeta as the struct to pass in the labels? https://github.com/kubernetes-sigs/cluster-api/blob/8ba3f47b053da8bbf63cf407c930a2ee10bfd754/controlplane/kubeadm/api/v1alpha4/kubeadm_control_plane_types.go#L93C2-L93C67

I agree, following the conventions setup out by KCP would be a good approach. However, this would require a new API version (i.e. v1beta2) as it would require changing KThreesControlPlaneSpec so that InfrastructureTemplate is moved into a new struct. With a bump in API version thsi would enable conversion webhooks so that the old version can be converted automatically.

I agree @richardcase. This should be the north star. But can we figure out a way to split the difference until we are ready to take an api version bump? For now, can we stick ObjectMeta in an "undesirable" place while we maybe begin to think about what a new apiversion would look like?

[lukebond]

Cool I'll do this 👍 thanks

[lukebond]

On the control plane machine as machine deployment topic, I don't think we want to go that route as it diverges too much from how the kubeadmcontrolplane works. We probably need to just implement the proper logic in this controlplane provider to get MHCs working.

Ah okay. Sounds good to me. I was mistaken in thinking that MHCs required MDs 👍 happy to contribute to this work at some point

[lukebond]

i have taken this intermediate approach that avoids the version bump by mimicking what kubeadm does. i understand that this additional spec field should someday go somewhere else. hopefully this is the middle-ground you're looking for. happy to rework if needed!

#59

[richardcase]

This should be the north star. But can we figure out a way to split the difference until we are ready to take an api version bump? For now, can we stick ObjectMeta in an "undesirable" place while we maybe begin to think about what a new apiversion would look like?

Thats a good point @zawachte. Off the top of my head there are a couple of ways to do this:

• It could be added directly to KThreesControlPlaneSpec
• It could be added to a new KThreesControlPlaneMachineTemplate along with the InfrastructureRef. This would then introduce a duplicate InfrastructureRef but you could mark the existing one as deprecated (so users are encouraged to use the new struct) and also add validation to the webhooks to ensure that users don't supply values for both.

I didn't see an issue to track future API changes so i created #62