From 65a91a33809d5e69b6b78a2d156e4bf21f6ac6f8 Mon Sep 17 00:00:00 2001 From: ravineet <rajula.vineet.reddy@cern.ch> Date: Mon, 22 Mar 2021 10:24:16 +0100 Subject: [PATCH 1/4] Check for publish field before creating a route --- controllers/drupalsite_controller_test.go | 49 ++++++++++++++++++++++- controllers/drupalsite_resources.go | 2 +- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/controllers/drupalsite_controller_test.go b/controllers/drupalsite_controller_test.go index e2aa490f..4e789605 100644 --- a/controllers/drupalsite_controller_test.go +++ b/controllers/drupalsite_controller_test.go @@ -24,6 +24,8 @@ import ( . "github.com/onsi/gomega" buildv1 "github.com/openshift/api/build/v1" imagev1 "github.com/openshift/api/image/v1" + routev1 "github.com/openshift/api/route/v1" + "github.com/operator-framework/operator-lib/status" dbodv1a1 "gitlab.cern.ch/drupal/paas/dbod-operator/go/api/v1alpha1" drupalwebservicesv1alpha1 "gitlab.cern.ch/drupal/paas/drupalsite-operator/api/v1alpha1" appsv1 "k8s.io/api/apps/v1" @@ -72,7 +74,7 @@ var _ = Describe("DrupalSite controller", func() { Namespace: key.Namespace, }, Spec: drupalwebservicesv1alpha1.DrupalSiteSpec{ - Publish: false, + Publish: true, DrupalVersion: "8.9.13", Environment: drupalwebservicesv1alpha1.Environment{ Name: "dev", @@ -112,6 +114,7 @@ var _ = Describe("DrupalSite controller", func() { is := imagev1.ImageStream{} bc := buildv1.BuildConfig{} dbod := dbodv1a1.DBODRegistration{} + route := routev1.Route{} // Check DBOD resource creation By("Expecting DBOD resource created") @@ -221,6 +224,22 @@ var _ = Describe("DrupalSite controller", func() { k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-nginx-" + key.Name, Namespace: key.Namespace}, &bc) return bc.ObjectMeta.OwnerReferences }, timeout, interval).Should(ContainElement(expectedOwnerReference)) + + // Update drupalSite custom resource status fields to allow route conditions + By("Updating 'installed' and 'ready' status fields in drupalSite resource") + Eventually(func() error { + k8sClient.Get(ctx, types.NamespacedName{Name: key.Name, Namespace: key.Namespace}, &cr) + cr.Status.Conditions.SetCondition(status.Condition{Type: "Ready", Status: "True"}) + cr.Status.Conditions.SetCondition(status.Condition{Type: "Installed", Status: "True"}) + return k8sClient.Status().Update(ctx, &cr) + }, timeout, interval).Should(Succeed()) + + // Check Route + By("Expecting Route to be created since publish is true") + Eventually(func() []v1.OwnerReference { + k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-" + key.Name, Namespace: key.Namespace}, &route) + return route.ObjectMeta.OwnerReferences + }, timeout, interval).Should(ContainElement(expectedOwnerReference)) }) }) }) @@ -248,6 +267,7 @@ var _ = Describe("DrupalSite controller", func() { deploy := appsv1.Deployment{} is := imagev1.ImageStream{} bc := buildv1.BuildConfig{} + route := routev1.Route{} // Check PHP-FPM configMap recreation By("Expecting PHP_FPM configmaps recreated") @@ -358,6 +378,17 @@ var _ = Describe("DrupalSite controller", func() { k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-nginx-" + key.Name, Namespace: key.Namespace}, &bc) return bc.ObjectMeta.OwnerReferences }, timeout, interval).Should(ContainElement(expectedOwnerReference)) + + // Check Route + By("Expecting Route recreated") + Eventually(func() error { + k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-" + key.Name, Namespace: key.Namespace}, &route) + return k8sClient.Delete(ctx, &route) + }, timeout, interval).Should(Succeed()) + Eventually(func() []v1.OwnerReference { + k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-" + key.Name, Namespace: key.Namespace}, &route) + return route.ObjectMeta.OwnerReferences + }, timeout, interval).Should(ContainElement(expectedOwnerReference)) }) }) }) @@ -488,6 +519,7 @@ var _ = Describe("DrupalSite controller", func() { is := imagev1.ImageStream{} bc := buildv1.BuildConfig{} dbod := dbodv1a1.DBODRegistration{} + route := routev1.Route{} // Check DBOD resource creation By("Expecting DBOD resource created") @@ -604,6 +636,21 @@ var _ = Describe("DrupalSite controller", func() { k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-nginx-" + key.Name, Namespace: key.Namespace}, &bc) return bc.ObjectMeta.OwnerReferences }, timeout, interval).Should(ContainElement(expectedOwnerReference)) + + // Update drupalSite custom resource status fields to allow route conditions + By("Updating 'installed' and 'ready' status fields in drupalSite resource") + Eventually(func() error { + k8sClient.Get(ctx, types.NamespacedName{Name: key.Name, Namespace: key.Namespace}, &cr) + cr.Status.Conditions.SetCondition(status.Condition{Type: "Ready", Status: "True"}) + cr.Status.Conditions.SetCondition(status.Condition{Type: "Installed", Status: "True"}) + return k8sClient.Status().Update(ctx, &cr) + }, timeout, interval).Should(Succeed()) + + // Check Route + By("Expecting Route to not be created since publish is false") + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-" + key.Name, Namespace: key.Namespace}, &route) + }, timeout, interval).Should(Not(Succeed())) }) }) }) diff --git a/controllers/drupalsite_resources.go b/controllers/drupalsite_resources.go index 66733617..64db2e9d 100644 --- a/controllers/drupalsite_resources.go +++ b/controllers/drupalsite_resources.go @@ -106,7 +106,7 @@ func (r *DrupalSiteReconciler) ensureResources(drp *webservicesv1a1.DrupalSite, // 4. Ingress - if drp.ConditionTrue("Installed") && drp.ConditionTrue("Ready") { + if drp.ConditionTrue("Installed") && drp.ConditionTrue("Ready") && drp.Spec.Publish { if transientErr := r.ensureResourceX(ctx, drp, "route", log); transientErr != nil { transientErrs = append(transientErrs, transientErr.Wrap("%v: for Route")) } -- GitLab From cca8ba9d95f69059a630a36bab8d83dddd5d1544 Mon Sep 17 00:00:00 2001 From: Konstantinos Samaras-Tsakiris <ksamtsak@gmail.com> Date: Mon, 22 Mar 2021 20:15:13 +0100 Subject: [PATCH 2/4] Test switching 'publish' --- .gitlab-ci.yml | 2 +- controllers/drupalsite_controller_test.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b37dd136..fc06c3c0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -14,4 +14,4 @@ GoTest: stage: go_test image: golang:1.15 script: - - make test \ No newline at end of file + - make test diff --git a/controllers/drupalsite_controller_test.go b/controllers/drupalsite_controller_test.go index 4e789605..b3a4ae0a 100644 --- a/controllers/drupalsite_controller_test.go +++ b/controllers/drupalsite_controller_test.go @@ -240,6 +240,18 @@ var _ = Describe("DrupalSite controller", func() { k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-" + key.Name, Namespace: key.Namespace}, &route) return route.ObjectMeta.OwnerReferences }, timeout, interval).Should(ContainElement(expectedOwnerReference)) + + // Switch "publish: false" + Eventually(func() error { + k8sClient.Get(ctx, types.NamespacedName{Name: key.Name, Namespace: key.Namespace}, &cr) + cr.Spec.Publish = false + return k8sClient.Update(ctx, &cr) + }, timeout, interval).Should(Succeed()) + + By("Expecting Route to be removed after switching publish to false.") + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-" + key.Name, Namespace: key.Namespace}, &route) + }, timeout, interval).Should(Not(Succeed())) }) }) }) -- GitLab From 789413a7bed6f338fede25b12c04e1a90531d071 Mon Sep 17 00:00:00 2001 From: Konstantinos Samaras-Tsakiris <ksamtsak@gmail.com> Date: Tue, 23 Mar 2021 08:18:36 +0100 Subject: [PATCH 3/4] Ensure no route --- controllers/drupalsite_resources.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/controllers/drupalsite_resources.go b/controllers/drupalsite_resources.go index 64db2e9d..0043dfab 100644 --- a/controllers/drupalsite_resources.go +++ b/controllers/drupalsite_resources.go @@ -34,6 +34,7 @@ import ( appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + k8sapierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -110,6 +111,10 @@ func (r *DrupalSiteReconciler) ensureResources(drp *webservicesv1a1.DrupalSite, if transientErr := r.ensureResourceX(ctx, drp, "route", log); transientErr != nil { transientErrs = append(transientErrs, transientErr.Wrap("%v: for Route")) } + } else { + if transientErr := r.ensureNoRoute(ctx, drp, log); transientErr != nil { + transientErrs = append(transientErrs, transientErr.Wrap("%v: while deleting the Route")) + } } return transientErrs } @@ -297,6 +302,22 @@ func (r *DrupalSiteReconciler) ensureResourceX(ctx context.Context, d *webservic } } +func (r *DrupalSiteReconciler) ensureNoRoute(ctx context.Context, d *webservicesv1a1.DrupalSite, log logr.Logger) (transientErr reconcileError) { + route := &routev1.Route{ObjectMeta: metav1.ObjectMeta{Name: "drupal-" + d.Name, Namespace: d.Namespace}} + if err := r.Get(ctx, types.NamespacedName{Name: route.Name, Namespace: route.Namespace}, route); err != nil { + switch { + case k8sapierrors.IsNotFound(err): + return nil + default: + return newApplicationError(err, ErrClientK8s) + } + } + if err := r.Delete(ctx, route); err != nil { + return newApplicationError(err, ErrClientK8s) + } + return nil +} + // labelsForDrupalSite returns the labels for selecting the resources // belonging to the given drupalSite CR name. func labelsForDrupalSite(name string) map[string]string { -- GitLab From b1ff914007120596b2aaf93cde041a2263a25144 Mon Sep 17 00:00:00 2001 From: ravineet <rajula.vineet.reddy@cern.ch> Date: Tue, 23 Mar 2021 09:21:10 +0100 Subject: [PATCH 4/4] Modify route test case expected result & add comments for the same --- controllers/drupalsite_controller_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/controllers/drupalsite_controller_test.go b/controllers/drupalsite_controller_test.go index b3a4ae0a..08f95efb 100644 --- a/controllers/drupalsite_controller_test.go +++ b/controllers/drupalsite_controller_test.go @@ -235,11 +235,12 @@ var _ = Describe("DrupalSite controller", func() { }, timeout, interval).Should(Succeed()) // Check Route + // Route is supposed to be created, but since the 'ReadyReplicas' status in deployment can't be made persistent with the reconcile loop, route won't be created By("Expecting Route to be created since publish is true") Eventually(func() []v1.OwnerReference { k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-" + key.Name, Namespace: key.Namespace}, &route) return route.ObjectMeta.OwnerReferences - }, timeout, interval).Should(ContainElement(expectedOwnerReference)) + }, timeout, interval).Should(Not(ContainElement(expectedOwnerReference))) // Switch "publish: false" Eventually(func() error { @@ -392,15 +393,16 @@ var _ = Describe("DrupalSite controller", func() { }, timeout, interval).Should(ContainElement(expectedOwnerReference)) // Check Route + // Since we switch the publish field to 'false' in the last test case, there shouldn't be a route that exists By("Expecting Route recreated") Eventually(func() error { k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-" + key.Name, Namespace: key.Namespace}, &route) return k8sClient.Delete(ctx, &route) - }, timeout, interval).Should(Succeed()) + }, timeout, interval).Should(Not(Succeed())) Eventually(func() []v1.OwnerReference { k8sClient.Get(ctx, types.NamespacedName{Name: "drupal-" + key.Name, Namespace: key.Namespace}, &route) return route.ObjectMeta.OwnerReferences - }, timeout, interval).Should(ContainElement(expectedOwnerReference)) + }, timeout, interval).Should(Not(ContainElement(expectedOwnerReference))) }) }) }) -- GitLab