From 0b9e6e5ee2c7c5939a8f838cf524175ea5fca283 Mon Sep 17 00:00:00 2001 From: Francisco Borges Aurindo Barros <francisco.borges.aurindo.barros@cern.ch> Date: Wed, 21 Sep 2022 17:03:45 +0200 Subject: [PATCH] Fetch ReleaseSpecs from SupportedDrupalVersions instead of hardcoded --- .../templates/manager-deploy.yaml | 5 +- chart/drupalsite-operator/values.yaml | 8 +-- controllers/drupalsite_controller.go | 10 +--- controllers/drupalsite_controller_test.go | 53 ++++++++++++++----- controllers/drupalsite_controller_utils.go | 51 ++++++++++++------ controllers/suite_test.go | 8 ++- main.go | 6 +-- 7 files changed, 83 insertions(+), 58 deletions(-) diff --git a/chart/drupalsite-operator/templates/manager-deploy.yaml b/chart/drupalsite-operator/templates/manager-deploy.yaml index f34e9f5b..3cf1db4c 100644 --- a/chart/drupalsite-operator/templates/manager-deploy.yaml +++ b/chart/drupalsite-operator/templates/manager-deploy.yaml @@ -28,14 +28,11 @@ spec: - --webdav-image={{.Values.drupalsiteOperator.webdavImage}} - --zap-stacktrace-level={{.Values.drupalsiteOperator.logStacktraceLevel}} - --zap-log-level={{.Values.drupalsiteOperator.logLevel}} - - --default-d8-release-spec={{.Values.drupalsiteOperator.defaultReleaseSpec}} - - --default-d93-release-spec={{.Values.drupalsiteOperator.defaultD93ReleaseSpec}} - - --default-d93-2-release-spec={{.Values.drupalsiteOperator.defaultD932ReleaseSpec}} - - --default-d94-release-spec={{.Values.drupalsiteOperator.defaultD94ReleaseSpec}} - --parallel-thread-count={{.Values.drupalsiteOperator.parallelThreadCount}} - --enable-topology-spread={{.Values.drupalsiteOperator.enableTopologySpread}} - --cluster-name={{.Values.drupalsiteOperator.clusterName}} - --easystart-backup-name={{.Values.drupalsiteOperator.easystartBackupName}} + - --supported-drupal-version-name={{.Values.drupalsiteOperator.supportedDrupalVersionName}} command: - /manager image: {{ .Values.image | quote }} diff --git a/chart/drupalsite-operator/values.yaml b/chart/drupalsite-operator/values.yaml index 1eb66bc5..ad2414bb 100644 --- a/chart/drupalsite-operator/values.yaml +++ b/chart/drupalsite-operator/values.yaml @@ -23,12 +23,8 @@ drupalsiteOperator: logLevel: "3" # Zap Level at and above which stacktraces are captured (one of 'info', 'error') logStacktraceLevel: "error" - # defaultReleaseSpec refers to the default D8 releaseSpec. In the operator code, it is tagged as 'defaultD8ReleaseSpec' - defaultReleaseSpec: "RELEASE-2022.04.05T09-31-46Z" - # defaultReleaseSpec refers to the default D9 releaseSpec - defaultD93ReleaseSpec: "RELEASE-2022.04.06T07-50-48Z" - defaultD932ReleaseSpec: "RELEASE-2022.04.06T08-04-25Z" - DefaultD94ReleaseSpec: "RELEASE-2022.07.20T13-09-32Z" + # SupportedDrupalVersionName is the name of the cluster-wide resource where the Drupal Versions are stored + supportedDrupalVersionName: "supported-drupal-versions" parallelThreadCount: 1 # Topology spread adds an anti-affinity rule to the server deployment, spreading critical sites across availability zones enableTopologySpread: false diff --git a/controllers/drupalsite_controller.go b/controllers/drupalsite_controller.go index 74c17074..b97ffea5 100644 --- a/controllers/drupalsite_controller.go +++ b/controllers/drupalsite_controller.go @@ -62,14 +62,6 @@ var ( SMTPHost string // VeleroNamespace refers to the namespace of the velero server to create backups VeleroNamespace string - // DefaultD8ReleaseSpec refers to the releaseSpec for Drupal v8.9-2 to be defaulted incase it is empty - DefaultD8ReleaseSpec string - // DefaultD93ReleaseSpec refers to the releaseSpec for Drupal v9.3-1 to be defaulted incase it is empty - DefaultD93ReleaseSpec string - // DefaultD932ReleaseSpec refers to the releaseSpec for Drupal v9.3-2 to be defaulted incase it is empty - DefaultD932ReleaseSpec string - // DefaultD94ReleaseSpec refers to the releaseSpec for Drupal v9.3-2 to be defaulted incase it is empty - DefaultD94ReleaseSpec string // ParallelThreadCount refers to the number of parallel reconciliations done by the Operator ParallelThreadCount int // EnableTopologySpread refers to enabling avaliability zone scheduling for critical site deployments @@ -78,6 +70,8 @@ var ( ClusterName string // EasystartBackupName refers to the name of the easystart backup EasystartBackupName string + // SupportedDrupalVersion Name + SupportedDrupalVersionName string ) // DrupalSiteReconciler reconciles a DrupalSite object diff --git a/controllers/drupalsite_controller_test.go b/controllers/drupalsite_controller_test.go index 8e5b1173..06a6f1b9 100644 --- a/controllers/drupalsite_controller_test.go +++ b/controllers/drupalsite_controller_test.go @@ -81,7 +81,7 @@ var _ = Describe("DrupalSite controller", func() { }, Spec: drupalwebservicesv1alpha1.DrupalSiteSpec{ Version: drupalwebservicesv1alpha1.Version{ - Name: "v8.9-1", + Name: "v9.3-2", ReleaseSpec: "stable", }, Configuration: drupalwebservicesv1alpha1.Configuration{ @@ -387,7 +387,7 @@ var _ = Describe("DrupalSite controller", func() { Name: Name, Namespace: Namespace, } - newVersion := "v8.9-1" + newVersion := "v9.3-2" newReleaseSpec := "new" // Create drupalSite object @@ -747,7 +747,7 @@ var _ = Describe("DrupalSite controller", func() { }, Spec: drupalwebservicesv1alpha1.DrupalSiteSpec{ Version: drupalwebservicesv1alpha1.Version{ - Name: "v8.9-1", + Name: "v9.3-2", ReleaseSpec: "stable", }, Configuration: drupalwebservicesv1alpha1.Configuration{ @@ -1016,7 +1016,7 @@ var _ = Describe("DrupalSite controller", func() { Name: Name + "-advanced", Namespace: "advanced", } - newVersion := "v8.9-1" + newVersion := "v9.3-2" newReleaseSpec := "new" // Create drupalSite object @@ -1139,7 +1139,7 @@ var _ = Describe("DrupalSite controller", func() { }, Spec: drupalwebservicesv1alpha1.DrupalSiteSpec{ Version: drupalwebservicesv1alpha1.Version{ - Name: "v8.9-1", + Name: "v9.3-2", ReleaseSpec: "stable", }, Configuration: drupalwebservicesv1alpha1.Configuration{ @@ -1171,6 +1171,29 @@ var _ = Describe("DrupalSite controller", func() { Name: Name + "-defaults", Namespace: "defaults", } + // Create a supporteddrupalversions resource for defaults + supportedDrupalVersionObject := &drupalwebservicesv1alpha1.SupportedDrupalVersions{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "drupal.webservices.cern.ch/v1alpha1", + Kind: "SupportedDrupalVersions", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "supported-drupal-versions", + }, + } + + By("By creating a supportedDrupalVersion resource for the drupalSite") + Eventually(func() error { + return k8sClient.Create(ctx, supportedDrupalVersionObject) + }, timeout, interval).Should(Succeed()) + + // Check if the supportedDrupalVersion resource is created + By("By checking if supportedDrupalVersion resource is created") + supportedDrupalVersionObject1 := drupalwebservicesv1alpha1.SupportedDrupalVersions{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "supported-drupal-versions", Namespace: ""}, &supportedDrupalVersionObject1) + }, timeout, interval).Should(Succeed()) + drupalSiteObject = &drupalwebservicesv1alpha1.DrupalSite{ TypeMeta: metav1.TypeMeta{ APIVersion: "drupal.webservices.cern.ch/v1alpha1", @@ -1182,7 +1205,7 @@ var _ = Describe("DrupalSite controller", func() { }, Spec: drupalwebservicesv1alpha1.DrupalSiteSpec{ Version: drupalwebservicesv1alpha1.Version{ - Name: "v8.9-1", + Name: "v9.3-2", }, SiteURL: []drupalwebservicesv1alpha1.Url{dummySiteUrl}, }, @@ -1218,11 +1241,9 @@ var _ = Describe("DrupalSite controller", func() { k8sClient.Get(ctx, key, &cr) return string(cr.Spec.Configuration.DiskSize) == "2000Mi" }, timeout, interval).Should(BeTrue()) - - By("Expecting the default configuration values to be set") Eventually(func() bool { k8sClient.Get(ctx, key, &cr) - return cr.Spec.Version.ReleaseSpec == DefaultD8ReleaseSpec + return len(cr.Spec.Version.ReleaseSpec) > 0 }, timeout, interval).Should(BeTrue()) trueVar := true @@ -1373,7 +1394,7 @@ var _ = Describe("DrupalSite controller", func() { }, Spec: drupalwebservicesv1alpha1.DrupalSiteSpec{ Version: drupalwebservicesv1alpha1.Version{ - Name: "v8.9-1", + Name: "v9.3-2", }, Configuration: drupalwebservicesv1alpha1.Configuration{ DatabaseClass: drupalwebservicesv1alpha1.DBODStandard, @@ -1405,6 +1426,10 @@ var _ = Describe("DrupalSite controller", func() { k8sClient.Get(ctx, key, &cr) return string(cr.Spec.Configuration.DiskSize) == "2000Mi" }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { + k8sClient.Get(ctx, key, &cr) + return len(cr.Spec.Version.ReleaseSpec) > 0 + }, timeout, interval).Should(BeTrue()) By("Expecting to delete successfully") Eventually(func() error { @@ -1428,7 +1453,7 @@ var _ = Describe("DrupalSite controller", func() { }, Spec: drupalwebservicesv1alpha1.DrupalSiteSpec{ Version: drupalwebservicesv1alpha1.Version{ - Name: "v8.9-1", + Name: "v9.3-2", }, Configuration: drupalwebservicesv1alpha1.Configuration{ QoSClass: "randomval", @@ -1471,7 +1496,7 @@ var _ = Describe("DrupalSite controller", func() { }, Spec: drupalwebservicesv1alpha1.DrupalSiteSpec{ Version: drupalwebservicesv1alpha1.Version{ - Name: "v8.9-1", + Name: "v9.3-2", ReleaseSpec: "stable", }, Configuration: drupalwebservicesv1alpha1.Configuration{ @@ -1856,7 +1881,7 @@ var _ = Describe("DrupalSite controller", func() { Name: Name + "-critical", Namespace: "critical", } - newVersion := "v8.9-1" + newVersion := "v9.3-2" newReleaseSpec := "new" // Create drupalSite object @@ -1978,7 +2003,7 @@ var _ = Describe("DrupalSite controller", func() { }, Spec: drupalwebservicesv1alpha1.DrupalSiteSpec{ Version: drupalwebservicesv1alpha1.Version{ - Name: "v8.9-1", + Name: "v9.3-2", ReleaseSpec: "stable", }, Configuration: drupalwebservicesv1alpha1.Configuration{ diff --git a/controllers/drupalsite_controller_utils.go b/controllers/drupalsite_controller_utils.go index 20934d52..94118d4f 100644 --- a/controllers/drupalsite_controller_utils.go +++ b/controllers/drupalsite_controller_utils.go @@ -21,7 +21,6 @@ import ( "encoding/hex" "errors" "fmt" - "strings" "github.com/go-logr/logr" buildv1 "github.com/openshift/api/build/v1" @@ -179,15 +178,16 @@ func (r *DrupalSiteReconciler) ensureSpecFinalizer(ctx context.Context, drp *web if !controllerutil.ContainsFinalizer(drp, finalizerStr) { log.V(3).Info("Adding finalizer") controllerutil.AddFinalizer(drp, finalizerStr) - update = true + update = true || update } if drp.Spec.Configuration.WebDAVPassword == "" { drp.Spec.Configuration.WebDAVPassword = generateRandomPassword() - update = true + update = true || update } // Set default value for DiskSize to 2000Mi if drp.Spec.Configuration.CloneFrom == "" && drp.Spec.Configuration.DiskSize == "" { drp.Spec.Configuration.DiskSize = "2000Mi" + update = true || update } // Validate that CloneFrom is an existing DrupalSite if drp.Spec.Configuration.CloneFrom != "" { @@ -195,9 +195,9 @@ func (r *DrupalSiteReconciler) ensureSpecFinalizer(ctx context.Context, drp *web err := r.Get(ctx, types.NamespacedName{Name: string(drp.Spec.Configuration.CloneFrom), Namespace: drp.Namespace}, &sourceSite) switch { case k8sapierrors.IsNotFound(err): - return false, newApplicationError(fmt.Errorf("CloneFrom DrupalSite doesn't exist"), ErrInvalidSpec) + return update, newApplicationError(fmt.Errorf("CloneFrom DrupalSite doesn't exist"), ErrInvalidSpec) case err != nil: - return false, newApplicationError(err, ErrClientK8s) + return update, newApplicationError(err, ErrClientK8s) } // The destination disk size must be at least as large as the source if drp.Spec.Configuration.DiskSize < sourceSite.Spec.Configuration.DiskSize { @@ -213,22 +213,39 @@ func (r *DrupalSiteReconciler) ensureSpecFinalizer(ctx context.Context, drp *web drp.Spec.Configuration.ExtraConfigurationRepository.Branch = sourceSite.Spec.Configuration.ExtraConfigurationRepository.Branch drp.Spec.Configuration.ExtraConfigurationRepository.RepositoryUrl = sourceSite.Spec.Configuration.ExtraConfigurationRepository.RepositoryUrl } + update = true || update } // Initialize 'spec.version.releaseSpec' if empty if len(drp.Spec.Version.ReleaseSpec) == 0 { - switch { - case strings.HasPrefix(drp.Spec.Version.Name, "v8"): - drp.Spec.Version.ReleaseSpec = DefaultD8ReleaseSpec - case strings.HasPrefix(drp.Spec.Version.Name, "v9.3-1"): - drp.Spec.Version.ReleaseSpec = DefaultD93ReleaseSpec - case strings.HasPrefix(drp.Spec.Version.Name, "v9.3-2"): - drp.Spec.Version.ReleaseSpec = DefaultD932ReleaseSpec - case strings.HasPrefix(drp.Spec.Version.Name, "v9.4-1"): - drp.Spec.Version.ReleaseSpec = DefaultD94ReleaseSpec - default: - log.V(3).Info("Cannot set default ReleaseSpec for version " + drp.Spec.Version.Name) + // Fetch the SupportedDrupalVersions instance + supportedDrupalVersions := &webservicesv1a1.SupportedDrupalVersions{} + namespacedName := types.NamespacedName{Name: SupportedDrupalVersionName, Namespace: ""} + err := r.Get(ctx, namespacedName, supportedDrupalVersions) + if err != nil { + if k8sapierrors.IsNotFound(err) { + // Request object not found, could have been deleted after reconcile request. + // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. + // Return and don't requeue + log.V(3).Info(fmt.Sprintf("SupportedDrupalVersions '%s' resource not found. Ignoring since object must be deleted", SupportedDrupalVersionName)) + return update, nil + } + // Error reading the object - fail the request. + log.Error(err, fmt.Sprintf("Failed to get SupportedDrupalVersions '%s'", SupportedDrupalVersionName)) + return update, newApplicationError(err, ErrClientK8s) + } + // Iterate over available versions to find if the one requested has a ReleaseSpec + for _, v := range supportedDrupalVersions.Status.AvailableVersions { + if drp.Spec.Name == v.Name { + drp.Spec.Version.ReleaseSpec = v.LatestReleaseSpec + update = true || update + break + } + } + // If no available ReleaseSpec, we log and don't update + if drp.Spec.Version.ReleaseSpec == "" { + log.V(3).Info(fmt.Sprintf("Failed to get a ReleaseSpec for version %s", drp.Spec.Version.Name)) + return update, nil } - update = true } return update, nil } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 4d1707f8..bda9695b 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -76,16 +76,14 @@ var _ = BeforeSuite(func(done Done) { // apiServerFlags := append([]string(nil), envtest.DefaultKubeAPIServerFlags...) // apiServerFlags = append(apiServerFlags, customApiServerFlags...) - SiteBuilderImage = "gitlab-registry.cern.ch/drupal/paas/drupal-runtime/site-builder" + SiteBuilderImage = "gitlab-registry.cern.ch/drupal/paas/cern-drupal-distribution/site-builder" VeleroNamespace = "openshift-cern-drupal" PhpFpmExporterImage = "test-phpfpmexporter" WebDAVImage = "test-webdav" - DefaultD8ReleaseSpec = "test-d8-spec" - DefaultD93ReleaseSpec = "test-d93-spec" - DefaultD932ReleaseSpec = "test-d93-2-spec" - DefaultD94ReleaseSpec = "test-d94-spec" + SupportedDrupalVersionName = "supported-drupal-versions" ClusterName = "test" EasystartBackupName = "easystart-backup" + SupportedDrupalVersionName = "supported-drupal-versions" By("bootstrapping test environment") testEnv = &envtest.Environment{ diff --git a/main.go b/main.go index bffc418e..692c3b0c 100644 --- a/main.go +++ b/main.go @@ -81,14 +81,12 @@ func main() { flag.StringVar(&controllers.WebDAVImage, "webdav-image", "gitlab-registry.cern.ch/drupal/paas/sabredav/webdav:RELEASE-2021.10.12T17-55-06Z", "The webdav source image name.") flag.StringVar(&controllers.SMTPHost, "smtp-host", "cernmx.cern.ch", "SMTP host used by Drupal server pods to send emails.") flag.StringVar(&controllers.VeleroNamespace, "velero-namespace", "openshift-cern-drupal", "The namespace of the Velero server to create backups") - flag.StringVar(&controllers.DefaultD8ReleaseSpec, "default-d8-release-spec", "RELEASE-2022.04.01T08-06-09Z", "The default releaseSpec value for version v8.9-2") - flag.StringVar(&controllers.DefaultD93ReleaseSpec, "default-d93-release-spec", "RELEASE-2022.04.01T08-06-16Z", "The default releaseSpec value for version v9.3-1") - flag.StringVar(&controllers.DefaultD932ReleaseSpec, "default-d93-2-release-spec", "RELEASE-2022.04.03T13-24-35Z", "The default releaseSpec value for version v9.3-2") - flag.StringVar(&controllers.DefaultD94ReleaseSpec, "default-d94-release-spec", "RELEASE-2022.07.20T13-09-32Z", "The default releaseSpec value for version v9.4-1") flag.IntVar(&controllers.ParallelThreadCount, "parallel-thread-count", 1, "The default number of parallel threads executed by the DrupalSite Operator controllers") flag.BoolVar(&controllers.EnableTopologySpread, "enable-topology-spread", false, "Enable avaliability zone scheduling for critical site deployments") flag.StringVar(&controllers.ClusterName, "cluster-name", "", "Name of the cluster the operator is deployed on") flag.StringVar(&controllers.EasystartBackupName, "easystart-backup-name", "", "The name of the easy-start backup") + // The variable name is set here: https://gitlab.cern.ch/drupal/paas/cern-drupal-distribution/-/blob/master/supporteddrupalversions/chart/templates/supported-drupal-versions.yaml + flag.StringVar(&controllers.SupportedDrupalVersionName, "supported-drupal-version-name", "supported-drupal-versions", "The name of the resource used cluster-wide for supported drupal versions") opts := zap.Options{ Development: false, } -- GitLab