Change WatcherAPI deployment to a statefulset

config/rbac/role.yaml

@@ -27,18 +27,6 @@ rules:
   - patch
   - update
   - watch
-- apiGroups:
-  - apps
-  resources:
-  - deployments
-  verbs:
-  - create
-  - delete
-  - get
-  - list
-  - patch
-  - update
-  - watch
 - apiGroups:
   - apps
   resources:

controllers/watcherapi_controller.go

@@ -23,7 +23,6 @@ import (
 	"net/url"
 	"path/filepath"
 	"strings"
-	"time"
 
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -39,12 +38,12 @@ import (
 	keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/lib-common/modules/common"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/deployment"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/endpoint"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/env"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/labels"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/statefulset"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 
@@ -83,7 +82,7 @@ func (r *WatcherAPIReconciler) GetLogger(ctx context.Context) logr.Logger {
 //+kubebuilder:rbac:groups=keystone.openstack.org,resources=keystoneendpoints,verbs=get;list;watch;create;update;patch;delete;
 //+kubebuilder:rbac:groups=memcached.openstack.org,resources=memcacheds,verbs=get;list;watch;update;patch
 //+kubebuilder:rbac:groups=memcached.openstack.org,resources=memcacheds/finalizers,verbs=update;patch
-//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete;
+//+kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch;create;update;patch;delete;
 //+kubebuilder:rbac:groups=route.openshift.io,resources=routes,verbs=get;list;watch;create;update;patch;delete;
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
@@ -166,6 +165,9 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			instance.Spec.PasswordSelectors.Service,
 			TransportURLSelector,
 			DatabaseAccount,
+			DatabaseUsername,
+			DatabaseHostname,
+			DatabasePassword,
 			watcher.GlobalCustomConfigFileName,
 		},
 		helper.GetClient(),
@@ -253,11 +255,17 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage)
 
-	result, err = r.createDeployment(ctx, helper, instance, prometheusSecret, inputHash)
+	result, err = r.ensureDeployment(ctx, helper, instance, prometheusSecret, inputHash)
 	if err != nil {
 		return result, err
 	}
 
+	// Only expose the service if the deployment succeeded
+	if !instance.Status.Conditions.IsTrue(condition.DeploymentReadyCondition) {
+		Log.Info("Waiting for the Deployment to become Ready before exposing the service in Keystone")
+		return ctrl.Result{}, nil
+	}
+
 	apiEndpoints, result, err := r.ensureServiceExposed(ctx, helper, instance)
 	if (err != nil || result != ctrl.Result{}) {
 		// We can ignore RequeueAfter as we are watching the Service resource
@@ -383,7 +391,7 @@ func (r *WatcherAPIReconciler) generateServiceConfigs(
 	return GenerateConfigsGeneric(ctx, helper, instance, envVars, templateParameters, customData, labels, false)
 }
 
-func (r *WatcherAPIReconciler) createDeployment(
+func (r *WatcherAPIReconciler) ensureDeployment(
 	ctx context.Context,
 	helper *helper.Helper,
 	instance *watcherv1beta1.WatcherAPI,
@@ -405,9 +413,10 @@ func (r *WatcherAPIReconciler) createDeployment(
 
 	}
 
-	// define a new Deployment object
-	deploymentDef, err := watcherapi.Deployment(instance, configHash, prometheusCaCert, getAPIServiceLabels())
+	// define a new StatefulSet object
+	statefulSetDef, err := watcherapi.StatefulSet(instance, configHash, prometheusCaCert, getAPIServiceLabels())
 	if err != nil {
+		Log.Error(err, "Defining statefulSet failed")
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.DeploymentReadyCondition,
 			condition.ErrorReason,
@@ -417,11 +426,11 @@ func (r *WatcherAPIReconciler) createDeployment(
 		return ctrl.Result{}, err
 	}
 
-	Log.Info(fmt.Sprintf("Getting deployment '%s'", instance.Name))
-	deploymentObject := deployment.NewDeployment(deploymentDef, time.Duration(5)*time.Second)
-	Log.Info(fmt.Sprintf("Got deployment '%s'", instance.Name))
-	ctrlResult, errorResult := deploymentObject.CreateOrPatch(ctx, helper)
+	Log.Info(fmt.Sprintf("Getting statefulSet '%s'", instance.Name))
+	statefulSetObject := statefulset.NewStatefulSet(statefulSetDef, r.RequeueTimeout)
+	ctrlResult, errorResult := statefulSetObject.CreateOrPatch(ctx, helper)
 	if errorResult != nil {
+		Log.Error(err, "Deployment failed")
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.DeploymentReadyCondition,
 			condition.ErrorReason,
@@ -430,6 +439,7 @@ func (r *WatcherAPIReconciler) createDeployment(
 			errorResult.Error()))
 		return ctrlResult, errorResult
 	} else if (ctrlResult != ctrl.Result{}) {
+		Log.Info("Deployment in progress")
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.DeploymentReadyCondition,
 			condition.RequestedReason,
@@ -438,11 +448,17 @@ func (r *WatcherAPIReconciler) createDeployment(
 		return ctrlResult, nil
 	}
 
-	instance.Status.ReadyCount = deploymentObject.GetDeployment().Status.ReadyReplicas
+	Log.Info(fmt.Sprintf("Got statefulSet '%s'", instance.Name))
+	statefulSet := statefulSetObject.GetStatefulSet()
+	if statefulSet.Generation == statefulSet.Status.ObservedGeneration {
+		instance.Status.ReadyCount = statefulSet.Status.ReadyReplicas
+	}
 
-	if instance.Status.ReadyCount > 0 {
+	if instance.Status.ReadyCount == *instance.Spec.Replicas && statefulSet.Generation == statefulSet.Status.ObservedGeneration {
+		Log.Info("Deployment is ready")
 		instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
 	} else {
+		Log.Info("Deployment not ready")
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.DeploymentReadyCondition,
 			condition.RequestedReason,
@@ -681,7 +697,7 @@ func (r *WatcherAPIReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&watcherv1beta1.WatcherAPI{}).
 		Owns(&corev1.Secret{}).
-		Owns(&appsv1.Deployment{}).
+		Owns(&appsv1.StatefulSet{}).
 		Owns(&corev1.Service{}).
 		Owns(&routev1.Route{}).
 		Owns(&keystonev1.KeystoneEndpoint{}).

pkg/watcherapi/deployment.go → pkg/watcherapi/statefulset.go

@@ -3,11 +3,14 @@ package watcherapi
 import (
 	"path/filepath"
 
+	"github.com/openstack-k8s-operators/lib-common/modules/common"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/affinity"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/env"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/utils/ptr"
 
 	watcherv1beta1 "github.com/openstack-k8s-operators/watcher-operator/api/v1beta1"
 	watcher "github.com/openstack-k8s-operators/watcher-operator/pkg/watcher"
@@ -18,28 +21,31 @@ const (
 	ServiceCommand = "/usr/local/bin/kolla_start"
 )
 
-// Deployment - returns a WatcherAPI Deployment
-func Deployment(
+// StatefulSet - returns a WatcherAPI StatefulSet
+func StatefulSet(
 	instance *watcherv1beta1.WatcherAPI,
 	configHash string,
 	prometheusCaCertSecret map[string]string,
 	labels map[string]string,
-) (*appsv1.Deployment, error) {
+) (*appsv1.StatefulSet, error) {
 
-	runAsUser := int64(0)
 	var config0644AccessMode int32 = 0644
 	envVars := map[string]env.Setter{}
 	envVars["KOLLA_CONFIG_STRATEGY"] = env.SetValue("COPY_ALWAYS")
 	envVars["CONFIG_HASH"] = env.SetValue(configHash)
+	// This allows the pod to start up slowly. The pod will only be killed
+	// if it does not succeed a probe in 60 seconds.
+	startupProbe := &corev1.Probe{
+		FailureThreshold: 6,
+		PeriodSeconds:    10,
+	}
 	livenessProbe := &corev1.Probe{
-		TimeoutSeconds:      5,
-		PeriodSeconds:       3,
-		InitialDelaySeconds: 5,
+		TimeoutSeconds: 5,
+		PeriodSeconds:  5,
 	}
 	readinessProbe := &corev1.Probe{
-		TimeoutSeconds:      5,
-		PeriodSeconds:       5,
-		InitialDelaySeconds: 5,
+		TimeoutSeconds: 5,
+		PeriodSeconds:  5,
 	}
 	args := []string{"-c", ServiceCommand}
 
@@ -50,6 +56,7 @@ func Deployment(
 		Port: intstr.IntOrString{Type: intstr.Int, IntVal: int32(watcher.WatcherPublicPort)},
 	}
 	readinessProbe.HTTPGet = livenessProbe.HTTPGet
+	startupProbe.HTTPGet = livenessProbe.HTTPGet
 
 	apiVolumes := append(watcher.GetLogVolume(),
 		corev1.Volume{
@@ -95,17 +102,18 @@ func Deployment(
 		)
 	}
 
-	deployment := &appsv1.Deployment{
+	statefulSet := &appsv1.StatefulSet{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      instance.Name,
 			Namespace: instance.Namespace,
 			Labels:    labels,
 		},
-		Spec: appsv1.DeploymentSpec{
+		Spec: appsv1.StatefulSetSpec{
 			Selector: &metav1.LabelSelector{
 				MatchLabels: labels,
 			},
-			Replicas: instance.Spec.Replicas,
+			PodManagementPolicy: appsv1.ParallelPodManagement,
+			Replicas:            instance.Spec.Replicas,
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: labels,
@@ -128,13 +136,14 @@ func Deployment(
 							},
 							Image: instance.Spec.ContainerImage,
 							SecurityContext: &corev1.SecurityContext{
-								RunAsUser: &runAsUser,
+								RunAsUser: ptr.To(watcher.WatcherUserID),
 							},
 							Env:            env.MergeEnvs([]corev1.EnvVar{}, envVars),
 							VolumeMounts:   watcher.GetLogVolumeMount(),
 							Resources:      instance.Spec.Resources,
 							ReadinessProbe: readinessProbe,
 							LivenessProbe:  livenessProbe,
+							StartupProbe:   startupProbe,
 						},
 						{
 							Name: watcher.ServiceName + "-api",
@@ -144,7 +153,7 @@ func Deployment(
 							Args:  args,
 							Image: instance.Spec.ContainerImage,
 							SecurityContext: &corev1.SecurityContext{
-								RunAsUser: &runAsUser,
+								RunAsUser: ptr.To(watcher.WatcherUserID),
 							},
 							Env: env.MergeEnvs([]corev1.EnvVar{}, envVars),
 							VolumeMounts: append(watcher.GetVolumeMounts(
@@ -156,18 +165,28 @@ func Deployment(
 							LivenessProbe:  livenessProbe,
 						},
 					},
+					// If possible two pods of the same service should not run
+					// on the same worker node. Of this is not possible they
+					// will still be created on the same worker node
+					Affinity: affinity.DistributePods(
+						common.AppSelector,
+						[]string{
+							labels[common.AppSelector],
+						},
+						corev1.LabelHostname,
+					),
 				},
 			},
 		},
 	}
 
-	deployment.Spec.Template.Spec.Volumes = append(watcher.GetVolumes(
+	statefulSet.Spec.Template.Spec.Volumes = append(watcher.GetVolumes(
 		instance.Name,
 		[]string{}),
 		apiVolumes...)
 
 	if instance.Spec.NodeSelector != nil {
-		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
+		statefulSet.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
-	return deployment, nil
+	return statefulSet, nil
 }

templates/watcher/config/watcher-api-config.json

@@ -30,29 +30,34 @@
     {
       "source": "/var/lib/config-data/default/10-watcher-wsgi-main.conf",
       "dest": "/etc/httpd/conf.d/10-watcher-wsgi-main.conf",
-      "owner": "root",
-      "perm": "0640",
+      "owner": "apache",
+      "perm": "0644",
       "optional": true
     },
     {
       "source": "/var/lib/config-data/default/httpd.conf",
       "dest": "/etc/httpd/conf/httpd.conf",
-      "owner": "root",
-      "perm": "0640",
+      "owner": "apache",
+      "perm": "0644",
       "optional": true
     },
 	{
       "source": "/var/lib/config-data/default/ssl.conf",
       "dest": "/etc/httpd/conf.d/ssl.conf",
-      "owner": "root",
-      "perm": "0640"
+      "owner": "apache",
+      "perm": "0444"
     }
   ],
   "permissions": [
     {
       "path": "/var/log/watcher",
       "owner": "watcher:watcher",
       "recurse": true
+    },
+    {
+        "path": "/etc/httpd/run/",
+        "owner": "watcher:apache",
+        "recurse": true
     }
   ]
 }

tests/functional/watcher_controller_test.go

@@ -267,11 +267,12 @@ var _ = Describe("Watcher controller", func() {
 			// We validate the full Watcher CR readiness status here
 			// DB Ready
 
+			// Simulate WatcherAPI deployment
+			th.SimulateStatefulSetReplicaReady(watcherTest.WatcherAPIStatefulSet)
+
 			// Simulate KeystoneEndpoint success
 			keystone.SimulateKeystoneEndpointReady(watcherTest.WatcherKeystoneEndpointName)
 
-			// Simulate WatcherAPI deployment
-			th.SimulateDeploymentReplicaReady(watcherTest.WatcherAPIDeployment)
 			th.ExpectCondition(
 				watcherTest.Instance,
 				ConditionGetterFunc(WatcherConditionGetter),
@@ -372,8 +373,8 @@ var _ = Describe("Watcher controller", func() {
 			Expect(WatcherAPI.Spec.CustomServiceConfig).To(Equal(""))
 			Expect(WatcherAPI.Spec.PrometheusSecret).Should(Equal("metric-storage-prometheus-config"))
 
-			// Assert that the watcher deployment is created
-			deployment := th.GetDeployment(watcherTest.WatcherAPIDeployment)
+			// Assert that the watcher statefulset is created
+			deployment := th.GetStatefulSet(watcherTest.WatcherAPIStatefulSet)
 			Expect(deployment.Spec.Template.Spec.ServiceAccountName).To(Equal("watcher-watcher"))
 			Expect(int(*deployment.Spec.Replicas)).To(Equal(1))
 			Expect(deployment.Spec.Template.Spec.Volumes).To(HaveLen(3))
@@ -677,12 +678,12 @@ var _ = Describe("Watcher controller", func() {
 			// Simulate dbsync success
 			th.SimulateJobSuccess(watcherTest.WatcherDBSync)
 
+			// Simulate WatcherAPI deployment
+			th.SimulateStatefulSetReplicaReady(watcherTest.WatcherAPIStatefulSet)
+
 			// Simulate KeystoneEndpoint success
 			keystone.SimulateKeystoneEndpointReady(watcherTest.WatcherKeystoneEndpointName)
 
-			// Simulate WatcherAPI deployment
-			th.SimulateDeploymentReplicaReady(watcherTest.WatcherAPIDeployment)
-
 			// We validate the full Watcher CR readiness status here
 			// DB Ready
 			th.ExpectCondition(
@@ -811,7 +812,7 @@ var _ = Describe("Watcher controller", func() {
 			Expect(WatcherAPI.Spec.PrometheusSecret).Should(Equal("custom-prometheus-config"))
 
 			// Assert that the watcher deployment is created
-			deployment := th.GetDeployment(watcherTest.WatcherAPIDeployment)
+			deployment := th.GetStatefulSet(watcherTest.WatcherAPIStatefulSet)
 			Expect(deployment.Spec.Template.Spec.ServiceAccountName).To(Equal("watcher-watcher"))
 			Expect(int(*deployment.Spec.Replicas)).To(Equal(2))
 			Expect(deployment.Spec.Template.Spec.Volumes).To(HaveLen(5))