diff --git a/controllers/snapshot/snapshot_adapter.go b/controllers/snapshot/snapshot_adapter.go index 777bfb480..7955a1793 100644 --- a/controllers/snapshot/snapshot_adapter.go +++ b/controllers/snapshot/snapshot_adapter.go @@ -206,11 +206,6 @@ func (a *Adapter) EnsureCreationOfEphemeralEnvironments() (controller.OperationR a.logger.Error(err, "Failed to get all environments.") return controller.RequeueWithError(err) } - components, err := a.loader.GetAllApplicationComponents(a.client, a.context, a.application) - if err != nil { - a.logger.Error(err, "Failed to get all components.") - return controller.RequeueWithError(err) - } for _, integrationTestScenario := range *integrationTestScenarios { integrationTestScenario := integrationTestScenario //G601 @@ -218,7 +213,7 @@ func (a *Adapter) EnsureCreationOfEphemeralEnvironments() (controller.OperationR continue } - _, err = a.ensureEphemeralEnvironmentForScenarioExists(&integrationTestScenario, testStatuses, components, allEnvironments) + _, err = a.ensureEphemeralEnvironmentForScenarioExists(&integrationTestScenario, testStatuses, allEnvironments) if err != nil { return controller.RequeueWithError(fmt.Errorf("failed to ensure existence of the environment for scenario %s: %w", integrationTestScenario.Name, err)) } @@ -356,11 +351,6 @@ func (a *Adapter) EnsureSnapshotEnvironmentBindingExist() (controller.OperationR return controller.RequeueWithError(err) } - components, err := a.loader.GetAllSnapshotComponents(a.client, a.context, a.snapshot) - if err != nil { - return controller.RequeueWithError(err) - } - for _, availableEnvironment := range *availableEnvironments { availableEnvironment := availableEnvironment // G601 snapshotEnvironmentBinding, err := a.loader.FindExistingSnapshotEnvironmentBinding(a.client, a.context, a.application, &availableEnvironment) @@ -368,7 +358,7 @@ func (a *Adapter) EnsureSnapshotEnvironmentBindingExist() (controller.OperationR return controller.RequeueWithError(err) } if snapshotEnvironmentBinding != nil { - err = a.updateExistingSnapshotEnvironmentBindingWithSnapshot(snapshotEnvironmentBinding, a.snapshot, components) + err = a.updateExistingSnapshotEnvironmentBindingWithSnapshot(snapshotEnvironmentBinding, a.snapshot) if err != nil { a.logger.Error(err, "Failed to update SnapshotEnvironmentBinding", "snapshotEnvironmentBinding.Environment", snapshotEnvironmentBinding.Spec.Environment, @@ -385,7 +375,7 @@ func (a *Adapter) EnsureSnapshotEnvironmentBindingExist() (controller.OperationR "snapshotEnvironmentBinding.Snapshot", snapshotEnvironmentBinding.Spec.Snapshot) } else { - snapshotEnvironmentBinding, err = a.createSnapshotEnvironmentBindingForSnapshot(a.application, &availableEnvironment, a.snapshot, components) + snapshotEnvironmentBinding, err = a.createSnapshotEnvironmentBindingForSnapshot(a.application, &availableEnvironment, a.snapshot) if err != nil { a.logger.Error(err, "Failed to create SnapshotEnvironmentBinding for snapshot", "environment", availableEnvironment.Name, @@ -449,9 +439,9 @@ func (a *Adapter) createEnvironmentForScenario(integrationTestScenario *v1beta1. } // 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) { +func (a *Adapter) createEnvironmentBindingForScenario(integrationTestScenario *v1beta1.IntegrationTestScenario, environment *applicationapiv1alpha1.Environment) (*applicationapiv1alpha1.SnapshotEnvironmentBinding, error) { scenarioLabelAndKey := map[string]string{gitops.SnapshotTestScenarioLabel: integrationTestScenario.Name} - binding, err := a.createSnapshotEnvironmentBindingForSnapshot(a.application, environment, a.snapshot, components, scenarioLabelAndKey) + binding, err := a.createSnapshotEnvironmentBindingForSnapshot(a.application, environment, a.snapshot, scenarioLabelAndKey) if err != nil { a.logger.Error(err, "Failed to create SnapshotEnvironmentBinding for snapshot", "snapshot", a.snapshot.Name, @@ -467,7 +457,7 @@ func (a *Adapter) createEnvironmentBindingForScenario(integrationTestScenario *v } // ensureEphemeralEnvironmentForScenarioExists creates or return existing epehemeral environment for the given test scenario -func (a *Adapter) ensureEphemeralEnvironmentForScenarioExists(integrationTestScenario *v1beta1.IntegrationTestScenario, testStatuses *intgteststat.SnapshotIntegrationTestStatuses, components *[]applicationapiv1alpha1.Component, allEnvironments *[]applicationapiv1alpha1.Environment) (*applicationapiv1alpha1.SnapshotEnvironmentBinding, error) { +func (a *Adapter) ensureEphemeralEnvironmentForScenarioExists(integrationTestScenario *v1beta1.IntegrationTestScenario, testStatuses *intgteststat.SnapshotIntegrationTestStatuses, allEnvironments *[]applicationapiv1alpha1.Environment) (*applicationapiv1alpha1.SnapshotEnvironmentBinding, error) { var ( environment *applicationapiv1alpha1.Environment @@ -499,7 +489,7 @@ func (a *Adapter) ensureEphemeralEnvironmentForScenarioExists(integrationTestSce } if binding == nil { // create binding and add scenario name to label of binding - binding, err = a.createEnvironmentBindingForScenario(integrationTestScenario, environment, components) + binding, err = a.createEnvironmentBindingForScenario(integrationTestScenario, environment) if err != nil { testStatuses.UpdateTestStatusIfChanged( integrationTestScenario.Name, @@ -649,17 +639,17 @@ func (a *Adapter) createIntegrationPipelineRun(application *applicationapiv1alph } // createSnapshotEnvironmentBindingForSnapshot creates and returns a new snapshotEnvironmentBinding -// for the given application, environment, snapshot, and components. +// for the given application, environment, and snapshot. // If it's not possible to create it and set the application as the owner, an error will be returned func (a *Adapter) createSnapshotEnvironmentBindingForSnapshot(application *applicationapiv1alpha1.Application, environment *applicationapiv1alpha1.Environment, snapshot *applicationapiv1alpha1.Snapshot, - components *[]applicationapiv1alpha1.Component, optionalLabelKeysAndValues ...map[string]string) (*applicationapiv1alpha1.SnapshotEnvironmentBinding, error) { + optionalLabelKeysAndValues ...map[string]string) (*applicationapiv1alpha1.SnapshotEnvironmentBinding, error) { bindingName := application.Name + "-" + environment.Name + "-" + "binding" snapshotEnvironmentBinding := gitops.NewSnapshotEnvironmentBinding( bindingName, application.Namespace, application.Name, environment.Name, - snapshot, *components) + snapshot) // The SEBs for ephemeral Environments are expected to be cleaned up along with the Environment, // while all other SEBs are meant to persist and be deleted with the Application @@ -694,15 +684,14 @@ func (a *Adapter) createSnapshotEnvironmentBindingForSnapshot(application *appli } // updateExistingSnapshotEnvironmentBindingWithSnapshot updates and returns snapshotEnvironmentBinding -// with the given snapshot and components. If it's not possible to patch, an error will be returned. +// with the given snapshot. If it's not possible to patch, an error will be returned. func (a *Adapter) updateExistingSnapshotEnvironmentBindingWithSnapshot(snapshotEnvironmentBinding *applicationapiv1alpha1.SnapshotEnvironmentBinding, - snapshot *applicationapiv1alpha1.Snapshot, - components *[]applicationapiv1alpha1.Component) error { + snapshot *applicationapiv1alpha1.Snapshot) error { patch := client.MergeFrom(snapshotEnvironmentBinding.DeepCopy()) snapshotEnvironmentBinding.Spec.Snapshot = snapshot.Name - snapshotComponents := gitops.NewBindingComponents(*components) + snapshotComponents := gitops.NewBindingComponents(snapshot) snapshotEnvironmentBinding.Spec.Components = *snapshotComponents err := a.client.Patch(a.context, snapshotEnvironmentBinding, patch) diff --git a/controllers/snapshot/snapshot_adapter_test.go b/controllers/snapshot/snapshot_adapter_test.go index 9d7418ccb..293502847 100644 --- a/controllers/snapshot/snapshot_adapter_test.go +++ b/controllers/snapshot/snapshot_adapter_test.go @@ -836,22 +836,18 @@ var _ = Describe("Snapshot Adapter", Ordered, func() { var snapshotEnvironmentBinding *applicationapiv1alpha1.SnapshotEnvironmentBinding // create snapshot environment - components := []applicationapiv1alpha1.Component{ - *hasComp, - } newLabels := map[string]string{} newLabels[gitops.SnapshotTestScenarioLabel] = integrationTestScenario.Name - snapshotEnvironmentBinding, err := adapter.createSnapshotEnvironmentBindingForSnapshot(adapter.application, env, hasSnapshot, &components, newLabels) + snapshotEnvironmentBinding, err := adapter.createSnapshotEnvironmentBindingForSnapshot(adapter.application, env, hasSnapshot, newLabels) Expect(err).To(BeNil()) Expect(snapshotEnvironmentBinding).NotTo(BeNil()) Expect(snapshotEnvironmentBinding.Labels[gitops.SnapshotTestScenarioLabel]).To(Equal(integrationTestScenario.Name)) - // update snapshot environment with new component - componentsUpdate := []applicationapiv1alpha1.Component{ - *secondComp, - } + otherSnapshot := hasSnapshot.DeepCopy() + otherSnapshot.Name = "other-snapshot" + otherSnapshot.Spec.Components = []applicationapiv1alpha1.SnapshotComponent{{Name: secondComp.Name, ContainerImage: sample_image}} - err = adapter.updateExistingSnapshotEnvironmentBindingWithSnapshot(snapshotEnvironmentBinding, hasSnapshot, &componentsUpdate) + err = adapter.updateExistingSnapshotEnvironmentBindingWithSnapshot(snapshotEnvironmentBinding, otherSnapshot) Expect(err).To(BeNil()) // get fresh copy to make sure that SEB was updated in k8s @@ -863,7 +859,8 @@ var _ = Describe("Snapshot Adapter", Ordered, func() { }, updatedSEB) return err == nil && len(updatedSEB.Spec.Components) == 1 }, time.Second*10).Should(BeTrue()) - Expect(updatedSEB.Spec.Components[0].Name == secondComp.Spec.ComponentName) + Expect(updatedSEB.Spec.Snapshot == otherSnapshot.Name) + Expect(updatedSEB.Spec.Components[0].Name == secondComp.Name) err = k8sClient.Delete(ctx, snapshotEnvironmentBinding) Expect(err == nil || errors.IsNotFound(err)).To(BeTrue()) diff --git a/gitops/binding.go b/gitops/binding.go index 8e885c90b..ffcbe02ff 100644 --- a/gitops/binding.go +++ b/gitops/binding.go @@ -37,8 +37,8 @@ const ( ) // NewSnapshotEnvironmentBinding creates a new SnapshotEnvironmentBinding using the provided info. -func NewSnapshotEnvironmentBinding(bindingName string, namespace string, applicationName string, environmentName string, snapshot *applicationapiv1alpha1.Snapshot, components []applicationapiv1alpha1.Component) *applicationapiv1alpha1.SnapshotEnvironmentBinding { - bindingComponents := NewBindingComponents(components) +func NewSnapshotEnvironmentBinding(bindingName string, namespace string, applicationName string, environmentName string, snapshot *applicationapiv1alpha1.Snapshot) *applicationapiv1alpha1.SnapshotEnvironmentBinding { + bindingComponents := NewBindingComponents(snapshot) snapshotEnvironmentBinding := &applicationapiv1alpha1.SnapshotEnvironmentBinding{ ObjectMeta: metav1.ObjectMeta{ @@ -58,11 +58,11 @@ func NewSnapshotEnvironmentBinding(bindingName string, namespace string, applica // NewBindingComponents gets all components from the Snapshot and formats them to be used in the // SnapshotEnvironmentBinding as BindingComponents. -func NewBindingComponents(components []applicationapiv1alpha1.Component) *[]applicationapiv1alpha1.BindingComponent { +func NewBindingComponents(snapshot *applicationapiv1alpha1.Snapshot) *[]applicationapiv1alpha1.BindingComponent { bindingComponents := []applicationapiv1alpha1.BindingComponent{} - for _, component := range components { + for _, component := range snapshot.Spec.Components { bindingComponents = append(bindingComponents, applicationapiv1alpha1.BindingComponent{ - Name: component.Spec.ComponentName, + Name: component.Name, }) } return &bindingComponents diff --git a/gitops/binding_test.go b/gitops/binding_test.go index e24a547a7..83d54cdf1 100644 --- a/gitops/binding_test.go +++ b/gitops/binding_test.go @@ -177,13 +177,13 @@ var _ = Describe("Gitops functions for managing Snapshots", Ordered, func() { It("ensures Binding Component is created", func() { gitops.MarkSnapshotAsPassed(k8sClient, ctx, hasSnapshot, "test passed") Expect(gitops.HaveAppStudioTestsSucceeded(hasSnapshot)).To(BeTrue()) - bindingComponents := gitops.NewBindingComponents([]applicationapiv1alpha1.Component{*hasComp}) + bindingComponents := gitops.NewBindingComponents(hasSnapshot) Expect(bindingComponents).NotTo(BeNil()) Expect(hasComp.Spec.Replicas).To(Equal(0)) }) It("ensures a new SnapshotEnvironmentBinding is created", func() { - newSnapshotEnvironmentBinding := gitops.NewSnapshotEnvironmentBinding("sample", namespace, hasApp.Name, env.Name, hasSnapshot, []applicationapiv1alpha1.Component{*hasComp}) + newSnapshotEnvironmentBinding := gitops.NewSnapshotEnvironmentBinding("sample", namespace, hasApp.Name, env.Name, hasSnapshot) Expect(newSnapshotEnvironmentBinding).NotTo(BeNil()) Expect(newSnapshotEnvironmentBinding.Spec.Snapshot).To(Equal(hasSnapshot.Name)) Expect(newSnapshotEnvironmentBinding.Spec.Environment).To(Equal(env.Name)) diff --git a/loader/loader.go b/loader/loader.go index 979ee19df..2f49f9c9d 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -38,7 +38,6 @@ type ObjectLoader interface { GetAllEnvironments(c client.Client, ctx context.Context, application *applicationapiv1alpha1.Application) (*[]applicationapiv1alpha1.Environment, error) GetReleasesWithSnapshot(c client.Client, ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) (*[]releasev1alpha1.Release, error) GetAllApplicationComponents(c client.Client, ctx context.Context, application *applicationapiv1alpha1.Application) (*[]applicationapiv1alpha1.Component, error) - GetAllSnapshotComponents(c client.Client, ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) (*[]applicationapiv1alpha1.Component, error) GetApplicationFromSnapshot(c client.Client, ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) (*applicationapiv1alpha1.Application, error) GetComponentFromSnapshot(c client.Client, ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) (*applicationapiv1alpha1.Component, error) GetComponentFromPipelineRun(c client.Client, ctx context.Context, pipelineRun *tektonv1beta1.PipelineRun) (*applicationapiv1alpha1.Component, error) @@ -121,27 +120,6 @@ func (l *loader) GetAllApplicationComponents(c client.Client, ctx context.Contex return &applicationComponents.Items, nil } -// GetAllSnapshotComponents loads from the cluster all Components associated with the given Snapshot. -// If the Snapshot doesn't have any Components or this is not found in the cluster, an error will be returned. -func (l *loader) GetAllSnapshotComponents(c client.Client, ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) (*[]applicationapiv1alpha1.Component, error) { - components := []applicationapiv1alpha1.Component{} - - for _, sc := range snapshot.Spec.Components { - component := &applicationapiv1alpha1.Component{} - err := c.Get(ctx, types.NamespacedName{ - Namespace: snapshot.Namespace, - Name: sc.Name, - }, component) - if err != nil { - return nil, err - } - - components = append(components, *component) - } - - return &components, nil -} - // GetApplicationFromSnapshot loads from the cluster the Application referenced in the given Snapshot. // If the Snapshot doesn't specify an Component or this is not found in the cluster, an error will be returned. func (l *loader) GetApplicationFromSnapshot(c client.Client, ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) (*applicationapiv1alpha1.Application, error) { diff --git a/loader/loader_mock.go b/loader/loader_mock.go index 748f4cd9b..9fd841c54 100644 --- a/loader/loader_mock.go +++ b/loader/loader_mock.go @@ -124,15 +124,6 @@ func (l *mockLoader) GetAllApplicationComponents(c client.Client, ctx context.Co return &components, err } -// GetAllSnapshotComponents returns the resource and error passed as values of the context. -func (l *mockLoader) GetAllSnapshotComponents(c client.Client, ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) (*[]applicationapiv1alpha1.Component, error) { - if ctx.Value(SnapshotComponentsContextKey) == nil { - return l.loader.GetAllSnapshotComponents(c, ctx, snapshot) - } - components, err := getMockedResourceAndErrorFromContext(ctx, SnapshotComponentsContextKey, []applicationapiv1alpha1.Component{}) - return &components, err -} - // GetApplicationFromSnapshot returns the resource and error passed as values of the context. func (l *mockLoader) GetApplicationFromSnapshot(c client.Client, ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) (*applicationapiv1alpha1.Application, error) { if ctx.Value(ApplicationContextKey) == nil { diff --git a/loader/loader_mock_test.go b/loader/loader_mock_test.go index d9977be3d..6c1172242 100644 --- a/loader/loader_mock_test.go +++ b/loader/loader_mock_test.go @@ -139,21 +139,6 @@ var _ = Describe("Release Adapter", Ordered, func() { }) }) - Context("When calling GetAllSnapshotComponents", func() { - It("returns resource and error from the context", func() { - snapshotComponents := []applicationapiv1alpha1.Component{} - mockContext := GetMockedContext(ctx, []MockData{ - { - ContextKey: SnapshotComponentsContextKey, - Resource: snapshotComponents, - }, - }) - resource, err := loader.GetAllSnapshotComponents(nil, mockContext, nil) - Expect(resource).To(Equal(&snapshotComponents)) - Expect(err).To(BeNil()) - }) - }) - Context("When calling GetApplicationFromSnapshot", func() { It("returns resource and error from the context", func() { application := &applicationapiv1alpha1.Application{} diff --git a/loader/loader_test.go b/loader/loader_test.go index 4cbf34a5c..6c3da71ba 100644 --- a/loader/loader_test.go +++ b/loader/loader_test.go @@ -443,50 +443,6 @@ var _ = Describe("Loader", Ordered, func() { Expect(applicationComponents).NotTo(BeNil()) }) - It("ensures the Snapshot Components can be found ", func() { - snapshotComponents, err := loader.GetAllSnapshotComponents(k8sClient, ctx, hasSnapshot) - Expect(err).To(BeNil()) - Expect(snapshotComponents).NotTo(BeNil()) - }) - - It("ensures GetAllSnapshotComponents does not gather non-snapshot application components ", func() { - // otherComp has the same namespace and application as the snapshot, - // but does not belong to the snapshot itself. When creating SEBs - // for older snapshots we do not want to accidentally add components - // for newer snapshots. - otherComp := &applicationapiv1alpha1.Component{ - ObjectMeta: metav1.ObjectMeta{ - Name: "other-component", - Namespace: "default", - }, - Spec: applicationapiv1alpha1.ComponentSpec{ - ComponentName: "other-component", - Application: applicationName, - ContainerImage: "", - Source: applicationapiv1alpha1.ComponentSource{ - ComponentSourceUnion: applicationapiv1alpha1.ComponentSourceUnion{ - GitSource: &applicationapiv1alpha1.GitSource{ - URL: SampleRepoLink, - }, - }, - }, - }, - Status: applicationapiv1alpha1.ComponentStatus{ - LastBuiltCommit: "", - }, - } - Expect(k8sClient.Create(ctx, otherComp)).Should(Succeed()) - defer k8sClient.Delete(ctx, otherComp) - - snapshotComponents, err := loader.GetAllSnapshotComponents(k8sClient, ctx, hasSnapshot) - Expect(err).To(BeNil()) - Expect(snapshotComponents).NotTo(BeNil()) - Expect(*snapshotComponents).To(HaveLen(1)) - for _, comp := range *snapshotComponents { - Expect(comp.Name).NotTo(Equal("other-component")) - } - }) - It("ensures we can get an Application from a Snapshot ", func() { app, err := loader.GetApplicationFromSnapshot(k8sClient, ctx, hasSnapshot) Expect(err).To(BeNil())