Let Glance statefulset create PVCs via template

This patch improves the way the glance statefulset uses PVCs: - ensurePVC is not required anymore and the logic has been moved away from controller to the statefulset creation function - storage{class, Request} are now fields of the GlanceAPITemplate - getVolumes don't require the pvc anymore - DBSync only mounts what it needs Signed-off-by: Francesco Pantano <[email protected]>
openstack-k8s-operators · Oct 26, 2023 · 98c1a9c · 98c1a9c
1 parent 27e1933
commit 98c1a9c
Show file tree

Hide file tree

Showing 13 changed files with 95 additions and 105 deletions.
diff --git a/api/bases/glance.openstack.org_glanceapis.yaml b/api/bases/glance.openstack.org_glanceapis.yaml
@@ -946,12 +946,17 @@ spec:
               serviceUser:
                 default: glance
                 type: string
+              storageClass:
+                type: string
+              storageRequest:
+                type: string
             required:
             - containerImage
             - databaseHostname
             - imageCacheSize
             - secret
             - serviceAccount
+            - storageRequest
             type: object
           status:
             properties:

diff --git a/api/bases/glance.openstack.org_glances.yaml b/api/bases/glance.openstack.org_glances.yaml
@@ -934,8 +934,13 @@ spec:
                           x-kubernetes-int-or-string: true
                         type: object
                     type: object
+                  storageClass:
+                    type: string
+                  storageRequest:
+                    type: string
                 required:
                 - containerImage
+                - storageRequest
                 type: object
               glanceAPIInternal:
                 properties:
@@ -1053,8 +1058,13 @@ spec:
                           x-kubernetes-int-or-string: true
                         type: object
                     type: object
+                  storageClass:
+                    type: string
+                  storageRequest:
+                    type: string
                 required:
                 - containerImage
+                - storageRequest
                 type: object
               imageCacheSize:
                 default: ""

diff --git a/api/v1beta1/common.go b/api/v1beta1/common.go
@@ -95,6 +95,13 @@ type GlanceAPITemplate struct {
 
 	// Override, provides the ability to override the generated manifest of several child resources.
 	Override APIOverrideSpec `json:"override,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	// StorageClass
+	StorageClass string `json:"storageClass,omitempty"`
+
+	// StorageRequest
+	StorageRequest string `json:"storageRequest"`
 }
 
 // APIOverrideSpec to override the generated manifest of several child resources.

diff --git a/config/crd/bases/glance.openstack.org_glanceapis.yaml b/config/crd/bases/glance.openstack.org_glanceapis.yaml
@@ -946,12 +946,17 @@ spec:
               serviceUser:
                 default: glance
                 type: string
+              storageClass:
+                type: string
+              storageRequest:
+                type: string
             required:
             - containerImage
             - databaseHostname
             - imageCacheSize
             - secret
             - serviceAccount
+            - storageRequest
             type: object
           status:
             properties:

diff --git a/config/crd/bases/glance.openstack.org_glances.yaml b/config/crd/bases/glance.openstack.org_glances.yaml
@@ -934,8 +934,13 @@ spec:
                           x-kubernetes-int-or-string: true
                         type: object
                     type: object
+                  storageClass:
+                    type: string
+                  storageRequest:
+                    type: string
                 required:
                 - containerImage
+                - storageRequest
                 type: object
               glanceAPIInternal:
                 properties:
@@ -1053,8 +1058,13 @@ spec:
                           x-kubernetes-int-or-string: true
                         type: object
                     type: object
+                  storageClass:
+                    type: string
+                  storageRequest:
+                    type: string
                 required:
                 - containerImage
+                - storageRequest
                 type: object
               imageCacheSize:
                 default: ""

diff --git a/controllers/glance_controller.go b/controllers/glance_controller.go
@@ -45,7 +45,6 @@ import (
 	"github.com/openstack-k8s-operators/lib-common/modules/common/job"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/labels"
 	nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/pvc"
 	common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac"
 	oko_secret "github.com/openstack-k8s-operators/lib-common/modules/common/secret"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
@@ -284,15 +283,6 @@ func (r *GlanceReconciler) reconcileInit(
 ) (ctrl.Result, error) {
 	r.Log.Info(fmt.Sprintf("Reconciling Service '%s' init", instance.Name))
 
-	// Define the PVCs objects required by Glance
-	ctrlResult, err := r.ensurePVC(ctx, helper, instance, serviceLabels)
-	if err != nil {
-		return ctrlResult, err
-	} else if (ctrlResult != ctrl.Result{}) {
-		return ctrlResult, nil
-	}
-	// End PVC creation/patch
-
 	//
 	// create service DB instance
 	//
@@ -305,7 +295,7 @@ func (r *GlanceReconciler) reconcileInit(
 		},
 	)
 	// create or patch the DB
-	ctrlResult, err = db.CreateOrPatchDB(
+	ctrlResult, err := db.CreateOrPatchDB(
 		ctx,
 		helper,
 	)
@@ -699,6 +689,10 @@ func (r *GlanceReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst
 		apiSpec.GlanceAPITemplate.NodeSelector = instance.Spec.NodeSelector
 	}
 
+	// Inherit the values required for PVC creation from the top-level CR
+	apiSpec.GlanceAPITemplate.StorageRequest = instance.Spec.StorageRequest
+	apiSpec.GlanceAPITemplate.StorageClass = instance.Spec.StorageClass
+
 	deployment := &glancev1.GlanceAPI{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      fmt.Sprintf("%s-%s", instance.Name, apiType),
@@ -834,39 +828,6 @@ func (r *GlanceReconciler) ensureRegisteredLimits(
 	return nil
 }
 
-// ensurePVC - Creates the PVCs required by Glance to start the GlanceAPI Pods
-func (r *GlanceReconciler) ensurePVC(
-	ctx context.Context,
-	h *helper.Helper,
-	instance *glancev1.Glance,
-	serviceLabels map[string]string,
-) (ctrl.Result, error) {
-	// Define a new PVC object
-	localPvc := pvc.NewPvc(
-		glance.Pvc(instance, serviceLabels, glance.PvcLocal),
-		time.Duration(5)*time.Second,
-	)
-
-	ctrlResult, err := localPvc.CreateOrPatch(ctx, h)
-
-	if err != nil {
-		return ctrlResult, err
-	} else if (ctrlResult != ctrl.Result{}) {
-		return ctrlResult, nil
-	}
-
-	// Handle an additional PVC creation an ImageCacheSize is provided
-	if len(instance.Spec.ImageCacheSize) > 0 {
-		cachePvc := pvc.NewPvc(
-			glance.Pvc(instance, serviceLabels, glance.PvcCache),
-			time.Duration(5)*time.Second,
-		)
-		ctrlResult, err = cachePvc.CreateOrPatch(ctx, h)
-	}
-	// End PVC creation/patch
-	return ctrlResult, err
-}
-
 // ensureCronJobs - Create the required CronJobs to clean DB entries and image-cache
 // if enabled
 func (r *GlanceReconciler) ensureCronJobs(

diff --git a/pkg/glance/dbsync.go b/pkg/glance/dbsync.go
@@ -37,34 +37,42 @@ func DbSyncJob(
 	annotations map[string]string,
 ) *batchv1.Job {
 	runAsUser := int64(0)
-
-	dbSyncExtraMounts := []glancev1.GlanceExtraVolMounts{}
-	secretNames := []string{}
 	var config0644AccessMode int32 = 0644
 
 	// Unlike the individual glanceAPI services, the DbSyncJob doesn't need a
 	// secret that contains all of the config snippets required by every
 	// service, The two snippet files that it does need (DefaultsConfigFileName
 	// and CustomConfigFileName) can be extracted from the top-level glance
 	// config-data secret.
-	dbSyncVolume := corev1.Volume{
-		Name: "db-sync-config-data",
-		VolumeSource: corev1.VolumeSource{
-			Secret: &corev1.SecretVolumeSource{
-				DefaultMode: &config0644AccessMode,
-				SecretName:  instance.Name + "-config-data",
-				Items: []corev1.KeyToPath{
-					{
-						Key:  DefaultsConfigFileName,
-						Path: DefaultsConfigFileName,
-					},
-					{
-						Key:  CustomConfigFileName,
-						Path: CustomConfigFileName,
+	dbSyncVolume := []corev1.Volume{
+		{
+			Name: "db-sync-config-data",
+			VolumeSource: corev1.VolumeSource{
+				Secret: &corev1.SecretVolumeSource{
+					DefaultMode: &config0644AccessMode,
+					SecretName:  instance.Name + "-config-data",
+					Items: []corev1.KeyToPath{
+						{
+							Key:  DefaultsConfigFileName,
+							Path: DefaultsConfigFileName,
+						},
+						{
+							Key:  CustomConfigFileName,
+							Path: CustomConfigFileName,
+						},
 					},
 				},
 			},
 		},
+		{
+			Name: "config-data",
+			VolumeSource: corev1.VolumeSource{
+				Secret: &corev1.SecretVolumeSource{
+					DefaultMode: &config0644AccessMode,
+					SecretName:  ServiceName + "-config-data",
+				},
+			},
+		},
 	}
 
 	dbSyncMounts := []corev1.VolumeMount{
@@ -116,27 +124,15 @@ func DbSyncJob(
 							SecurityContext: &corev1.SecurityContext{
 								RunAsUser: &runAsUser,
 							},
-							Env: env.MergeEnvs([]corev1.EnvVar{}, envVars),
-							VolumeMounts: append(GetVolumeMounts(
-								secretNames, false,
-								dbSyncExtraMounts, DbsyncPropagation),
-								dbSyncMounts...),
+							Env:          env.MergeEnvs([]corev1.EnvVar{}, envVars),
+							VolumeMounts: dbSyncMounts,
 						},
 					},
 				},
 			},
 		},
 	}
-
-	job.Spec.Template.Spec.Volumes = append(GetVolumes(
-		instance.Name,
-		ServiceName,
-		false,
-		secretNames,
-		dbSyncExtraMounts,
-		DbsyncPropagation),
-		dbSyncVolume,
-	)
+	job.Spec.Template.Spec.Volumes = dbSyncVolume
 
 	return job
 }
diff --git a/pkg/glance/pvc.go b/pkg/glance/pvc.go
@@ -9,7 +9,7 @@ import (
 )
 
 // Pvc - creates and returns a PVC object for a backing store
-func Pvc(api *glancev1.Glance, labels map[string]string, pvcType PvcType) *corev1.PersistentVolumeClaim {
+func GetPvc(api *glancev1.GlanceAPI, labels map[string]string, pvcType PvcType) corev1.PersistentVolumeClaim {
 	// By default we point to a local storage pvc request
 	// that will be customized in case the pvc is requested
 	// for cache purposes
@@ -20,7 +20,7 @@ func Pvc(api *glancev1.Glance, labels map[string]string, pvcType PvcType) *corev
 		// append -cache to avoid confusion when listing PVCs
 		pvcName = fmt.Sprintf("%s-cache", ServiceName)
 	}
-	pvc := &corev1.PersistentVolumeClaim{
+	pvc := corev1.PersistentVolumeClaim{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      pvcName,
 			Namespace: api.Namespace,
@@ -38,6 +38,5 @@ func Pvc(api *glancev1.Glance, labels map[string]string, pvcType PvcType) *corev
 			StorageClassName: &api.Spec.StorageClass,
 		},
 	}
-
 	return pvc
 }
diff --git a/pkg/glance/volumes.go b/pkg/glance/volumes.go
@@ -38,14 +38,6 @@ func GetVolumes(name string, pvcName string, hasCinder bool, secretNames []strin
 				},
 			},
 		},
-		{
-			Name: "lib-data",
-			VolumeSource: corev1.VolumeSource{
-				PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
-					ClaimName: pvcName,
-				},
-			},
-		},
 	}
 
 	for _, exv := range extraVol {
@@ -153,7 +145,7 @@ func GetVolumeMounts(secretNames []string, hasCinder bool, extraVol []glancev1.G
 			ReadOnly:  true,
 		},
 		{
-			Name:      "lib-data",
+			Name:      ServiceName,
 			MountPath: "/var/lib/glance",
 			ReadOnly:  false,
 		},

diff --git a/pkg/glanceapi/deployment.go b/pkg/glanceapi/deployment.go
@@ -231,6 +231,11 @@ func StatefulSet(
 			},
 		},
 	}
+	deployment.Spec.VolumeClaimTemplates = []corev1.PersistentVolumeClaim{glance.GetPvc(instance, labels, glance.PvcLocal)}
+
+	if len(instance.Spec.ImageCacheSize) > 0 {
+		deployment.Spec.VolumeClaimTemplates = append(deployment.Spec.VolumeClaimTemplates, glance.GetPvc(instance, labels, glance.PvcCache))
+	}
 	deployment.Spec.Template.Spec.Volumes = append(glance.GetVolumes(
 		instance.Name,
 		glance.ServiceName,

diff --git a/test/functional/base_test.go b/test/functional/base_test.go
@@ -109,7 +109,8 @@ func GetGlanceDefaultSpecWithQuota() map[string]interface{} {
 
 func GetGlanceAPIDefaultSpec(apiType APIType) map[string]interface{} {
 	return map[string]interface{}{
-		"replicas": 1,
+		"replicas":       1,
+		"storageRequest": glanceTest.GlancePVCSize,
 	}
 }
 
@@ -165,6 +166,7 @@ func GetDefaultGlanceAPITemplate(apiType APIType) map[string]interface{} {
 		"containerImage": glanceTest.ContainerImage,
 		"serviceAccount": glanceTest.GlanceSA.Name,
 		"apiType":        apiType,
+		"storageRequest": glanceTest.GlancePVCSize,
 	}
 }
 

diff --git a/test/functional/glance_controller_test.go b/test/functional/glance_controller_test.go
@@ -240,10 +240,6 @@ var _ = Describe("Glance controller", func() {
 			keystone.SimulateKeystoneServiceReady(glanceTest.Instance)
 			keystone.SimulateKeystoneEndpointReady(glanceTest.GlanceInternalRoute)
 		})
-		It("should have a local pvc", func() {
-			AssertPVCExist(glanceTest.Instance)
-			AssertPVCDoesNotExist(glanceTest.GlanceCache)
-		})
 		It("Creates glanceAPI", func() {
 			GlanceAPIExists(glanceTest.GlanceInternal)
 			GlanceAPIExists(glanceTest.GlanceExternal)
@@ -332,11 +328,13 @@ var _ = Describe("Glance controller", func() {
 			keystone.SimulateKeystoneServiceReady(glanceTest.Instance)
 		})
 		It("Check the resulting endpoints of the generated sub-CRs", func() {
-			th.SimulateDeploymentReadyWithPods(
+			th.SimulateStatefulSetReplicaReadyWithPods(
+				//th.SimulateDeploymentReadyWithPods(
 				glanceTest.GlanceInternalAPI,
 				map[string][]string{glanceName.Namespace + "/internalapi": {"10.0.0.1"}},
 			)
-			th.SimulateDeploymentReadyWithPods(
+			th.SimulateStatefulSetReplicaReadyWithPods(
+				//th.SimulateDeploymentReadyWithPods(
 				glanceTest.GlanceExternalAPI,
 				map[string][]string{glanceName.Namespace + "/internalapi": {"10.0.0.1"}},
 			)