Merge pull request #381 from hongweiliu17/STONEINTG-642

fix(STONEINTG-642) stop reconciliation if env is not found for ITS

controllers/snapshot/snapshot_adapter.go

@@ -22,7 +22,6 @@ import (
 	"reflect"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	applicationapiv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
@@ -215,6 +214,11 @@ func (a *Adapter) EnsureCreationOfEphemeralEnvironments() (controller.OperationR
 
 		_, err = a.ensureEphemeralEnvironmentForScenarioExists(&integrationTestScenario, testStatuses, allEnvironments)
 		if err != nil {
+			if h.IsEnvironmentNotInNamespaceError(err) {
+				a.logger.Error(err, "environment doesn't exist in the same namespace as integrationScenario at all, try again after creating environment",
+					"integrationTestScenario.Name", integrationTestScenario.Name)
+				return controller.StopProcessing()
+			}
 			return controller.RequeueWithError(fmt.Errorf("failed to ensure existence of the environment for scenario %s: %w", integrationTestScenario.Name, err))
 		}
 	}
@@ -416,28 +420,6 @@ func (a *Adapter) getExistingEnvironmentForScenario(integrationTestScenario *v1b
 	return nil, false
 }
 
-// createEnvironmentForScenario creates new epehemeral environment for scenario by copying existing environment
-func (a *Adapter) createEnvironmentForScenario(integrationTestScenario *v1beta1.IntegrationTestScenario) (*applicationapiv1alpha1.Environment, error) {
-	//get the existing environment according to environment name from integrationTestScenario
-	existingEnv, err := a.getEnvironmentFromIntegrationTestScenario(integrationTestScenario)
-	if err != nil {
-		a.logger.Error(err, "Failed to find the env defined in integrationTestScenario",
-			"integrationTestScenario.Namespace", integrationTestScenario.Namespace,
-			"integrationTestScenario.Name", integrationTestScenario.Name)
-		return nil, err
-	}
-
-	//create an ephemeral copy env of existing environment
-	copyEnv, err := a.createCopyOfExistingEnvironment(existingEnv, a.snapshot.Namespace, integrationTestScenario, a.snapshot, a.application)
-
-	if err != nil {
-		a.logger.Error(err, "Copying of environment failed")
-		return nil, fmt.Errorf("copying of environment failed: %w", err)
-	}
-
-	return copyEnv, nil
-}
-
 // createEnvironmentBindingForScenario creates SnapshotEnvironmentBinding for the given test scenario and snapshot
 func (a *Adapter) createEnvironmentBindingForScenario(integrationTestScenario *v1beta1.IntegrationTestScenario, environment *applicationapiv1alpha1.Environment) (*applicationapiv1alpha1.SnapshotEnvironmentBinding, error) {
 	scenarioLabelAndKey := map[string]string{gitops.SnapshotTestScenarioLabel: integrationTestScenario.Name}
@@ -467,8 +449,20 @@ func (a *Adapter) ensureEphemeralEnvironmentForScenarioExists(integrationTestSce
 	environment, ok := a.getExistingEnvironmentForScenario(integrationTestScenario, allEnvironments)
 	if !ok {
 		// environment doesn't exist, we have to create a new one
-		environment, err = a.createEnvironmentForScenario(integrationTestScenario)
+		existingEnv := findEnvironmentFromAllEnvironmentsByName(allEnvironments, integrationTestScenario.Spec.Environment.Name)
+		if existingEnv == nil {
+			err := h.NewEnvironmentNotInNamespaceError(integrationTestScenario.Spec.Environment.Name, integrationTestScenario.Namespace)
+			testStatuses.UpdateTestStatusIfChanged(
+				integrationTestScenario.Name, intgteststat.IntegrationTestStatusEnvironmentProvisionError,
+				fmt.Sprintf("Creation of copied ephemeral environment failed: %s. Try again after creating environment.", err))
+			a.writeIntegrationTestStatusAtError(testStatuses)
+			return nil, err
+		}
+		//create an ephemeral copy env of existing environment
+		environment, err = a.createCopyOfExistingEnvironment(existingEnv, a.snapshot.Namespace, integrationTestScenario, a.snapshot, a.application)
+
 		if err != nil {
+			a.logger.Error(err, "Copying of environment failed")
 			testStatuses.UpdateTestStatusIfChanged(
 				integrationTestScenario.Name, intgteststat.IntegrationTestStatusEnvironmentProvisionError,
 				fmt.Sprintf("Creation of ephemeral environment failed: %s", err))
@@ -768,23 +762,15 @@ func (a *Adapter) CreateDeploymentTargetClaimForEnvironment(namespace string, de
 	return deploymentTargetClaim, nil
 }
 
-// getEnvironmentFromIntegrationTestScenario looks for already existing environment, if it exists it is returned, if not, nil is returned then together with
-// information about what went wrong
-func (a *Adapter) getEnvironmentFromIntegrationTestScenario(integrationTestScenario *v1beta1.IntegrationTestScenario) (*applicationapiv1alpha1.Environment, error) {
-	existingEnv := &applicationapiv1alpha1.Environment{}
-
-	err := a.client.Get(a.context, types.NamespacedName{
-		Namespace: a.application.Namespace,
-		Name:      integrationTestScenario.Spec.Environment.Name,
-	}, existingEnv)
-
-	if err != nil {
-		a.logger.Info("Environment doesn't exist in same namespace as IntegrationTestScenario at all.",
-			"integrationTestScenario:", integrationTestScenario.Name,
-			"environment:", integrationTestScenario.Spec.Environment.Name)
-		return nil, fmt.Errorf("environment %s doesn't exist in same namespace as IntegrationTestScenario at all: %w", integrationTestScenario.Spec.Environment.Name, err)
+// findEnvironmentFromAllEnvironmentsByName try to find environment from all environments by name, return the environment if it exists, otherwise return nil
+func findEnvironmentFromAllEnvironmentsByName(allEnvironments *[]applicationapiv1alpha1.Environment, envname string) *applicationapiv1alpha1.Environment {
+	for _, e := range *allEnvironments {
+		e := e //G601
+		if e.Name == envname {
+			return &e
+		}
 	}
-	return existingEnv, nil
+	return nil
 }
 
 // writeIntegrationTestStatusAtError writes updates of integration test statuses into snapshot

controllers/snapshot/snapshot_adapter_test.go

@@ -60,6 +60,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 		integrationTestScenarioWithoutEnv     *v1beta1.IntegrationTestScenario
 		integrationTestScenarioWithoutEnvCopy *v1beta1.IntegrationTestScenario
 		env                                   *applicationapiv1alpha1.Environment
+		tmpEnv                                *applicationapiv1alpha1.Environment
 	)
 	const (
 		SampleRepoLink  = "https://github.com/devfile-samples/devfile-sample-java-springboot-basic"
@@ -1021,6 +1022,83 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 		})
 	})
 
+	When("An environment defined in ITS doesn't exist", func() {
+		var (
+			buf bytes.Buffer
+		)
+
+		BeforeAll(func() {
+			tmpEnv = &applicationapiv1alpha1.Environment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "envname-tmp",
+					Namespace: "default",
+				},
+				Spec: applicationapiv1alpha1.EnvironmentSpec{
+					Type:               "POC",
+					DisplayName:        "envname-tmp",
+					DeploymentStrategy: applicationapiv1alpha1.DeploymentStrategy_Manual,
+					ParentEnvironment:  "",
+					Tags:               []string{},
+					Configuration: applicationapiv1alpha1.EnvironmentConfiguration{
+						Env: []applicationapiv1alpha1.EnvVarPair{
+							{
+								Name:  "var_name",
+								Value: "test",
+							},
+						},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, tmpEnv)).Should(Succeed())
+
+			log := helpers.IntegrationLogger{Logger: buflogr.NewWithBuffer(&buf)}
+			adapter = NewAdapter(hasSnapshotPR, hasApp, hasComp, log, loader.NewMockLoader(), k8sClient, ctx)
+			adapter.context = loader.GetMockedContext(ctx, []loader.MockData{
+				{
+					ContextKey: loader.ApplicationContextKey,
+					Resource:   hasApp,
+				},
+				{
+					ContextKey: loader.ComponentContextKey,
+					Resource:   hasComp,
+				},
+				{
+					ContextKey: loader.SnapshotContextKey,
+					Resource:   hasSnapshotPR,
+				},
+				{
+					ContextKey: loader.EnvironmentContextKey,
+					Resource:   tmpEnv,
+				},
+				{
+					ContextKey: loader.SnapshotEnvironmentBindingContextKey,
+					Resource:   nil,
+				},
+				{
+					ContextKey: loader.ApplicationComponentsContextKey,
+					Resource:   []applicationapiv1alpha1.Component{*hasComp},
+				},
+				{
+					ContextKey: loader.AllIntegrationTestScenariosContextKey,
+					Resource:   []v1beta1.IntegrationTestScenario{*integrationTestScenario},
+				},
+			})
+		})
+
+		AfterAll(func() {
+			err := k8sClient.Delete(ctx, tmpEnv)
+			Expect(err == nil || errors.IsNotFound(err)).To(BeTrue())
+		})
+
+		It("Ensure stopping processing when environment defined in ITS doesn't exist", func() {
+			result, err := adapter.EnsureCreationOfEphemeralEnvironments()
+			Expect(result.CancelRequest && !result.RequeueRequest && err == nil).To(BeTrue())
+			expectedLogEntry := "environment doesn't exist in the same namespace as integrationScenario at all"
+			Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
+		})
+
+	})
+
 	Describe("shouldScenarioRunInEphemeralEnv", func() {
 		It("returns true when env is defined in scenario", func() {
 			Expect(shouldScenarioRunInEphemeralEnv(integrationTestScenario)).To(BeTrue())

helpers/errorhandlers.go

@@ -20,6 +20,39 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
+const (
+	ReasonEnvironmentNotInNamespace = "EnvironmentNotInNamespace"
+	ReasonUnknownError              = "UnknownError"
+)
+
+type IntegrationError struct {
+	Reason  string
+	Message string
+}
+
+func (ie *IntegrationError) Error() string {
+	return ie.Message
+}
+
+func getReason(err error) string {
+	integrationErr, ok := err.(*IntegrationError)
+	if !ok {
+		return ReasonUnknownError
+	}
+	return integrationErr.Reason
+}
+
+func NewEnvironmentNotInNamespaceError(environment, namespace string) error {
+	return &IntegrationError{
+		Reason:  ReasonEnvironmentNotInNamespace,
+		Message: fmt.Sprintf("Environment %s not found in namespace %s", environment, namespace),
+	}
+}
+
+func IsEnvironmentNotInNamespaceError(err error) bool {
+	return getReason(err) == ReasonEnvironmentNotInNamespace
+}
+
 func HandleLoaderError(logger IntegrationLogger, err error, resource, from string) (ctrl.Result, error) {
 	if errors.IsNotFound(err) {
 		logger.Info(fmt.Sprintf("Could not get %[1]s from %[2]s.  %[1]s may have been removed.  Declining to proceed with reconciliation", resource, from))

helpers/errorhandlers_test.go

@@ -18,6 +18,7 @@ package helpers_test
 
 import (
 	"bytes"
+	"fmt"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -69,4 +70,17 @@ var _ = Describe("Helpers for error handlers", Ordered, func() {
 			Expect(logbuf.String()).Should(ContainSubstring("Failed to get Component from the Snapshot"))
 		})
 	})
+
+	Context("Handling customized integration error", func() {
+		It("Can define an NewEnvironmentNotInNamespaceError", func() {
+			err := helpers.NewEnvironmentNotInNamespaceError("env", "namespace")
+			Expect(helpers.IsEnvironmentNotInNamespaceError(err)).To(BeTrue())
+			Expect(err.Error()).To(Equal("Environment env not found in namespace namespace"))
+		})
+
+		It("Can handle non integration error", func() {
+			err := fmt.Errorf("failed")
+			Expect(helpers.IsEnvironmentNotInNamespaceError(err)).To(BeFalse())
+		})
+	})
 })