Add KamajiControlPlaneTemplate for ClusterClass support #107
Conversation
There was a problem hiding this comment.
Hey @chess-knight, this is a very nice and helpful implementation, thanks for it!
Let me know if you're up to the minor changes in your code, otherwise, happy to jump on it and co-author.
| // KamajiControlPlaneTemplateResourceSpec defines the desired state of KamajiControlPlane. | ||
| type KamajiControlPlaneTemplateResourceSpec struct { | ||
| // The Kamaji DataStore to use for the given TenantControlPlane. | ||
| // Retrieve the list of the allowed ones by issuing "kubectl get datastores.kamaji.clastix.io". | ||
| DataStoreName string `json:"dataStoreName"` | ||
| // The addons that must be managed by Kamaji, such as CoreDNS, kube-proxy, and konnectivity. | ||
| Addons AddonsSpec `json:"addons,omitempty"` | ||
| // List of the admission controllers to configure for the TenantControlPlane kube-apiserver. | ||
| // By default, no admission controllers are enabled, refer to the desired Kubernetes version. | ||
| // | ||
| // More info: https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/ | ||
| AdmissionControllers kamajiv1alpha1.AdmissionControllers `json:"admissionControllers,omitempty"` | ||
| // Override the container registry used to pull the components image. | ||
| // Helpful if running in an air-gapped environment. | ||
| // +kubebuilder:default="registry.k8s.io" | ||
| ContainerRegistry string `json:"registry,omitempty"` | ||
|
|
||
| ControllerManager ControlPlaneComponent `json:"controllerManager,omitempty"` | ||
| ApiServer ControlPlaneComponent `json:"apiServer,omitempty"` //nolint:revive,stylecheck | ||
| Scheduler ControlPlaneComponent `json:"scheduler,omitempty"` | ||
| Kine KineComponent `json:"kine,omitempty"` | ||
| // Configure the Kubelet options, such as the preferred address types, or the expected cgroupfs. | ||
| // +kubebuilder:default={preferredAddressTypes:{"Hostname","InternalIP","ExternalIP"},cgroupfs:"systemd"} | ||
| Kubelet kamajiv1alpha1.KubeletSpec `json:"kubelet,omitempty"` | ||
| // Configure how the TenantControlPlane should be exposed. | ||
| // +kubebuilder:default={serviceType:"LoadBalancer"} | ||
| Network NetworkComponent `json:"network,omitempty"` | ||
| // Configure how the TenantControlPlane Deployment object should be configured. | ||
| Deployment DeploymentComponent `json:"deployment,omitempty"` | ||
| } |
There was a problem hiding this comment.
I had a discussion here with some CAPI maintainers and essentially this is the same scheme we're having for the KamajiControlPlane resource.
Having two separated schemas could be challenging, such as forgetting changes between them: I'm wondering if we could take advantage of the Go's struct embedding to simplify the maintainability in the long run.
May I ask you to give it a try by removing the non required fields for the template-based struct?
There was a problem hiding this comment.
Hi, thank you for the review.
As I wrote in the PR description, KamajiControlPlaneTemplateResourceSpec is copied and pasted from KamajiControlPlaneSpec with removed Replicas and Version fields. I followed https://github.com/kubernetes-sigs/cluster-api/blob/main/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go#L72 and also found it e.g. in https://github.com/k3s-io/cluster-api-k3s/blob/main/controlplane/api/v1beta2/kthreescontrolplanetemplate_types.go#L38.
I agree, we can use KamajiControlPlaneSpec directly(e.g. Spec KamajiControlPlaneSpec `json:"spec"`) instead of KamajiControlPlaneTemplateResourceSpec or use something more advanced.
AFAIK, the only required fields are DataStoreName and Version.
Please feel free to jump in for the minor changes, or we can talk about it right here and I will make changes - it's up to you.
There was a problem hiding this comment.
Thanks, @chess-knight!
I just force-pushed with the struct embedding, linter and manifests seem unchanged.
May I ask you for a final review so we can get this merged?
There was a problem hiding this comment.
It looks good(and better) to me now, thanks!
I found only one typo in the commit message introucing.
We can merge it then.
prometherion
force-pushed
the
feat/KamajiControlPlaneTemplate
branch
2 times, most recently
from
757eac5 to
678132b
Compare
…pport Co-authored-by: Roman Hros <[email protected]> Signed-off-by: Dario Tranchitella <[email protected]>
prometherion
force-pushed
the
feat/KamajiControlPlaneTemplate
branch
from
678132b to
f1e6741
Compare
Files were generated by
kubebuilder create api --group controlplane --version v1alpha1 --kind KamajiControlPlaneTemplateand modified according to other providers and CAPI docs.It means that
KamajiControlPlaneTemplate.spec.template.spec=KamajiControlPlane.spec(w/o replicas & version)