From 1d71e17d5b9ba7952936a730346c861686b1ce84 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Mon, 7 Oct 2024 19:08:37 +0200 Subject: [PATCH 1/3] feat: making default datastore optional Signed-off-by: Dario Tranchitella --- api/v1alpha1/tenantcontrolplane_types.go | 6 ++++-- cmd/manager/cmd.go | 2 +- controllers/tenantcontrolplane_controller.go | 15 +++++++++------ internal/builders/controlplane/deployment.go | 2 +- internal/datastore/utils/check.go | 4 ++++ internal/webhook/handlers/tcp_datastore.go | 12 ++++++++++-- internal/webhook/handlers/tcp_defaults.go | 2 +- 7 files changed, 30 insertions(+), 13 deletions(-) diff --git a/api/v1alpha1/tenantcontrolplane_types.go b/api/v1alpha1/tenantcontrolplane_types.go index 8dc23f40..fce654b3 100644 --- a/api/v1alpha1/tenantcontrolplane_types.go +++ b/api/v1alpha1/tenantcontrolplane_types.go @@ -257,8 +257,10 @@ type AddonsSpec struct { // +kubebuilder:validation:XValidation:rule="!has(oldSelf.dataStore) || has(self.dataStore)", message="unsetting the dataStore is not supported" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.dataStoreSchema) || has(self.dataStoreSchema)", message="unsetting the dataStoreSchema is not supported" type TenantControlPlaneSpec struct { - // DataStore allows to specify a DataStore that should be used to store the Kubernetes data for the given Tenant Control Plane. - // This parameter is optional and acts as an override over the default one which is used by the Kamaji Operator. + // DataStore specifies the DataStore that should be used to store the Kubernetes data for the given Tenant Control Plane. + // When Kamaji runs with the default DataStore flag, all empty values will inherit the default value. + // By leaving it empty and running Kamaji with no default DataStore flag, it is possible to achieve automatic assignment to a specific DataStore object. + // // Migration from one DataStore to another backed by the same Driver is possible. See: https://kamaji.clastix.io/guides/datastore-migration/ // Migration from one DataStore to another backed by a different Driver is not supported. DataStore string `json:"dataStore,omitempty"` diff --git a/cmd/manager/cmd.go b/cmd/manager/cmd.go index 9a24d4e5..4b234e83 100644 --- a/cmd/manager/cmd.go +++ b/cmd/manager/cmd.go @@ -72,7 +72,7 @@ func NewCmd(scheme *runtime.Scheme) *cobra.Command { klog.SetOutput(io.Discard) klog.LogToStderr(false) - if err = cmdutils.CheckFlags(cmd.Flags(), []string{"kine-image", "datastore", "migrate-image", "tmp-directory", "pod-namespace", "webhook-service-name", "serviceaccount-name", "webhook-ca-path"}...); err != nil { + if err = cmdutils.CheckFlags(cmd.Flags(), []string{"kine-image", "migrate-image", "tmp-directory", "pod-namespace", "webhook-service-name", "serviceaccount-name", "webhook-ca-path"}...); err != nil { return err } diff --git a/controllers/tenantcontrolplane_controller.go b/controllers/tenantcontrolplane_controller.go index 72dd1411..16537596 100644 --- a/controllers/tenantcontrolplane_controller.go +++ b/controllers/tenantcontrolplane_controller.go @@ -303,15 +303,18 @@ func (r *TenantControlPlaneReconciler) RemoveFinalizer(ctx context.Context, tena // dataStore retrieves the override DataStore for the given Tenant Control Plane if specified, // otherwise fallback to the default one specified in the Kamaji setup. func (r *TenantControlPlaneReconciler) dataStore(ctx context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) (*kamajiv1alpha1.DataStore, error) { - dataStoreName := tenantControlPlane.Spec.DataStore - if len(dataStoreName) == 0 { - dataStoreName = r.Config.DefaultDataStoreName + if tenantControlPlane.Spec.DataStore == "" && r.Config.DefaultDataStoreName == "" { + return nil, fmt.Errorf("the Tenant Control Plane doesn't have a DataStore assigned, and Kamaji is running with no default DataStore fallback") } - ds := &kamajiv1alpha1.DataStore{} - if err := r.Client.Get(ctx, k8stypes.NamespacedName{Name: dataStoreName}, ds); err != nil { + if tenantControlPlane.Spec.DataStore == "" { + tenantControlPlane.Spec.DataStore = r.Config.DefaultDataStoreName + } + + var ds kamajiv1alpha1.DataStore + if err := r.Client.Get(ctx, k8stypes.NamespacedName{Name: tenantControlPlane.Spec.DataStore}, &ds); err != nil { return nil, errors.Wrap(err, "cannot retrieve *kamajiv1alpha.DataStore object") } - return ds, nil + return &ds, nil } diff --git a/internal/builders/controlplane/deployment.go b/internal/builders/controlplane/deployment.go index 00e5eb33..e71fafcb 100644 --- a/internal/builders/controlplane/deployment.go +++ b/internal/builders/controlplane/deployment.go @@ -1017,7 +1017,7 @@ func (d Deployment) templateLabels(ctx context.Context, tenantControlPlane *kama "component.kamaji.clastix.io/front-proxy-client-certificate": hash(ctx, tenantControlPlane.GetNamespace(), tenantControlPlane.Status.Certificates.FrontProxyClient.SecretName), "component.kamaji.clastix.io/service-account": hash(ctx, tenantControlPlane.GetNamespace(), tenantControlPlane.Status.Certificates.SA.SecretName), "component.kamaji.clastix.io/scheduler-kubeconfig": hash(ctx, tenantControlPlane.GetNamespace(), tenantControlPlane.Status.KubeConfig.Scheduler.SecretName), - "component.kamaji.clastix.io/datastore": tenantControlPlane.Spec.DataStore, + "component.kamaji.clastix.io/datastore": tenantControlPlane.Status.Storage.DataStoreName, } return labels diff --git a/internal/datastore/utils/check.go b/internal/datastore/utils/check.go index a587ddd3..9f11bd63 100644 --- a/internal/datastore/utils/check.go +++ b/internal/datastore/utils/check.go @@ -18,6 +18,10 @@ import ( // CheckExists ensures that the default Datastore exists before starting the manager. func CheckExists(ctx context.Context, scheme *runtime.Scheme, datastoreName string) error { + if datastoreName == "" { + return nil + } + ctrlClient, err := client.New(ctrl.GetConfigOrDie(), client.Options{Scheme: scheme}) if err != nil { return fmt.Errorf("unable to create controlerruntime.Client: %w", err) diff --git a/internal/webhook/handlers/tcp_datastore.go b/internal/webhook/handlers/tcp_datastore.go index d5cce7ab..8708326c 100644 --- a/internal/webhook/handlers/tcp_datastore.go +++ b/internal/webhook/handlers/tcp_datastore.go @@ -26,7 +26,11 @@ func (t TenantControlPlaneDataStore) OnCreate(object runtime.Object) AdmissionRe return func(ctx context.Context, req admission.Request) ([]jsonpatch.JsonPatchOperation, error) { tcp := object.(*kamajiv1alpha1.TenantControlPlane) //nolint:forcetypeassert - return nil, t.check(ctx, tcp.Spec.DataStore) + if tcp.Spec.DataStore != "" { + return nil, t.check(ctx, tcp.Spec.DataStore) + } + + return nil, nil } } @@ -38,7 +42,11 @@ func (t TenantControlPlaneDataStore) OnUpdate(object runtime.Object, _ runtime.O return func(ctx context.Context, req admission.Request) ([]jsonpatch.JsonPatchOperation, error) { tcp := object.(*kamajiv1alpha1.TenantControlPlane) //nolint:forcetypeassert - return nil, t.check(ctx, tcp.Spec.DataStore) + if tcp.Spec.DataStore != "" { + return nil, t.check(ctx, tcp.Spec.DataStore) + } + + return nil, nil } } diff --git a/internal/webhook/handlers/tcp_defaults.go b/internal/webhook/handlers/tcp_defaults.go index 6274d13c..d37d584e 100644 --- a/internal/webhook/handlers/tcp_defaults.go +++ b/internal/webhook/handlers/tcp_defaults.go @@ -48,7 +48,7 @@ func (t TenantControlPlaneDefaults) OnUpdate(runtime.Object, runtime.Object) Adm } func (t TenantControlPlaneDefaults) defaultUnsetFields(tcp *kamajiv1alpha1.TenantControlPlane) { - if len(tcp.Spec.DataStore) == 0 { + if len(tcp.Spec.DataStore) == 0 && t.DefaultDatastore != "" { tcp.Spec.DataStore = t.DefaultDatastore } From c77a2002fae0bdcf42b3456cf122e25cc1d84c07 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Mon, 7 Oct 2024 19:08:59 +0200 Subject: [PATCH 2/3] feat(helm): making default datastore optional Signed-off-by: Dario Tranchitella --- charts/kamaji/README.md | 2 +- .../kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml | 6 ++++-- charts/kamaji/templates/controller.yaml | 5 +++-- charts/kamaji/values.yaml | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/charts/kamaji/README.md b/charts/kamaji/README.md index a98bb0e3..3b7fa260 100644 --- a/charts/kamaji/README.md +++ b/charts/kamaji/README.md @@ -70,7 +70,7 @@ Here the values you can override: | Key | Type | Default | Description | |-----|------|---------|-------------| | affinity | object | `{}` | Kubernetes affinity rules to apply to Kamaji controller pods | -| defaultDatastoreName | string | `"default"` | Specify the default DataStore name for the Kamaji instance. | +| defaultDatastoreName | string | `"default"` | If specified, all the Kamaji instances with an unassigned DataStore will inherit this default value. | | extraArgs | list | `[]` | A list of extra arguments to add to the kamaji controller default ones | | fullnameOverride | string | `""` | | | healthProbeBindAddress | string | `":8081"` | The address the probe endpoint binds to. (default ":8081") | diff --git a/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml b/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml index 60578b77..b72f2ae2 100644 --- a/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml +++ b/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml @@ -6413,8 +6413,10 @@ spec: type: object dataStore: description: |- - DataStore allows to specify a DataStore that should be used to store the Kubernetes data for the given Tenant Control Plane. - This parameter is optional and acts as an override over the default one which is used by the Kamaji Operator. + DataStore specifies the DataStore that should be used to store the Kubernetes data for the given Tenant Control Plane. + When Kamaji runs with the default DataStore flag, all empty values will inherit the default value. + By leaving it empty and running Kamaji with no default DataStore flag, it is possible to achieve automatic assignment to a specific DataStore object. + Migration from one DataStore to another backed by the same Driver is possible. See: https://kamaji.clastix.io/guides/datastore-migration/ Migration from one DataStore to another backed by a different Driver is not supported. type: string diff --git a/charts/kamaji/templates/controller.yaml b/charts/kamaji/templates/controller.yaml index d4ca6dc5..38d635dd 100644 --- a/charts/kamaji/templates/controller.yaml +++ b/charts/kamaji/templates/controller.yaml @@ -33,8 +33,9 @@ spec: - --leader-elect - --metrics-bind-address={{ .Values.metricsBindAddress }} - --tmp-directory={{ .Values.temporaryDirectoryPath }} - {{- $datastoreName := .Values.defaultDatastoreName | required ".Values.defaultDatastoreName is required!" }} - - --datastore={{ $datastoreName }} + {{- if not (eq .Values.defaultDatastoreName "") }} + - --datastore={{ .Values.defaultDatastoreName }} + {{- end }} {{- if .Values.telemetry.disabled }} - --disable-telemetry {{- end }} diff --git a/charts/kamaji/values.yaml b/charts/kamaji/values.yaml index 21b529e0..79dd254a 100644 --- a/charts/kamaji/values.yaml +++ b/charts/kamaji/values.yaml @@ -95,7 +95,7 @@ loggingDevel: # -- Development Mode defaults(encoder=consoleEncoder,logLevel=Debug,stackTraceLevel=Warn). Production Mode defaults(encoder=jsonEncoder,logLevel=Info,stackTraceLevel=Error) (default false) enable: false -# -- Specify the default DataStore name for the Kamaji instance. +# -- If specified, all the Kamaji instances with an unassigned DataStore will inherit this default value. defaultDatastoreName: default kamaji-etcd: From 079f6040b5a048bdd2e9ec4149df18da6efbd572 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Mon, 7 Oct 2024 19:09:04 +0200 Subject: [PATCH 3/3] docs: making default datastore optional Signed-off-by: Dario Tranchitella --- docs/content/reference/api.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md index d9c43a42..5c81f3a1 100644 --- a/docs/content/reference/api.md +++ b/docs/content/reference/api.md @@ -837,8 +837,10 @@ such as the number of Pod replicas, the Service resource, or the Ingress.
dataStore string - DataStore allows to specify a DataStore that should be used to store the Kubernetes data for the given Tenant Control Plane. -This parameter is optional and acts as an override over the default one which is used by the Kamaji Operator. + DataStore specifies the DataStore that should be used to store the Kubernetes data for the given Tenant Control Plane. +When Kamaji runs with the default DataStore flag, all empty values will inherit the default value. +By leaving it empty and running Kamaji with no default DataStore flag, it is possible to achieve automatic assignment to a specific DataStore object. + Migration from one DataStore to another backed by the same Driver is possible. See: https://kamaji.clastix.io/guides/datastore-migration/ Migration from one DataStore to another backed by a different Driver is not supported.