feat: making default datastore optional (#597)

* feat: making default datastore optional

Signed-off-by: Dario Tranchitella <dario@tranchitella.eu>

* feat(helm): making default datastore optional

Signed-off-by: Dario Tranchitella <dario@tranchitella.eu>

* docs: making default datastore optional

Signed-off-by: Dario Tranchitella <dario@tranchitella.eu>

---------

Signed-off-by: Dario Tranchitella <dario@tranchitella.eu>

api/v1alpha1/tenantcontrolplane_types.go

@@ -277,8 +277,10 @@ type AddonsSpec struct {
 // +kubebuilder:validation:XValidation:rule="oldSelf.controlPlane.service.serviceType != self.controlPlane.service.serviceType || (!has(oldSelf.networkProfile.loadBalancerClass) && has(self.networkProfile.loadBalancerClass))",message="LoadBalancerClass can not be unset"
 
 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"`

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") |

charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml

@@ -6412,8 +6412,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

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 }}

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:

cmd/manager/cmd.go

@@ -73,7 +73,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
 			}
 
@@ -304,7 +304,7 @@ func NewCmd(scheme *runtime.Scheme) *cobra.Command {
 	cmd.Flags().BoolVar(&leaderElect, "leader-elect", true, "Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.")
 	cmd.Flags().StringVar(&tmpDirectory, "tmp-directory", "/tmp/kamaji", "Directory which will be used to work with temporary files.")
 	cmd.Flags().StringVar(&kineImage, "kine-image", "rancher/kine:v0.11.10-amd64", "Container image along with tag to use for the Kine sidecar container (used only if etcd-storage-type is set to one of kine strategies).")
-	cmd.Flags().StringVar(&datastore, "datastore", "etcd", "The default DataStore that should be used by Kamaji to setup the required storage.")
+	cmd.Flags().StringVar(&datastore, "datastore", "", "Optional, the default DataStore that should be used by Kamaji to setup the required storage of Tenant Control Planes with undeclared DataStore.")
 	cmd.Flags().StringVar(&migrateJobImage, "migrate-image", fmt.Sprintf("clastix/kamaji:%s", internal.GitTag), "Specify the container image to launch when a TenantControlPlane is migrated to a new datastore.")
 	cmd.Flags().IntVar(&maxConcurrentReconciles, "max-concurrent-tcp-reconciles", 1, "Specify the number of workers for the Tenant Control Plane controller (beware of CPU consumption)")
 	cmd.Flags().StringVar(&managerNamespace, "pod-namespace", os.Getenv("POD_NAMESPACE"), "The Kubernetes Namespace on which the Operator is running in, required for the TenantControlPlane migration jobs.")

controllers/tenantcontrolplane_controller.go

@@ -122,6 +122,12 @@ func (r *TenantControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
 	// Retrieving the DataStore to use for the current reconciliation
 	ds, err := r.dataStore(ctx, tenantControlPlane)
 	if err != nil {
+		if errors.Is(err, ErrMissingDataStore) {
+			log.Info(err.Error())
+
+			return ctrl.Result{Requeue: true}, nil
+		}
+
 		log.Error(err, "cannot retrieve the DataStore for the given instance")
 
 		return ctrl.Result{}, err
@@ -300,18 +306,23 @@ func (r *TenantControlPlaneReconciler) RemoveFinalizer(ctx context.Context, tena
 	return r.Client.Update(ctx, tenantControlPlane)
 }
 
+var ErrMissingDataStore = errors.New("the Tenant Control Plane doesn't have a DataStore assigned, and Kamaji is running with no default DataStore fallback")
+
 // 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, ErrMissingDataStore
 	}
 
-	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
 }

docs/content/reference/api.md

@@ -837,8 +837,10 @@ such as the number of Pod replicas, the Service resource, or the Ingress.<br/>
         <td><b>dataStore</b></td>
         <td>string</td>
         <td>
-          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.<br/>
         </td>

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

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)

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
 	}
 }
 

internal/webhook/handlers/tcp_defaults.go

@@ -64,7 +64,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
 	}