Error reporting in DeploymentTarget/DeploymentTargetClaim.

Author: Jayendra Parsai

appstudio-controller/controllers/appstudio.redhat.com/deploymenttarget_controller.go

@@ -379,7 +379,7 @@ func (r *DeploymentTargetReconciler) findDeploymentTargetsForSpaceRequests(sr cl
 }
 
 // updateStatusConditionOfDeploymentTarget calls SetCondition() with DeploymentTarget conditions
-func updateStatusConditionOfDeploymentTarget(ctx context.Context, client client.Client,
+func updateStatusConditionOfDeploymentTarget(ctx context.Context, k8sClient client.Client,
 	message string, deploymentTarget *applicationv1alpha1.DeploymentTarget, conditionType string,
 	status metav1.ConditionStatus, reason string, log logr.Logger) error {
 
@@ -390,12 +390,19 @@ func updateStatusConditionOfDeploymentTarget(ctx context.Context, client client.
 		Reason:  reason,
 	}
 
-	changed, newConditions := insertOrUpdateConditionsInSlice(newCondition, deploymentTarget.Status.Conditions)
+	dt := deploymentTarget
+
+	if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dt), dt); err != nil {
+		log.Error(err, "unable to fetch deploymentTargetClaim.")
+		return nil
+	}
+
+	changed, newConditions := insertOrUpdateConditionsInSlice(newCondition, dt.Status.Conditions)
 
 	if changed {
-		deploymentTarget.Status.Conditions = newConditions
+		dt.Status.Conditions = newConditions
 
-		if err := client.Status().Update(ctx, deploymentTarget); err != nil {
+		if err := k8sClient.Status().Update(ctx, dt); err != nil {
 			log.Error(err, "unable to update deploymentTarget status condition.")
 			return err
 		}

appstudio-controller/controllers/appstudio.redhat.com/deploymenttargetclaim_controller.go

@@ -956,17 +956,19 @@ func updateStatusConditionOfDeploymentTargetClaim(ctx context.Context, k8sClient
 		Reason:  reason,
 	}
 
-	if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(deploymentTargetClaim), deploymentTargetClaim); err != nil {
+	dtc := deploymentTargetClaim
+
+	if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dtc), dtc); err != nil {
 		log.Error(err, "unable to fetch deploymentTargetClaim.")
 		return nil
 	}
 
-	changed, newConditions := insertOrUpdateConditionsInSlice(newCondition, deploymentTargetClaim.Status.Conditions)
+	changed, newConditions := insertOrUpdateConditionsInSlice(newCondition, dtc.Status.Conditions)
 
 	if changed {
-		deploymentTargetClaim.Status.Conditions = newConditions
+		dtc.Status.Conditions = newConditions
 
-		if err := k8sClient.Status().Update(ctx, deploymentTargetClaim); err != nil {
+		if err := k8sClient.Status().Update(ctx, dtc); err != nil {
 			log.Error(err, "unable to update deploymentTargetClaim status condition.")
 			return err
 		}

tests-e2e/appstudio/deploymenttargetclaim_binder_test.go

@@ -109,6 +109,14 @@ var _ = Describe("DeploymentTargetClaim Binding controller tests", func() {
 
 			Eventually(dt, "5m", "1s").Should(
 				dtfixture.HasStatusPhase(appstudiosharedv1.DeploymentTargetPhase_Bound))
+
+			Eventually(dtc, "3m", "1s").Should(dtcfixture.HaveDeploymentTargetClaimCondition(
+				metav1.Condition{
+					Type:    appstudiocontrollers.DeploymentTargetConditionTypeErrorOccurred,
+					Message: "",
+					Status:  metav1.ConditionTrue,
+					Reason:  appstudiocontrollers.DeploymentTargetReasonBound,
+				}))
 		})
 
 		It("should handle a DTC that targets a user created DT", func() {
@@ -137,6 +145,14 @@ var _ = Describe("DeploymentTargetClaim Binding controller tests", func() {
 				dtfixture.HasStatusPhase(appstudiosharedv1.DeploymentTargetPhase_Bound))
 
 			Eventually(&dt).Should(k8s.HasFinalizers([]string{appstudiocontrollers.FinalizerDT}, k8sClient))
+
+			Eventually(dtc, "3m", "1s").Should(dtcfixture.HaveDeploymentTargetClaimCondition(
+				metav1.Condition{
+					Type:    appstudiocontrollers.DeploymentTargetConditionTypeErrorOccurred,
+					Message: "",
+					Status:  metav1.ConditionTrue,
+					Reason:  appstudiocontrollers.DeploymentTargetReasonBound,
+				}))
 		})
 
 		It("should bind with a best match DT in the absence of provisioner/user created DT", func() {
@@ -204,6 +220,14 @@ var _ = Describe("DeploymentTargetClaim Binding controller tests", func() {
 
 			Eventually(dt, "5m", "1s").Should(
 				dtfixture.HasStatusPhase(appstudiosharedv1.DeploymentTargetPhase_Bound))
+
+			Eventually(dtc, "3m", "1s").Should(dtcfixture.HaveDeploymentTargetClaimCondition(
+				metav1.Condition{
+					Type:    appstudiocontrollers.DeploymentTargetConditionTypeErrorOccurred,
+					Message: "",
+					Status:  metav1.ConditionTrue,
+					Reason:  appstudiocontrollers.DeploymentTargetReasonBound,
+				}))
 		})
 
 		It("should handle deletion of DTC and release binded DT", func() {

tests-e2e/fixture/deploymenttarget/fixture.go

@@ -9,6 +9,7 @@ import (
 	appstudiosharedv1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
 	"github.com/redhat-appstudio/managed-gitops/tests-e2e/fixture"
 	k8sFixture "github.com/redhat-appstudio/managed-gitops/tests-e2e/fixture/k8s"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -38,3 +39,35 @@ func HasStatusPhase(phase appstudiosharedv1.DeploymentTargetPhase) matcher.Gomeg
 		return true
 	}, BeTrue())
 }
+
+func HaveDeploymentTargetCondition(expected metav1.Condition) matcher.GomegaMatcher {
+	return WithTransform(func(dt appstudiosharedv1.DeploymentTarget) bool {
+
+		config, err := fixture.GetE2ETestUserWorkspaceKubeConfig()
+		Expect(err).ToNot(HaveOccurred())
+
+		k8sClient, err := fixture.GetKubeClient(config)
+		if err != nil {
+			fmt.Println(k8sFixture.K8sClientError, err)
+			return false
+		}
+
+		err = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(&dt), &dt)
+		if err != nil {
+			fmt.Println(k8sFixture.K8sClientError, err)
+			return false
+		}
+
+		if len(dt.Status.Conditions) == 0 {
+			fmt.Println("HaveDeploymentTargetCondition: DeploymentTargetClaimCondition is nil")
+			return false
+		}
+		actual := dt.Status.Conditions[0]
+		fmt.Println("HaveDeploymentTargetCondition:", "expected: ", expected, "actual: ", actual)
+		return actual.Type == expected.Type &&
+			actual.Status == expected.Status &&
+			actual.Reason == expected.Reason &&
+			actual.Message == expected.Message
+
+	}, BeTrue())
+}