No Notification for "Completed" and without controller Pods

controllers/idler/idler_controller.go

@@ -132,7 +132,13 @@ func (r *Reconciler) ensureIdling(logger logr.Logger, idler *toolchainv1alpha1.I
 			// Already tracking this pod. Check the timeout.
 			if time.Now().After(trackedPod.StartTime.Add(time.Duration(idler.Spec.TimeoutSeconds) * time.Second)) {
 				podLogger.Info("Pod running for too long. Killing the pod.", "start_time", trackedPod.StartTime.Format("2006-01-02T15:04:05Z"), "timeout_seconds", idler.Spec.TimeoutSeconds)
-
+				var podreason string
+				podcondition := pod.Status.Conditions
+				for _, podtype := range podcondition {
+					if podtype.Type == "Ready" {
+						podreason = podtype.Reason
+					}
+				}
 				// Check if it belongs to a controller (Deployment, DeploymentConfig, etc) and scale it down to zero.
 				appType, appName, deletedByController, err := r.scaleControllerToZero(podLogger, pod.ObjectMeta)
 				if err != nil {
@@ -150,13 +156,15 @@ func (r *Reconciler) ensureIdling(logger logr.Logger, idler *toolchainv1alpha1.I
 					appName = pod.Name
 					appType = "Pod"
 				}
-
-				// By now either a pod has been deleted or scaled to zero by controller, idler Triggered notification should be sent
-				if err := r.createNotification(logger, idler, appName, appType); err != nil {
-					logger.Error(err, "failed to create Notification")
-					if err = r.setStatusIdlerNotificationCreationFailed(idler, err.Error()); err != nil {
-						logger.Error(err, "failed to set status IdlerNotificationCreationFailed")
-					} // not returning error to continue tracking remaining pods
+				// Do not send notification if Pod Not managed by a controller and is in completed state
+				if podreason != "PodCompleted" && !deletedByController {
+					// By now either a pod has been deleted or scaled to zero by controller, idler Triggered notification should be sent
+					if err := r.createNotification(logger, idler, appName, appType); err != nil {
+						logger.Error(err, "failed to create Notification")
+						if err = r.setStatusIdlerNotificationCreationFailed(idler, err.Error()); err != nil {
+							logger.Error(err, "failed to set status IdlerNotificationCreationFailed")
+						} // not returning error to continue tracking remaining pods
+					}
 				}
 
 			} else {

controllers/idler/idler_controller_test.go

@@ -686,22 +686,23 @@ func TestAppNameTypeForInidividualPods(t *testing.T) {
 	//given
 	idler := &toolchainv1alpha1.Idler{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "alex-stage",
+			Name: "feny-stage",
 			Labels: map[string]string{
-				toolchainv1alpha1.SpaceLabelKey: "alex",
+				toolchainv1alpha1.SpaceLabelKey: "feny",
 			},
 		},
 		Spec: toolchainv1alpha1.IdlerSpec{TimeoutSeconds: 60},
 	}
+	namespaces := []string{"dev", "stage"}
+	usernames := []string{"feny"}
+	nsTmplSet := newNSTmplSet(test.MemberOperatorNs, "feny", "advanced", "abcde11", namespaces, usernames)
+	mur := newMUR("feny")
 
 	t.Run("Test AppName/Type in notification", func(t *testing.T) {
-		namespaces := []string{"dev", "stage"}
-		usernames := []string{"alex"}
-		nsTmplSet := newNSTmplSet(test.MemberOperatorNs, "alex", "advanced", "abcde11", namespaces, usernames)
-		mur := newMUR("alex")
 		reconciler, req, cl, _ := prepareReconcile(t, idler.Name, getHostCluster, idler, nsTmplSet, mur)
 		idlerTimeoutPlusOneSecondAgo := time.Now().Add(-time.Duration(idler.Spec.TimeoutSeconds+1) * time.Second)
 		p := preparePayloadsSinglePod(t, reconciler, idler.Name, "todelete-", idlerTimeoutPlusOneSecondAgo).standalonePods[0]
+
 		// first reconcile to track pods
 		res, err := reconciler.Reconcile(context.TODO(), req)
 		assert.NoError(t, err)
@@ -718,16 +719,43 @@ func TestAppNameTypeForInidividualPods(t *testing.T) {
 		notification := &toolchainv1alpha1.Notification{}
 		err = hostCl.Client.Get(context.TODO(), types.NamespacedName{
 			Namespace: test.HostOperatorNs,
-			Name:      "alex-stage-idled",
+			Name:      "feny-stage-idled",
 		}, notification)
 		require.NoError(t, err)
-		require.Equal(t, "alex@test.com", notification.Spec.Recipient)
+		require.Equal(t, "feny@test.com", notification.Spec.Recipient)
 		require.Equal(t, "idled", notification.Labels[toolchainv1alpha1.NotificationTypeLabelKey])
 		require.Equal(t, "Pod", notification.Spec.Context["AppType"])
 		require.Equal(t, p.Name, notification.Spec.Context["AppName"])
 
 	})
 
+	t.Run("Test No notification sent for no controller and completed status pod", func(t *testing.T) {
+		reconciler, req, cl, _ := prepareReconcile(t, idler.Name, getHostCluster, idler, nsTmplSet, mur)
+		idlerTimeoutPlusOneSecondAgo := time.Now().Add(-time.Duration(idler.Spec.TimeoutSeconds+1) * time.Second)
+		pcond := corev1.PodCondition{Type: "Ready", Reason: "PodCompleted"}
+		preparePayloadsSinglePod(t, reconciler, idler.Name, "todelete-", idlerTimeoutPlusOneSecondAgo, pcond)
+		// first reconcile to track pods
+		res, err := reconciler.Reconcile(context.TODO(), req)
+		assert.NoError(t, err)
+		assert.True(t, res.Requeue)
+
+		// second reconcile should delete pods and create notification
+		res, err = reconciler.Reconcile(context.TODO(), req)
+		//then
+		assert.NoError(t, err)
+		memberoperatortest.AssertThatIdler(t, idler.Name, cl).
+			HasConditions(memberoperatortest.Running())
+		//check the notification is not created
+		hostCl, _ := reconciler.GetHostCluster()
+		notification := &toolchainv1alpha1.Notification{}
+		err = hostCl.Client.Get(context.TODO(), types.NamespacedName{
+			Namespace: test.HostOperatorNs,
+			Name:      "feny-stage-idled",
+		}, notification)
+		require.EqualError(t, err, "notifications.toolchain.dev.openshift.com \"feny-stage-idled\" not found")
+
+	})
+
 }
 func TestCreateNotification(t *testing.T) {
 	idler := &toolchainv1alpha1.Idler{
@@ -1135,15 +1163,15 @@ func preparePayloads(t *testing.T, r *Reconciler, namespace, namePrefix string,
 	}
 }
 
-func preparePayloadsSinglePod(t *testing.T, r *Reconciler, namespace, namePrefix string, startTime time.Time) payloads {
+func preparePayloadsSinglePod(t *testing.T, r *Reconciler, namespace, namePrefix string, startTime time.Time, conditions ...corev1.PodCondition) payloads {
 	sTime := metav1.NewTime(startTime)
 
 	// Pods with no owner.
 	standalonePods := make([]*corev1.Pod, 0, 1)
 	for i := 0; i < 1; i++ {
 		pod := &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("%s%s-pod-%d", namePrefix, namespace, i), Namespace: namespace},
-			Status:     corev1.PodStatus{StartTime: &sTime},
+			Status:     corev1.PodStatus{StartTime: &sTime, Conditions: conditions},
 		}
 		standalonePods = append(standalonePods, pod)
 		err := r.AllNamespacesClient.Create(context.TODO(), pod)