[#344] add valid status condition, reflect reserved label as failure …

…reason, true otherwise

api/v1beta1/activemqartemis_types.go

@@ -421,4 +421,8 @@ const (
 	DeployedConditionNotReadyReason  = "PodsNotReady"
 	DeployedConditionZeroSizeReason  = "ZeroSizeDeployment"
 	DeployedConditionZeroSizeMessage = "Pods not scheduled. Deployment size is 0"
+
+	ValidConditionType                      = "Valid"
+	ValidConditionSuccessReason             = "ValidationSucceeded"
+	ValidConditionFailedReservedLabelReason = "ReservedLabelReference"
 )

controllers/activemqartemis_controller.go

@@ -156,7 +156,6 @@ func (r *ActiveMQArtemisReconciler) Reconcile(ctx context.Context, request ctrl.
 	if err == nil {
 		return ctrl.Result{}, err
 	} else {
-		reqLogger.Error(err, "ActiveMQArtemis Controller Reconcile, requeuing request", "Request Namespace", request.Namespace, "Request Name", request.Name)
 		return ctrl.Result{RequeueAfter: common.GetReconcileResyncPeriod()}, err
 	}
 }

controllers/activemqartemis_controller_test.go

@@ -1319,6 +1319,7 @@ var _ = Describe("artemis controller", func() {
 					g.Expect(len(createdCrd.Status.PodStatus.Ready)).Should(BeEquivalentTo(1))
 					g.Expect(meta.IsStatusConditionTrue(createdCrd.Status.Conditions, brokerv1beta1.DeployedConditionType)).Should(BeTrue())
 					g.Expect(meta.IsStatusConditionTrue(createdCrd.Status.Conditions, common.ReadyConditionType)).Should(BeTrue())
+					g.Expect(meta.IsStatusConditionTrue(createdCrd.Status.Conditions, brokerv1beta1.ValidConditionType)).Should(BeTrue())
 				}, timeout*2, interval).Should(Succeed())
 
 				By("applying taints to node")
@@ -2093,6 +2094,7 @@ var _ = Describe("artemis controller", func() {
 					Message: brokerv1beta1.DeployedConditionZeroSizeMessage,
 				})).Should(BeTrue())
 				g.Expect(meta.IsStatusConditionFalse(createdCrd.Status.Conditions, common.ReadyConditionType)).Should(BeTrue())
+				g.Expect(meta.IsStatusConditionTrue(createdCrd.Status.Conditions, brokerv1beta1.ValidConditionType)).Should(BeTrue())
 
 			}, timeout, interval).Should(Succeed())
 
@@ -2654,7 +2656,7 @@ var _ = Describe("artemis controller", func() {
 	})
 
 	Context("Validation", func() {
-		It("Test labels", func() {
+		It("with test labels", func() {
 			ctx := context.Background()
 			var err error
 			var deployedCrdKey types.NamespacedName
@@ -2663,35 +2665,36 @@ var _ = Describe("artemis controller", func() {
 			var deployedResources map[reflect.Type][]client.Object
 			statefulSetType := reflect.TypeOf(appsv1.StatefulSet{})
 
-			By("By creating invalid crd")
+			By("creating invalid crd with reserved label")
 			invalidCrd := generateArtemisSpec(defaultNamespace)
 			invalidCrd.Spec.DeploymentPlan.Labels = map[string]string{"application": "test"}
 			Expect(k8sClient.Create(ctx, &invalidCrd)).Should(Succeed())
 
-			By("By creating valid crd")
+			By("creating valid crd")
 			validCrd := generateArtemisSpec(defaultNamespace)
 			validCrd.Spec.DeploymentPlan.Labels = map[string]string{"test": "test"}
 			Expect(k8sClient.Create(ctx, &validCrd)).Should(Succeed())
 
-			By("Checking valid CR")
+			By("checking valid CR")
 			deployedCrdKey = types.NamespacedName{Name: validCrd.ObjectMeta.Name, Namespace: defaultNamespace}
 			Eventually(func(g Gomega) {
 				g.Expect(k8sClient.Get(ctx, deployedCrdKey, &deployedCrd)).Should(Succeed())
 				g.Expect(deployedCrd.Name).Should(Equal(validCrd.ObjectMeta.Name))
 				g.Expect(len(deployedCrd.Status.PodStatus.Stopped)).Should(Equal(1))
 				g.Expect(deployedCrd.Status.PodStatus.Stopped[0]).Should(Equal(namer.CrToSS(validCrd.Name)))
+				g.Expect(meta.IsStatusConditionTrue(deployedCrd.Status.Conditions, brokerv1beta1.ValidConditionType)).Should(BeTrue())
 			}, timeout, interval).Should(Succeed())
 
-			By("Checking deployed resources of valid CR")
+			By("checking deployed resources of valid CR")
 			deployedResources, err = getDeployedResources(&validCrd, k8sClient)
 			Expect(err).Should(Succeed())
 			Expect(deployedResources).ShouldNot(BeEmpty())
 
-			By("Checking template labels of valid CR")
+			By("checking template labels of valid CR")
 			deployedStatefulSet = deployedResources[statefulSetType][0].(*appsv1.StatefulSet)
 			Expect(deployedStatefulSet.Spec.Template.Labels["test"]).Should(Equal("test"))
 
-			By("Checking invalid CR")
+			By("checking invalid CR")
 			deployedCrdKey = types.NamespacedName{Name: invalidCrd.ObjectMeta.Name, Namespace: defaultNamespace}
 			Eventually(func(g Gomega) {
 				g.Expect(k8sClient.Get(ctx, deployedCrdKey, &deployedCrd)).Should(Succeed())
@@ -2702,7 +2705,16 @@ var _ = Describe("artemis controller", func() {
 			Expect(err).Should(Succeed())
 			Expect(deployedResources).Should(BeEmpty())
 
-			By("By updating invalid crd")
+			By("verify status valid false")
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, deployedCrdKey, &deployedCrd)).Should(Succeed())
+				g.Expect(meta.IsStatusConditionFalse(deployedCrd.Status.Conditions, brokerv1beta1.ValidConditionType)).Should(BeTrue())
+				g.Expect(meta.FindStatusCondition(deployedCrd.Status.Conditions, brokerv1beta1.ValidConditionType).Reason).Should(Equal(brokerv1beta1.ValidConditionFailedReservedLabelReason))
+				g.Expect(meta.FindStatusCondition(deployedCrd.Status.Conditions, brokerv1beta1.ValidConditionType).Message).Should(ContainSubstring("application"))
+				g.Expect(meta.IsStatusConditionFalse(deployedCrd.Status.Conditions, common.ReadyConditionType)).Should(BeTrue())
+			}, timeout, interval).Should(Succeed())
+
+			By("updating invalid crd")
 			invalidCrd.Spec.DeploymentPlan.Labels = map[string]string{"test": "test"}
 			Eventually(func(g Gomega) {
 				g.Expect(k8sClient.Get(ctx, deployedCrdKey, &deployedCrd)).Should(Succeed())
@@ -2711,21 +2723,24 @@ var _ = Describe("artemis controller", func() {
 				Expect(k8sClient.Update(ctx, &deployedCrd)).Should(Succeed())
 			}, timeout, interval).Should(Succeed())
 
-			By("Checking updated invalid CR")
+			By("checking updated invalid CR")
 			deployedCrdKey = types.NamespacedName{Name: invalidCrd.ObjectMeta.Name, Namespace: defaultNamespace}
 			Eventually(func(g Gomega) {
 				g.Expect(k8sClient.Get(ctx, deployedCrdKey, &deployedCrd)).Should(Succeed())
 				g.Expect(deployedCrd.Name).Should(Equal(invalidCrd.ObjectMeta.Name))
 				g.Expect(len(deployedCrd.Status.PodStatus.Stopped)).Should(Equal(1))
 				g.Expect(deployedCrd.Status.PodStatus.Stopped[0]).Should(Equal(namer.CrToSS(invalidCrd.Name)))
+				By("verify status valid true")
+				g.Expect(meta.IsStatusConditionTrue(deployedCrd.Status.Conditions, brokerv1beta1.ValidConditionType)).Should(BeTrue())
+				g.Expect(meta.FindStatusCondition(deployedCrd.Status.Conditions, brokerv1beta1.ValidConditionType).Reason).Should(Equal(brokerv1beta1.ValidConditionSuccessReason))
 			}, timeout, interval).Should(Succeed())
 
-			By("Checking deployed resources of updated invalid CR")
+			By("checking deployed resources of updated invalid CR")
 			deployedResources, err = getDeployedResources(&validCrd, k8sClient)
 			Expect(err).Should(Succeed())
 			Expect(deployedResources).ShouldNot(BeEmpty())
 
-			By("Checking template labels of updated invalid CR")
+			By("checking template labels of updated invalid CR")
 			deployedStatefulSet = deployedResources[statefulSetType][0].(*appsv1.StatefulSet)
 			Expect(deployedStatefulSet.Spec.Template.Labels["test"]).Should(Equal("test"))
 

controllers/activemqartemis_reconciler.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"encoding/hex"
 	"encoding/json"
-	goerrors "errors"
 	"fmt"
 	"hash/adler32"
 	osruntime "runtime"
@@ -1484,12 +1483,25 @@ func (reconciler *ActiveMQArtemisReconcilerImpl) NewPodTemplateSpecForCR(customR
 		for key, value := range customResource.Spec.DeploymentPlan.Labels {
 			reqLogger.V(1).Info("Adding CR Label", "key", key, "value", value)
 			if key == selectors.LabelAppKey || key == selectors.LabelResourceKey {
-				return nil, goerrors.New("Label key not allowed: " + key)
+
+				meta.SetStatusCondition(&customResource.Status.Conditions, metav1.Condition{
+					Type:    brokerv1beta1.ValidConditionType,
+					Status:  metav1.ConditionFalse,
+					Reason:  brokerv1beta1.ValidConditionFailedReservedLabelReason,
+					Message: fmt.Sprintf("'%s' is a reserved label, it is not allowed in Spec.DeploymentPlan.Labels", key),
+				})
+				return nil, fmt.Errorf("label key '%s' not allowed because it is reserved", key)
 			} else {
 				labels[key] = value
 			}
 		}
 	}
+	// validation success
+	meta.SetStatusCondition(&customResource.Status.Conditions, metav1.Condition{
+		Type:   brokerv1beta1.ValidConditionType,
+		Status: metav1.ConditionTrue,
+		Reason: brokerv1beta1.ValidConditionSuccessReason,
+	})
 
 	pts := pods.MakePodTemplateSpec(current, namespacedName, labels)
 	podSpec := &pts.Spec
@@ -2211,6 +2223,7 @@ func UpdatePodStatus(cr *brokerv1beta1.ActiveMQArtemis, client rtclient.Client,
 	reqLogger.V(1).Info("Stopped Count......................", "info:", len(podStatus.Stopped))
 	reqLogger.V(1).Info("Starting Count.....................", "info:", len(podStatus.Starting))
 
+	meta.SetStatusCondition(&cr.Status.Conditions, getValidCondition(cr))
 	meta.SetStatusCondition(&cr.Status.Conditions, getDeploymentCondition(cr, podStatus))
 
 	var err error
@@ -2224,6 +2237,20 @@ func UpdatePodStatus(cr *brokerv1beta1.ActiveMQArtemis, client rtclient.Client,
 	return err
 }
 
+func getValidCondition(cr *brokerv1beta1.ActiveMQArtemis) metav1.Condition {
+	// add valid true if none exists
+	for _, c := range cr.Status.Conditions {
+		if c.Type == brokerv1beta1.ValidConditionType {
+			return c
+		}
+	}
+	return metav1.Condition{
+		Type:   brokerv1beta1.ValidConditionType,
+		Reason: brokerv1beta1.ValidConditionSuccessReason,
+		Status: metav1.ConditionTrue,
+	}
+}
+
 func getDeploymentCondition(cr *brokerv1beta1.ActiveMQArtemis, podStatus olm.DeploymentStatus) metav1.Condition {
 	if cr.Spec.DeploymentPlan.Size == 0 {
 		return metav1.Condition{

pkg/draincontroller/controller.go

@@ -549,7 +549,7 @@ func (c *Controller) getClaims(sts *appsv1.StatefulSet) (claimsGroupedByOrdinal
 	return claimsMap, nil
 }
 
-// create service account, role and rolbe binding for drain pod
+// create service account, role and role binding for drain pod
 func (c *Controller) createDrainRBACResources(namespace string) {
 	dlog.Info("Creating drain pod rbac resources", "namespace", namespace)
 	rbacutil.CreateServiceAccount(DrainServiceAccountName, namespace, c.kubeclientset)