From 599ced00187c841b77cecb5d1064de53e24efa7c Mon Sep 17 00:00:00 2001
From: Konstantinos Samaras-Tsakiris <ksamtsak@gmail.com>
Date: Sun, 28 Nov 2021 22:00:32 +0100
Subject: [PATCH 1/4] Fix install job creation
---
controllers/drupalsite_controller.go | 36 +++++++++++++---------------
controllers/drupalsite_resources.go | 18 ++++++++------
2 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/controllers/drupalsite_controller.go b/controllers/drupalsite_controller.go
index b518eeae..43605aa9 100644
--- a/controllers/drupalsite_controller.go
+++ b/controllers/drupalsite_controller.go
@@ -270,7 +270,11 @@ func (r *DrupalSiteReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// Check if the site is installed or cloned and mark the condition
if !drupalSite.ConditionTrue("Initialized") {
- if r.isDrupalSiteInstalled(ctx, drupalSite) || r.isCloneJobCompleted(ctx, drupalSite) {
+ installed, err := r.isDrupalSiteInstalled(ctx, drupalSite)
+ if err != nil {
+ // Initialized? Unknown
+ update = setConditionStatus(drupalSite, "Initialized", false, err, true) || update
+ } else if installed {
update = setInitialized(drupalSite) || update
} else {
update = setNotInitialized(drupalSite) || update
@@ -429,19 +433,6 @@ func (r *DrupalSiteReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// business logic
-// isInstallJobCompleted checks if the drush job is successfully completed
-func (r *DrupalSiteReconciler) isInstallJobCompleted(ctx context.Context, d *webservicesv1a1.DrupalSite) bool {
- found := &batchv1.Job{}
- jobObject := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: "install-" + d.Name, Namespace: d.Namespace}}
- err := r.Get(ctx, types.NamespacedName{Name: jobObject.Name, Namespace: jobObject.Namespace}, found)
- if err == nil {
- if found.Status.Succeeded != 0 {
- return true
- }
- }
- return false
-}
-
// isCloneJobCompleted checks if the clone job is successfully completed
func (r *DrupalSiteReconciler) isCloneJobCompleted(ctx context.Context, d *webservicesv1a1.DrupalSite) bool {
cloneJob := &batchv1.Job{}
@@ -467,14 +458,21 @@ func (r *DrupalSiteReconciler) isDrupalSiteReady(ctx context.Context, d *webserv
}
// isDrupalSiteInstalled checks if the drupal site is initialized by running drush status command in the PHP pod
-func (r *DrupalSiteReconciler) isDrupalSiteInstalled(ctx context.Context, d *webservicesv1a1.DrupalSite) bool {
+func (r *DrupalSiteReconciler) isDrupalSiteInstalled(ctx context.Context, d *webservicesv1a1.DrupalSite) (bool, reconcileError) {
if r.isDrupalSiteReady(ctx, d) {
- if _, err := r.execToServerPodErrOnStderr(ctx, d, "php-fpm", nil, checkIfSiteIsInstalled()...); err != nil {
- return false
+ _, stderr, err := r.execToServerPod(ctx, d, "php-fpm", nil, checkIfSiteIsInstalled()...)
+ // Error running exec => condition unknown
+ if err != nil {
+ return false, newApplicationError(err, ErrClientK8s)
}
- return true
+ // The script executed and returned this error message
+ // TODO: check error code instead of message!
+ if stderr == "Drupal is not installed" {
+ return false, nil
+ }
+ return true, nil
}
- return false
+ return false, nil
}
// isDBODProvisioned checks if the DBOD has been provisioned by checking the status of DBOD custom resource
diff --git a/controllers/drupalsite_resources.go b/controllers/drupalsite_resources.go
index 78908e3a..85b5c831 100644
--- a/controllers/drupalsite_resources.go
+++ b/controllers/drupalsite_resources.go
@@ -247,13 +247,17 @@ func (r *DrupalSiteReconciler) ensureResources(drp *webservicesv1a1.DrupalSite,
transientErrs = append(transientErrs, transientErr.Wrap("%v: for Nginx SVC"))
}
if r.isDBODProvisioned(ctx, drp) {
- if drp.Spec.Configuration.CloneFrom == "" {
- if transientErr := r.ensureResourceX(ctx, drp, "site_install_job", log); transientErr != nil {
- transientErrs = append(transientErrs, transientErr.Wrap("%v: for site install Job"))
- }
- } else {
- if transientErr := r.ensureResourceX(ctx, drp, "clone_job", log); transientErr != nil {
- transientErrs = append(transientErrs, transientErr.Wrap("%v: for clone Job"))
+ // Important check to confirm that the site isn't initialized already,
+ // before creating an install/clone job!
+ if !drp.ConditionTrue("Initialized") {
+ if drp.Spec.Configuration.CloneFrom == "" {
+ if transientErr := r.ensureResourceX(ctx, drp, "site_install_job", log); transientErr != nil {
+ transientErrs = append(transientErrs, transientErr.Wrap("%v: for site install Job"))
+ }
+ } else {
+ if transientErr := r.ensureResourceX(ctx, drp, "clone_job", log); transientErr != nil {
+ transientErrs = append(transientErrs, transientErr.Wrap("%v: for clone Job"))
+ }
}
}
}
--
GitLab
From aba9c7e2e065120096d602274a3dda5f5e1145db Mon Sep 17 00:00:00 2001
From: Konstantinos Samaras-Tsakiris <ksamtsak@gmail.com>
Date: Sun, 28 Nov 2021 22:06:33 +0100
Subject: [PATCH 2/4] Take into account condition being unknown
---
api/v1alpha1/drupalsite_types.go | 6 ++++++
controllers/drupalsite_resources.go | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/api/v1alpha1/drupalsite_types.go b/api/v1alpha1/drupalsite_types.go
index 638b6344..28ac475a 100644
--- a/api/v1alpha1/drupalsite_types.go
+++ b/api/v1alpha1/drupalsite_types.go
@@ -211,6 +211,12 @@ func (drp DrupalSite) ConditionTrue(condition status.ConditionType) (update bool
return init != nil && init.Status == v1.ConditionTrue
}
+// ConditionFalse reports if the condition is false
+func (drp DrupalSite) ConditionFalse(condition status.ConditionType) (update bool) {
+ init := drp.Status.Conditions.GetCondition(condition)
+ return init != nil && init.Status == v1.ConditionFalse
+}
+
// ConditionReasonSet reports if the condition Reason is not empty
func (drp DrupalSite) ConditionReasonSet(condition status.ConditionType) (update bool) {
init := drp.Status.Conditions.GetCondition(condition)
diff --git a/controllers/drupalsite_resources.go b/controllers/drupalsite_resources.go
index 85b5c831..9376679b 100644
--- a/controllers/drupalsite_resources.go
+++ b/controllers/drupalsite_resources.go
@@ -249,7 +249,7 @@ func (r *DrupalSiteReconciler) ensureResources(drp *webservicesv1a1.DrupalSite,
if r.isDBODProvisioned(ctx, drp) {
// Important check to confirm that the site isn't initialized already,
// before creating an install/clone job!
- if !drp.ConditionTrue("Initialized") {
+ if drp.ConditionFalse("Initialized") {
if drp.Spec.Configuration.CloneFrom == "" {
if transientErr := r.ensureResourceX(ctx, drp, "site_install_job", log); transientErr != nil {
transientErrs = append(transientErrs, transientErr.Wrap("%v: for site install Job"))
--
GitLab
From f36361c840fe457d225d14c23cc0624cc9b61597 Mon Sep 17 00:00:00 2001
From: Konstantinos Samaras-Tsakiris <ksamtsak@gmail.com>
Date: Sun, 28 Nov 2021 22:36:07 +0100
Subject: [PATCH 3/4] Make QoS class name 'eco'
---
api/v1alpha1/drupalsite_types.go | 2 +-
config/crd/bases/drupal.webservices.cern.ch_drupalsites.yaml | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/api/v1alpha1/drupalsite_types.go b/api/v1alpha1/drupalsite_types.go
index 28ac475a..ae755f23 100644
--- a/api/v1alpha1/drupalsite_types.go
+++ b/api/v1alpha1/drupalsite_types.go
@@ -75,7 +75,7 @@ type Configuration struct {
// TODO: support branches https://gitlab.cern.ch/drupal/paas/drupalsite-operator/-/issues/28
// QoSClass specifies the website's performance and availability requirements. The default value is "standard".
- // +kubebuilder:validation:Enum:=critical;test;standard
+ // +kubebuilder:validation:Enum:=critical;eco;standard
// +kubebuilder:default=standard
// +optional
QoSClass `json:"qosClass,omitempty"`
diff --git a/config/crd/bases/drupal.webservices.cern.ch_drupalsites.yaml b/config/crd/bases/drupal.webservices.cern.ch_drupalsites.yaml
index b4ab1ba0..2e9d0210 100644
--- a/config/crd/bases/drupal.webservices.cern.ch_drupalsites.yaml
+++ b/config/crd/bases/drupal.webservices.cern.ch_drupalsites.yaml
@@ -82,7 +82,7 @@ spec:
availability requirements. The default value is "standard".
enum:
- critical
- - test
+ - eco
- standard
type: string
webDAVPassword:
--
GitLab
From e04d6e0de6d224f6e938f8dba7f48dcf71342c6a Mon Sep 17 00:00:00 2001
From: Konstantinos Samaras-Tsakiris <konstantinos.samaras-tsakiris@cern.ch>
Date: Sun, 28 Nov 2021 23:07:20 +0100
Subject: [PATCH 4/4] Apply 3 suggestion(s) to 1 file(s)
---
controllers/drupalsite_controller.go | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/controllers/drupalsite_controller.go b/controllers/drupalsite_controller.go
index 43605aa9..21fcf1a1 100644
--- a/controllers/drupalsite_controller.go
+++ b/controllers/drupalsite_controller.go
@@ -273,7 +273,7 @@ func (r *DrupalSiteReconciler) Reconcile(ctx context.Context, req ctrl.Request)
installed, err := r.isDrupalSiteInstalled(ctx, drupalSite)
if err != nil {
// Initialized? Unknown
- update = setConditionStatus(drupalSite, "Initialized", false, err, true) || update
+ update = setConditionStatus(drupalSite, "Initialized", true, err, true) || update
} else if installed {
update = setInitialized(drupalSite) || update
} else {
@@ -463,7 +463,7 @@ func (r *DrupalSiteReconciler) isDrupalSiteInstalled(ctx context.Context, d *web
_, stderr, err := r.execToServerPod(ctx, d, "php-fpm", nil, checkIfSiteIsInstalled()...)
// Error running exec => condition unknown
if err != nil {
- return false, newApplicationError(err, ErrClientK8s)
+ return true, newApplicationError(err, ErrClientK8s)
}
// The script executed and returned this error message
// TODO: check error code instead of message!
@@ -472,7 +472,7 @@ func (r *DrupalSiteReconciler) isDrupalSiteInstalled(ctx context.Context, d *web
}
return true, nil
}
- return false, nil
+ return true, newApplicationError(fmt.Errorf("Can't check install status"), ErrTemporary)
}
// isDBODProvisioned checks if the DBOD has been provisioned by checking the status of DBOD custom resource
--
GitLab