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