fix(operator): Changed checks on pod owner replicas (#412)

Signed-off-by: RealAnna <anna.reale@dynatrace.com>

kuttl-test.yaml

@@ -1,8 +1,6 @@
 apiVersion: kuttl.dev/v1beta1
 kind: TestSuite
 crdDir: ./operator/config/crd/bases
-commands:
-    - command: kubectl apply -f ./test/.build/install.yaml --wait
 testDirs:
   - ./test/integration/
 timeout: 150

operator/api/v1alpha1/keptnworkload_types.go

@@ -71,6 +71,7 @@ type KeptnWorkloadList struct {
 type ResourceReference struct {
 	UID  types.UID `json:"uid"`
 	Kind string    `json:"kind"`
+	Name string    `json:"name"`
 }
 
 func init() {

operator/config/crd/bases/lifecycle.keptn.sh_keptnworkloadinstances.yaml

@@ -95,6 +95,8 @@ spec:
                 properties:
                   kind:
                     type: string
+                  name:
+                    type: string
                   uid:
                     description: UID is a type that holds unique ID values, including
                       UUIDs.  Because we don't ONLY use UUIDs, this is an alias to
@@ -103,6 +105,7 @@ spec:
                     type: string
                 required:
                 - kind
+                - name
                 - uid
                 type: object
               traceId:

operator/config/crd/bases/lifecycle.keptn.sh_keptnworkloads.yaml

@@ -64,6 +64,8 @@ spec:
                 properties:
                   kind:
                     type: string
+                  name:
+                    type: string
                   uid:
                     description: UID is a type that holds unique ID values, including
                       UUIDs.  Because we don't ONLY use UUIDs, this is an alias to
@@ -72,6 +74,7 @@ spec:
                     type: string
                 required:
                 - kind
+                - name
                 - uid
                 type: object
               version:

operator/controllers/keptnworkloadinstance/controller_test.go

@@ -5,13 +5,55 @@ import (
 	klcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1"
 	"github.com/stretchr/testify/require"
 	testrequire "github.com/stretchr/testify/require"
+	appsv1 "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"testing"
 )
 
+func TestKeptnWorkloadInstanceReconciler_isReferencedWorkloadRunning(t *testing.T) {
+
+	rep := int32(1)
+	replicasetFail := makeReplicaSet("myrep", "default", &rep, 0)
+	statefulsetFail := makeStatefulSet("mystat", "default", &rep, 0)
+
+	r := &KeptnWorkloadInstanceReconciler{
+		Client: fake.NewClientBuilder().WithObjects(replicasetFail, statefulsetFail).Build(),
+	}
+	isOwnerRunning, err := r.isReferencedWorkloadRunning(context.TODO(), klcv1alpha1.ResourceReference{UID: "myrep", Name: "myrep", Kind: "ReplicaSet"}, "default")
+	testrequire.Nil(t, err)
+	if isOwnerRunning {
+		t.Errorf("Should fail!")
+	}
+
+	isOwnerRunning, err = r.isReferencedWorkloadRunning(context.TODO(), klcv1alpha1.ResourceReference{UID: "mystat", Name: "mystat", Kind: "StatefulSet"}, "default")
+	testrequire.Nil(t, err)
+	if isOwnerRunning {
+		t.Errorf("Should fail!")
+	}
+
+	replicasetPass := makeReplicaSet("myrep", "default", &rep, 1)
+	statefulsetPass := makeStatefulSet("mystat", "default", &rep, 1)
+
+	r2 := &KeptnWorkloadInstanceReconciler{
+		Client: fake.NewClientBuilder().WithObjects(replicasetPass, statefulsetPass).Build(),
+	}
+	isOwnerRunning, err = r2.isReferencedWorkloadRunning(context.TODO(), klcv1alpha1.ResourceReference{UID: "myrep", Name: "myrep", Kind: "ReplicaSet"}, "default")
+	testrequire.Nil(t, err)
+	if !isOwnerRunning {
+		t.Errorf("Should find a replica owner!")
+	}
+
+	isOwnerRunning, err = r2.isReferencedWorkloadRunning(context.TODO(), klcv1alpha1.ResourceReference{UID: "mystat", Name: "mystat", Kind: "StatefulSet"}, "default")
+	testrequire.Nil(t, err)
+	if !isOwnerRunning {
+		t.Errorf("Should find a stateful set owner!")
+	}
+
+}
+
 func TestKeptnWorkloadInstanceReconciler_IsPodRunning(t *testing.T) {
 	p1 := makeNominatedPod("pod1", "node1", v1.PodRunning)
 	p2 := makeNominatedPod("pod2", "node1", v1.PodPending)
@@ -20,7 +62,7 @@ func TestKeptnWorkloadInstanceReconciler_IsPodRunning(t *testing.T) {
 	r := &KeptnWorkloadInstanceReconciler{
 		Client: fake.NewClientBuilder().WithLists(podList).Build(),
 	}
-	isPodRunning, err := r.isPodRunning(context.TODO(), klcv1alpha1.ResourceReference{UID: types.UID("pod1")}, "node1")
+	isPodRunning, err := r.isPodRunning(context.TODO(), klcv1alpha1.ResourceReference{UID: "pod1"}, "node1")
 	testrequire.Nil(t, err)
 	if !isPodRunning {
 		t.Errorf("Wrong!")
@@ -29,7 +71,7 @@ func TestKeptnWorkloadInstanceReconciler_IsPodRunning(t *testing.T) {
 	r2 := &KeptnWorkloadInstanceReconciler{
 		Client: fake.NewClientBuilder().WithLists(podList2).Build(),
 	}
-	isPodRunning, err = r2.isPodRunning(context.TODO(), klcv1alpha1.ResourceReference{UID: types.UID("pod1")}, "node1")
+	isPodRunning, err = r2.isPodRunning(context.TODO(), klcv1alpha1.ResourceReference{UID: "pod1"}, "node1")
 	testrequire.Nil(t, err)
 	if isPodRunning {
 		t.Errorf("Wrong!")
@@ -51,6 +93,48 @@ func makeNominatedPod(podName string, nodeName string, phase v1.PodPhase) v1.Pod
 	}
 }
 
+func makeReplicaSet(name string, namespace string, wanted *int32, available int32) *appsv1.ReplicaSet {
+
+	return &appsv1.ReplicaSet{
+		TypeMeta: metav1.TypeMeta{
+			Kind: "ReplicaSet",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+			UID:       types.UID(name),
+		},
+		Spec: appsv1.ReplicaSetSpec{
+			Replicas: wanted,
+		},
+		Status: appsv1.ReplicaSetStatus{
+			AvailableReplicas: available,
+		},
+	}
+
+}
+
+func makeStatefulSet(name string, namespace string, wanted *int32, available int32) *appsv1.StatefulSet {
+
+	return &appsv1.StatefulSet{
+		TypeMeta: metav1.TypeMeta{
+			Kind: "StatefulSet",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+			UID:       types.UID(name),
+		},
+		Spec: appsv1.StatefulSetSpec{
+			Replicas: wanted,
+		},
+		Status: appsv1.StatefulSetStatus{
+			AvailableReplicas: available,
+		},
+	}
+
+}
+
 func Test_getLatestAppVersion(t *testing.T) {
 	type args struct {
 		apps *klcv1alpha1.KeptnAppVersionList

operator/controllers/keptnworkloadinstance/reconcile_deploymentstate.go

@@ -2,12 +2,10 @@ package keptnworkloadinstance
 
 import (
 	"context"
-
 	klcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1"
 	"github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -24,7 +22,7 @@ func (r *KeptnWorkloadInstanceReconciler) reconcileDeployment(ctx context.Contex
 			workloadInstance.Status.DeploymentStatus = common.StateProgressing
 		}
 	} else {
-		isReplicaRunning, err := r.isReplicaSetRunning(ctx, workloadInstance.Spec.ResourceReference, workloadInstance.Namespace)
+		isReplicaRunning, err := r.isReferencedWorkloadRunning(ctx, workloadInstance.Spec.ResourceReference, workloadInstance.Namespace)
 		if err != nil {
 			return common.StateUnknown, err
 		}
@@ -42,24 +40,30 @@ func (r *KeptnWorkloadInstanceReconciler) reconcileDeployment(ctx context.Contex
 	return workloadInstance.Status.DeploymentStatus, nil
 }
 
-func (r *KeptnWorkloadInstanceReconciler) isReplicaSetRunning(ctx context.Context, resource klcv1alpha1.ResourceReference, namespace string) (bool, error) {
-	replica := &appsv1.ReplicaSetList{}
-	if err := r.Client.List(ctx, replica, client.InNamespace(namespace)); err != nil {
-		return false, err
-	}
-	for _, re := range replica.Items {
-		if re.UID == resource.UID {
-			replicas, err := r.getDesiredReplicas(ctx, re.OwnerReferences[0], namespace)
-			if err != nil {
-				return false, err
-			}
-			if re.Status.ReadyReplicas == replicas {
-				return true, nil
-			}
-			return false, nil
+func (r *KeptnWorkloadInstanceReconciler) isReferencedWorkloadRunning(ctx context.Context, resource klcv1alpha1.ResourceReference, namespace string) (bool, error) {
+
+	var replicas *int32
+	var desiredReplicas int32
+	switch resource.Kind {
+	case "ReplicaSet":
+		rep := appsv1.ReplicaSet{}
+		err := r.Client.Get(ctx, types.NamespacedName{Name: resource.Name, Namespace: namespace}, &rep)
+		if err != nil {
+			return false, err
 		}
+		replicas = rep.Spec.Replicas
+		desiredReplicas = rep.Status.AvailableReplicas
+	case "StatefulSet":
+		sts := appsv1.StatefulSet{}
+		err := r.Client.Get(ctx, types.NamespacedName{Name: resource.Name, Namespace: namespace}, &sts)
+		if err != nil {
+			return false, err
+		}
+		replicas = sts.Spec.Replicas
+		desiredReplicas = sts.Status.AvailableReplicas
 	}
-	return false, nil
+
+	return *replicas == desiredReplicas, nil
 
 }
 
@@ -78,26 +82,3 @@ func (r *KeptnWorkloadInstanceReconciler) isPodRunning(ctx context.Context, reso
 	}
 	return false, nil
 }
-
-func (r *KeptnWorkloadInstanceReconciler) getDesiredReplicas(ctx context.Context, reference v1.OwnerReference, namespace string) (int32, error) {
-	var replicas *int32
-	switch reference.Kind {
-	case "Deployment":
-		dep := appsv1.Deployment{}
-		err := r.Client.Get(ctx, types.NamespacedName{Name: reference.Name, Namespace: namespace}, &dep)
-		if err != nil {
-			return 0, err
-		}
-		replicas = dep.Spec.Replicas
-	case "StatefulSet":
-		sts := appsv1.StatefulSet{}
-		err := r.Client.Get(ctx, types.NamespacedName{Name: reference.Name, Namespace: namespace}, &sts)
-		if err != nil {
-			return 0, err
-		}
-		replicas = sts.Spec.Replicas
-	}
-
-	return *replicas, nil
-
-}

operator/webhooks/pod_mutating_webhook.go

@@ -429,7 +429,7 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P
 		Spec: klcv1alpha1.KeptnWorkloadSpec{
 			AppName:                   applicationName,
 			Version:                   version,
-			ResourceReference:         klcv1alpha1.ResourceReference{UID: ownerRef.UID, Kind: ownerRef.Kind},
+			ResourceReference:         klcv1alpha1.ResourceReference{UID: ownerRef.UID, Kind: ownerRef.Kind, Name: ownerRef.Name},
 			PreDeploymentTasks:        preDeploymentTasks,
 			PostDeploymentTasks:       postDeploymentTasks,
 			PreDeploymentEvaluations:  preDeploymentEvaluation,

scheduler/test/e2e/fake/v1alpha1/keptnworkload_types.go

@@ -68,6 +68,7 @@ type KeptnWorkloadList struct {
 type ResourceReference struct {
 	UID  types.UID `json:"uid"`
 	Kind string    `json:"kind"`
+	Name string    `json:"name"`
 }
 
 func init() {

scheduler/test/e2e/scheduler_test.go

@@ -146,7 +146,12 @@ func initWorkloadInstance() *testv1alpha1.KeptnWorkloadInstance {
 			APIVersion: "lifecycle.keptn.sh/v1alpha1",
 		},
 		ObjectMeta: metav1.ObjectMeta{Name: "myapp-myworkload-1.0.0", Namespace: "default"},
-		Status:     testv1alpha1.KeptnWorkloadInstanceStatus{PreDeploymentEvaluationStatus: "Succeeded"},
+		Spec: testv1alpha1.KeptnWorkloadInstanceSpec{
+			KeptnWorkloadSpec: testv1alpha1.KeptnWorkloadSpec{
+				ResourceReference: testv1alpha1.ResourceReference{Name: "myfakeres"},
+			},
+		},
+		Status: testv1alpha1.KeptnWorkloadInstanceStatus{},
 	}
 
 	return &fakeInstance

test/integration/simple-deployment-annotated/00-assert.yaml

@@ -4,6 +4,5 @@ metadata:
   labels:
     app: test
   name: test
-  namespace: test-annotation
 status:
-  readyReplicas: 1
+  readyReplicas: 2

test/integration/simple-deployment-annotated/00-install.yaml

@@ -5,7 +5,6 @@ apiVersion: lifecycle.keptn.sh/v1alpha1
 kind: KeptnTaskDefinition
 metadata:
   name: pre-deployment-hello
-  namespace: test-annotation
 spec:
   function:
     inline:
@@ -20,14 +19,13 @@ metadata:
   labels:
     app: test
   name: test
-  namespace: test-annotation
   annotations:
     keptn.sh/workload: waiter
     keptn.sh/version: "0.4"
     keptn.sh/pre-deployment-tasks: pre-deployment-hello
     keptn.sh/post-deployment-tasks: pre-deployment-hello
 spec:
-  replicas: 1
+  replicas: 2
   selector:
     matchLabels:
       app: test

test/integration/simple-deployment-annotated/00-teststep.yaml

@@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - script: kubectl annotate ns $NAMESPACE keptn.sh/lifecycle-toolkit='enabled'

test/integration/simple-deployment-annotated/01-assert.yaml

@@ -2,15 +2,12 @@ apiVersion: lifecycle.keptn.sh/v1alpha1
 kind: KeptnWorkload
 metadata:
   name: waiter-waiter
-  namespace: test-annotation
-
 ---
 
 apiVersion: lifecycle.keptn.sh/v1alpha1
 kind: KeptnWorkloadInstance
 metadata:
   name: waiter-waiter-0.4
-  namespace: test-annotation
 status:
   currentPhase: Completed
   deploymentStatus: Succeeded

test/integration/simple-deployment/00-assert.yaml

@@ -4,6 +4,5 @@ metadata:
   labels:
     app: test
   name: test
-  namespace: test
 status:
   readyReplicas: 1

test/integration/simple-deployment/00-install.yaml

@@ -1,11 +1,9 @@
 
----
 
 apiVersion: lifecycle.keptn.sh/v1alpha1
 kind: KeptnTaskDefinition
 metadata:
   name: pre-deployment-hello
-  namespace: test
 spec:
   function:
     inline:
@@ -20,7 +18,6 @@ metadata:
   labels:
     app: test
   name: test
-  namespace: test
 spec:
   replicas: 1
   selector: