From 9caf9365ec0fc5a7c74f36ca0d760799d62f5009 Mon Sep 17 00:00:00 2001 From: Hongwei Liu Date: Wed, 25 Oct 2023 11:06:41 +0800 Subject: [PATCH] fix(STONEINTG-642): stop reconciliation if env is not found for ITS * stop reconciliation if the env defined in ITS doesn't exist Signed-off-by: Hongwei Liu --- controllers/snapshot/snapshot_adapter.go | 66 +++++++--------- controllers/snapshot/snapshot_adapter_test.go | 78 +++++++++++++++++++ helpers/errorhandlers.go | 33 ++++++++ helpers/errorhandlers_test.go | 14 ++++ 4 files changed, 151 insertions(+), 40 deletions(-) diff --git a/controllers/snapshot/snapshot_adapter.go b/controllers/snapshot/snapshot_adapter.go index 777bfb480..413fc5687 100644 --- a/controllers/snapshot/snapshot_adapter.go +++ b/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" @@ -220,6 +219,11 @@ func (a *Adapter) EnsureCreationOfEphemeralEnvironments() (controller.OperationR _, err = a.ensureEphemeralEnvironmentForScenarioExists(&integrationTestScenario, testStatuses, components, 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)) } } @@ -426,28 +430,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, components *[]applicationapiv1alpha1.Component) (*applicationapiv1alpha1.SnapshotEnvironmentBinding, error) { scenarioLabelAndKey := map[string]string{gitops.SnapshotTestScenarioLabel: integrationTestScenario.Name} @@ -477,8 +459,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)) @@ -778,23 +772,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 diff --git a/controllers/snapshot/snapshot_adapter_test.go b/controllers/snapshot/snapshot_adapter_test.go index 9d7418ccb..3967de240 100644 --- a/controllers/snapshot/snapshot_adapter_test.go +++ b/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" @@ -1024,6 +1025,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()) diff --git a/helpers/errorhandlers.go b/helpers/errorhandlers.go index b612edbc2..38d4019f6 100644 --- a/helpers/errorhandlers.go +++ b/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)) diff --git a/helpers/errorhandlers_test.go b/helpers/errorhandlers_test.go index 0317f059f..a6246f91f 100644 --- a/helpers/errorhandlers_test.go +++ b/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()) + }) + }) })