Merge pull request #228 from Josh-Everett/433

feat(STONEINTG-433): cleanup ephemeral envs for some errors

controllers/binding/snapshotenvironmentbinding_adapter.go

@@ -18,6 +18,7 @@ import (
 	"fmt"
 
 	"github.com/redhat-appstudio/operator-goodies/reconciler"
+	"k8s.io/apimachinery/pkg/api/meta"
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	applicationapiv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
@@ -69,6 +70,11 @@ func (a *Adapter) EnsureIntegrationTestPipelineForScenarioExists() (reconciler.O
 		return reconciler.ContinueProcessing()
 	}
 
+	if gitops.HaveBindingsFailed(a.snapshotEnvironmentBinding) {
+		a.logger.Info("The SnapshotEnvrionmentBinding has failed to deploy on ephemeral envrionment and will be deleted later.", "snapshotEnvironmentBinding.Name", a.snapshotEnvironmentBinding.Name)
+		return reconciler.ContinueProcessing()
+	}
+
 	if a.integrationTestScenario != nil {
 		integrationPipelineRun, err := loader.GetLatestPipelineRunForSnapshotAndScenario(a.client, a.context, a.loader, a.snapshot, a.integrationTestScenario)
 		if err != nil {
@@ -99,6 +105,39 @@ func (a *Adapter) EnsureIntegrationTestPipelineForScenarioExists() (reconciler.O
 	return reconciler.ContinueProcessing()
 }
 
+// EnsureEphemeralEnvironmentsCleanedUp will ensure that ephemeral environment(s) associated with the
+// SnapshotEnvironmentBinding are cleaned up.
+func (a *Adapter) EnsureEphemeralEnvironmentsCleanedUp() (reconciler.OperationResult, error) {
+
+	if !gitops.HaveBindingsFailed(a.snapshotEnvironmentBinding) {
+		return reconciler.ContinueProcessing()
+	}
+
+	// mark snapshot as failed
+	snapshotErrorMessage := "Encountered issue deploying snapshot on ephemeral environments: " +
+		meta.FindStatusCondition(a.snapshotEnvironmentBinding.Status.ComponentDeploymentConditions, "ErrorOccurred").Message
+	_, err := gitops.MarkSnapshotAsFailed(a.client, a.context, a.snapshot, snapshotErrorMessage)
+	if err != nil {
+		a.logger.Error(err, "Failed to Update Snapshot status")
+		return reconciler.RequeueOnErrorOrStop(err)
+	}
+
+	deploymentTargetClaim, err := a.loader.GetDeploymentTargetClaimForEnvironment(a.client, a.context, a.environment)
+	if err != nil {
+		a.logger.Error(err, "failed to find deploymentTargetClaim defined in environment %s", a.environment.Name)
+		return reconciler.RequeueOnErrorOrStop(err)
+	}
+
+	err = h.CleanUpEphemeralEnvironments(a.client, &a.logger, a.context, a.environment, deploymentTargetClaim)
+	if err != nil {
+		a.logger.Error(err, "Failed to delete the Ephemeral Environment")
+		return reconciler.RequeueOnErrorOrStop(err)
+	}
+
+	return reconciler.ContinueProcessing()
+
+}
+
 // createIntegrationPipelineRunWithEnvironment creates new integration PipelineRun. The Pipeline information and the parameters to it
 // will be extracted from the given integrationScenario. The integration's Snapshot will also be passed to the integration PipelineRun.
 // If the creation of the PipelineRun is unsuccessful, an error will be returned.

controllers/binding/snapshotenvironmentbinding_adapter_test.go

@@ -14,12 +14,14 @@ limitations under the License.
 package binding
 
 import (
+	"bytes"
 	"fmt"
 	"reflect"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/tonglil/buflogr"
 
 	ctrl "sigs.k8s.io/controller-runtime"
 
@@ -429,4 +431,52 @@ var _ = Describe("Binding Adapter", Ordered, func() {
 		}, time.Second*10).Should(BeTrue())
 	})
 
+	It("ensures ephemeral environment is deleted for the given pipelineRun ", func() {
+		var buf bytes.Buffer
+		log := helpers.IntegrationLogger{Logger: buflogr.NewWithBuffer(&buf)}
+		hasEnv.Spec.Tags = append(hasEnv.Spec.Tags, "ephemeral")
+		hasBinding.Status = applicationapiv1alpha1.SnapshotEnvironmentBindingStatus{
+			ComponentDeploymentConditions: []metav1.Condition{
+				{
+					Reason:             "ErrorOccurred",
+					Status:             "True",
+					Type:               gitops.BindingErrorOccurredStatusConditionType,
+					LastTransitionTime: metav1.Time{Time: time.Now()},
+				},
+			},
+		}
+
+		adapter = NewAdapter(hasBinding, hasSnapshot, hasEnv, hasApp, hasComp, integrationTestScenario, log, loader.NewMockLoader(), k8sClient, ctx)
+		adapter.context = loader.GetMockedContext(ctx, []loader.MockData{
+			{
+				ContextKey: loader.ApplicationContextKey,
+				Resource:   hasApp,
+			},
+			{
+				ContextKey: loader.EnvironmentContextKey,
+				Resource:   hasEnv,
+			},
+			{
+				ContextKey: loader.DeploymentTargetContextKey,
+				Resource:   deploymentTarget,
+			},
+		})
+
+		dtc, _ := adapter.loader.GetDeploymentTargetClaimForEnvironment(k8sClient, adapter.context, hasEnv)
+		Expect(dtc).NotTo(BeNil())
+
+		dt, _ := adapter.loader.GetDeploymentTargetForDeploymentTargetClaim(k8sClient, adapter.context, dtc)
+		Expect(dt).NotTo(BeNil())
+
+		result, err := adapter.EnsureEphemeralEnvironmentsCleanedUp()
+		Expect(!result.CancelRequest && err == nil).To(BeTrue())
+
+		Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeFalse())
+
+		expectedLogEntry := "DeploymentTargetClaim deleted"
+		Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
+
+		expectedLogEntry = "Ephemeral environment and its owning snapshotEnvironmentBinding deleted"
+		Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
+	})
 })

controllers/binding/snapshotenvironmentbinding_controller.go

@@ -30,6 +30,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
 
 // Reconciler reconciles a SnapshotEnvironmentBinding object
@@ -115,6 +116,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	return reconciler.ReconcileHandler([]reconciler.ReconcileOperation{
 		adapter.EnsureIntegrationTestPipelineForScenarioExists,
+		adapter.EnsureEphemeralEnvironmentsCleanedUp,
 	})
 }
 
@@ -199,6 +201,9 @@ func SetupController(manager ctrl.Manager, log *logr.Logger) error {
 // setupControllerWithManager sets up the controller with the Manager which monitors new SnapshotEnvironmentBindings
 func setupControllerWithManager(manager ctrl.Manager, reconciler *Reconciler) error {
 	return ctrl.NewControllerManagedBy(manager).
-		For(&applicationapiv1alpha1.SnapshotEnvironmentBinding{}, builder.WithPredicates(predicates.GenerationUnchangedOnUpdatePredicate{}, gitops.DeploymentSucceededForIntegrationBindingPredicate())).
+		For(&applicationapiv1alpha1.SnapshotEnvironmentBinding{},
+			builder.WithPredicates(predicates.GenerationUnchangedOnUpdatePredicate{})).
+		WithEventFilter(predicate.Or(
+			gitops.DeploymentSucceededForIntegrationBindingPredicate(), gitops.DeploymentFailedForIntegrationBindingPredicate())).
 		Complete(reconciler)
 }

gitops/binding.go

@@ -27,6 +27,10 @@ const (
 	// BindingDeploymentStatusConditionType is the condition type to retrieve from the ComponentDeploymentConditions
 	// in the SnapshotEnvironmentBinding's status to copy into the Release status
 	BindingDeploymentStatusConditionType string = "AllComponentsDeployed"
+
+	// BindingErrorOccurredStatusConditionType is the condition to check for failures within the
+	// SnapshotEnvironmentBindingConditions status
+	BindingErrorOccurredStatusConditionType string = "ErrorOccurred"
 )
 
 // NewSnapshotEnvironmentBinding creates a new SnapshotEnvironmentBinding using the provided info.
@@ -61,6 +65,14 @@ func NewBindingComponents(components []applicationapiv1alpha1.Component) *[]appl
 	return &bindingComponents
 }
 
+func HaveBindingsFailed(snapshotEnvironmentBinding *applicationapiv1alpha1.SnapshotEnvironmentBinding) bool {
+	bindingStatus := meta.FindStatusCondition(snapshotEnvironmentBinding.Status.ComponentDeploymentConditions, BindingErrorOccurredStatusConditionType)
+	if bindingStatus == nil {
+		return false
+	}
+	return bindingStatus.Status == metav1.ConditionTrue
+}
+
 // hasDeploymentSucceeded returns a boolean that is only true if the first passed object
 // is a SnapshotEnvironmentBinding with the componentDeployment status anything other than True and
 // the second passed object is a SnapshotEnvironmentBinding with the componentDeployment status True.
@@ -79,3 +91,20 @@ func hasDeploymentSucceeded(objectOld, objectNew client.Object) bool {
 
 	return (oldCondition == nil || oldCondition.Status != metav1.ConditionTrue) && newCondition.Status == metav1.ConditionTrue
 }
+
+// hasDeploymentFailed returns a boolean that is only true if the first passed object
+// is a SnapshotEnvironmentBinding with the BindingConditions status anything other than True and
+// the second passed object is a SnapshotEnvironmentBinding with the BindingErrorOccurred status True.
+func hasDeploymentFailed(objectOld, objectNew client.Object) bool {
+	var oldCondition, newCondition *metav1.Condition
+	if oldBinding, ok := objectOld.(*applicationapiv1alpha1.SnapshotEnvironmentBinding); ok {
+		oldCondition = meta.FindStatusCondition(oldBinding.Status.BindingConditions, BindingErrorOccurredStatusConditionType)
+	}
+	if newBinding, ok := objectNew.(*applicationapiv1alpha1.SnapshotEnvironmentBinding); ok {
+		newCondition = meta.FindStatusCondition(newBinding.Status.BindingConditions, BindingErrorOccurredStatusConditionType)
+		if newCondition == nil {
+			return false
+		}
+	}
+	return (oldCondition == nil || oldCondition.Status != metav1.ConditionTrue) && newCondition.Status == metav1.ConditionTrue
+}

gitops/predicates.go

@@ -29,3 +29,13 @@ func DeploymentSucceededForIntegrationBindingPredicate() predicate.Predicate {
 		},
 	}
 }
+
+// DeploymentFailedForIntegrationBindingPredicate returns a predicate which filters out update events to a
+// SnapshotEnvironmentBinding that have resulted in a deployment failure.
+func DeploymentFailedForIntegrationBindingPredicate() predicate.Predicate {
+	return predicate.Funcs{
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			return hasDeploymentFailed(e.ObjectOld, e.ObjectNew)
+		},
+	}
+}

gitops/predicates_test.go

@@ -15,6 +15,7 @@ package gitops_test
 
 import (
 	"context"
+
 	"github.com/redhat-appstudio/integration-service/gitops"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -36,7 +37,7 @@ var _ = Describe("Predicates", Ordered, func() {
 		snapshotName    = "test-snapshot"
 	)
 
-	var bindingMissingStatus, bindingFalseStatus, bindingTrueStatus *applicationapiv1alpha1.SnapshotEnvironmentBinding
+	var bindingMissingStatus, bindingFalseStatus, bindingTrueStatus, bindingDeploymentFailedStatus *applicationapiv1alpha1.SnapshotEnvironmentBinding
 
 	BeforeAll(func() {
 		bindingMissingStatus = &applicationapiv1alpha1.SnapshotEnvironmentBinding{
@@ -79,11 +80,26 @@ var _ = Describe("Predicates", Ordered, func() {
 				Components:  []applicationapiv1alpha1.BindingComponent{},
 			},
 		}
+		bindingDeploymentFailedStatus = &applicationapiv1alpha1.SnapshotEnvironmentBinding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:       "bindingdeploymentfailedstatus",
+				Namespace:  namespace,
+				Generation: 1,
+				Labels:     map[string]string{gitops.SnapshotTestScenarioLabel: "test-scenario"},
+			},
+			Spec: applicationapiv1alpha1.SnapshotEnvironmentBindingSpec{
+				Application: applicationName,
+				Environment: environmentName,
+				Snapshot:    snapshotName,
+				Components:  []applicationapiv1alpha1.BindingComponent{},
+			},
+		}
 		ctx := context.Background()
 
 		Expect(k8sClient.Create(ctx, bindingMissingStatus)).Should(Succeed())
 		Expect(k8sClient.Create(ctx, bindingFalseStatus)).Should(Succeed())
 		Expect(k8sClient.Create(ctx, bindingTrueStatus)).Should(Succeed())
+		Expect(k8sClient.Create(ctx, bindingDeploymentFailedStatus)).Should(Succeed())
 
 		// Set the binding statuses after they are created
 		bindingFalseStatus.Status.ComponentDeploymentConditions = []metav1.Condition{
@@ -98,6 +114,12 @@ var _ = Describe("Predicates", Ordered, func() {
 				Status: metav1.ConditionTrue,
 			},
 		}
+		bindingDeploymentFailedStatus.Status.BindingConditions = []metav1.Condition{
+			{
+				Type:   gitops.BindingErrorOccurredStatusConditionType,
+				Status: metav1.ConditionTrue,
+			},
+		}
 	})
 
 	AfterAll(func() {
@@ -128,4 +150,22 @@ var _ = Describe("Predicates", Ordered, func() {
 			Expect(instance.Update(contextEvent)).To(BeTrue())
 		})
 	})
+	Context("when testing DeploymentFailedPredicate predicate", func() {
+		instance := gitops.DeploymentFailedForIntegrationBindingPredicate()
+		It("returns true when the .Status.BindingConditions.ErrorOccured field of old SEB is set to false and that of new SEB is set to true", func() {
+			contextEvent := event.UpdateEvent{
+				ObjectOld: bindingFalseStatus,
+				ObjectNew: bindingDeploymentFailedStatus,
+			}
+			Expect(instance.Update(contextEvent)).To(BeTrue())
+		})
+
+		It("returns true when the .Status.BindingConditions.ErrorOccured field of old SEB is unset and that of new SEB is set to true", func() {
+			contextEvent := event.UpdateEvent{
+				ObjectOld: bindingMissingStatus,
+				ObjectNew: bindingDeploymentFailedStatus,
+			}
+			Expect(instance.Update(contextEvent)).To(BeTrue())
+		})
+	})
 })

helpers/integration.go

@@ -303,9 +303,7 @@ func IsEnvironmentEphemeral(testEnvironment *applicationapiv1alpha1.Environment)
 	return isEphemeral
 }
 
-func CleanUpEphemeralEnvironments(client client.Client, logger *IntegrationLogger, ctx context.Context, env *applicationapiv1alpha1.Environment,
-	dtc *applicationapiv1alpha1.DeploymentTargetClaim) error {
-
+func CleanUpEphemeralEnvironments(client client.Client, logger *IntegrationLogger, ctx context.Context, env *applicationapiv1alpha1.Environment, dtc *applicationapiv1alpha1.DeploymentTargetClaim) error {
 	logger.Info("Deleting deploymentTargetClaim", "deploymentTargetClaim.Name", dtc.Name)
 	err := client.Delete(ctx, dtc)
 	if err != nil {