diff --git a/docs/statusreport-controller.md b/docs/statusreport-controller.md index bc6d7ccd7..7a25824e2 100644 --- a/docs/statusreport-controller.md +++ b/docs/statusreport-controller.md @@ -15,9 +15,7 @@ flowchart TD get_required_scenarios(Get all required
IntegrationTestScenarios) parse_snapshot_status(Parse the Snapshot's
status annotation) check_finished_tests{Did Snapshot
finish all required
integration tests?} - check_supersede{Does Snapshot need
to be superseded
with a composite Snapshot?} check_passed_tests{Did Snapshot
pass all required
integration tests?} - create_snapshot(Create composite Snapshot) update_status(Update Snapshot status accordingly) continue_processing_tests(Controller continues processing) @@ -26,11 +24,7 @@ flowchart TD predicate ----> |"EnsureSnapshotFinishedAllTests()"|get_required_scenarios get_required_scenarios ---> parse_snapshot_status parse_snapshot_status ---> check_finished_tests - check_finished_tests --Yes--> check_supersede - check_finished_tests --No--> continue_processing_tests - check_supersede --Yes--> create_snapshot - check_supersede --No--> check_passed_tests - create_snapshot ---> update_status + check_finished_tests ---> continue_processing_tests check_passed_tests --Yes--> update_status check_passed_tests --No--> update_status update_status ----> continue_processing_tests diff --git a/gitops/snapshot.go b/gitops/snapshot.go index 20893e6de..f9507055a 100644 --- a/gitops/snapshot.go +++ b/gitops/snapshot.go @@ -83,9 +83,6 @@ const ( // SnapshotComponentType is the type of Snapshot which was created for a single component build. SnapshotComponentType = "component" - // SnapshotCompositeType is the type of Snapshot which was created for multiple components. - SnapshotCompositeType = "composite" - // SnapshotOverrideType is the type of Snapshot which was created for override Global Candidate List. SnapshotOverrideType = "override" @@ -823,8 +820,8 @@ func ResetSnapshotStatusConditions(ctx context.Context, adapterClient client.Cli } // CopySnapshotLabelsAndAnnotation coppies labels and annotations from build pipelineRun or tested snapshot -// into regular or composite snapshot -func CopySnapshotLabelsAndAnnotation(application *applicationapiv1alpha1.Application, snapshot *applicationapiv1alpha1.Snapshot, componentName string, source *metav1.ObjectMeta, prefix string, isComposite bool) { +// into regular snapshot +func CopySnapshotLabelsAndAnnotation(application *applicationapiv1alpha1.Application, snapshot *applicationapiv1alpha1.Snapshot, componentName string, source *metav1.ObjectMeta, prefix string) { if snapshot.Labels == nil { snapshot.Labels = map[string]string{} @@ -833,11 +830,7 @@ func CopySnapshotLabelsAndAnnotation(application *applicationapiv1alpha1.Applica if snapshot.Annotations == nil { snapshot.Annotations = map[string]string{} } - if !isComposite { - snapshot.Labels[SnapshotTypeLabel] = SnapshotComponentType - } else { - snapshot.Labels[SnapshotTypeLabel] = SnapshotCompositeType - } + snapshot.Labels[SnapshotTypeLabel] = SnapshotComponentType snapshot.Labels[SnapshotComponentLabel] = componentName snapshot.Labels[ApplicationNameLabel] = application.Name diff --git a/internal/controller/buildpipeline/buildpipeline_adapter.go b/internal/controller/buildpipeline/buildpipeline_adapter.go index 6c2448fbe..aa59c54cf 100644 --- a/internal/controller/buildpipeline/buildpipeline_adapter.go +++ b/internal/controller/buildpipeline/buildpipeline_adapter.go @@ -210,7 +210,7 @@ func (a *Adapter) prepareSnapshotForPipelineRun(pipelineRun *tektonv1.PipelineRu return nil, err } - gitops.CopySnapshotLabelsAndAnnotation(application, snapshot, a.component.Name, &pipelineRun.ObjectMeta, gitops.BuildPipelineRunPrefix, false) + gitops.CopySnapshotLabelsAndAnnotation(application, snapshot, a.component.Name, &pipelineRun.ObjectMeta, gitops.BuildPipelineRunPrefix) snapshot.Labels[gitops.BuildPipelineRunNameLabel] = pipelineRun.Name if pipelineRun.Status.CompletionTime != nil { diff --git a/internal/controller/buildpipeline/buildpipeline_adapter_test.go b/internal/controller/buildpipeline/buildpipeline_adapter_test.go index 81df2522c..ca3f7ba73 100644 --- a/internal/controller/buildpipeline/buildpipeline_adapter_test.go +++ b/internal/controller/buildpipeline/buildpipeline_adapter_test.go @@ -415,7 +415,7 @@ var _ = Describe("Pipeline Adapter", Ordered, func() { Expect(err).ToNot(HaveOccurred()) Expect(copyToSnapshot).NotTo(BeNil()) - gitops.CopySnapshotLabelsAndAnnotation(hasApp, copyToSnapshot, hasComp.Name, &buildPipelineRun.ObjectMeta, gitops.BuildPipelineRunPrefix, false) + gitops.CopySnapshotLabelsAndAnnotation(hasApp, copyToSnapshot, hasComp.Name, &buildPipelineRun.ObjectMeta, gitops.BuildPipelineRunPrefix) Expect(copyToSnapshot.Labels[gitops.SnapshotTypeLabel]).To(Equal(gitops.SnapshotComponentType)) Expect(copyToSnapshot.Labels[gitops.SnapshotComponentLabel]).To(Equal(hasComp.Name)) Expect(copyToSnapshot.Labels[gitops.ApplicationNameLabel]).To(Equal(hasApp.Name)) diff --git a/internal/controller/component/component_adapter.go b/internal/controller/component/component_adapter.go index 31021bebd..02d408d59 100644 --- a/internal/controller/component/component_adapter.go +++ b/internal/controller/component/component_adapter.go @@ -135,11 +135,6 @@ func (a *Adapter) createUpdatedSnapshot(snapshotComponents *[]applicationapiv1al if snapshot.Labels == nil { snapshot.Labels = map[string]string{} } - snapshotType := gitops.SnapshotCompositeType - if len(*snapshotComponents) == 1 { - snapshotType = gitops.SnapshotComponentType - } - snapshot.Labels[gitops.SnapshotTypeLabel] = snapshotType err := ctrl.SetControllerReference(a.application, snapshot, a.client.Scheme()) if err != nil { diff --git a/internal/controller/statusreport/statusreport_adapter.go b/internal/controller/statusreport/statusreport_adapter.go index ff8f41e3c..33e0061da 100644 --- a/internal/controller/statusreport/statusreport_adapter.go +++ b/internal/controller/statusreport/statusreport_adapter.go @@ -23,7 +23,6 @@ import ( "github.com/konflux-ci/operator-toolkit/controller" applicationapiv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1" - "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/konflux-ci/integration-service/api/v1beta2" @@ -31,10 +30,7 @@ import ( "github.com/konflux-ci/integration-service/helpers" "github.com/konflux-ci/integration-service/loader" intgteststat "github.com/konflux-ci/integration-service/pkg/integrationteststatus" - "github.com/konflux-ci/integration-service/pkg/metrics" "github.com/konflux-ci/integration-service/status" - "github.com/konflux-ci/integration-service/tekton" - "github.com/konflux-ci/operator-toolkit/metadata" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -122,8 +118,6 @@ func (a *Adapter) EnsureSnapshotTestStatusReportedToGitProvider() (controller.Op // EnsureSnapshotFinishedAllTests is an operation that will ensure that a pipeline Snapshot // to the PipelineRun being processed finished and passed all tests for all defined required IntegrationTestScenarios. -// If the Snapshot doesn't have the freshest state of components, a composite Snapshot will be created instead -// and the original Snapshot will be marked as Invalid. func (a *Adapter) EnsureSnapshotFinishedAllTests() (controller.OperationResult, error) { // Get all required integrationTestScenarios for the Application and then use the Snapshot status annotation // to check if all Integration tests were finished for that Snapshot @@ -178,46 +172,6 @@ func (a *Adapter) EnsureSnapshotFinishedAllTests() (controller.OperationResult, a.logger.LogAuditEvent(finishedStatusMessage, a.snapshot, helpers.LogActionUpdate) } - // If the Snapshot is a component type, check if the global component list changed in the meantime and - // create a composite snapshot if it did. Does not apply for PAC pull request events. - if metadata.HasLabelWithValue(a.snapshot, gitops.SnapshotTypeLabel, gitops.SnapshotComponentType) && gitops.IsSnapshotCreatedByPACPushEvent(a.snapshot) { - var component *applicationapiv1alpha1.Component - err = retry.OnError(retry.DefaultRetry, func(_ error) bool { return true }, func() error { - component, err = a.loader.GetComponentFromSnapshot(a.context, a.client, a.snapshot) - return err - }) - if err != nil { - if _, err = helpers.HandleLoaderError(a.logger, err, fmt.Sprintf("Component or '%s' label", tekton.ComponentNameLabel), "Snapshot"); err != nil { - return controller.RequeueWithError(err) - } - return controller.ContinueProcessing() - } - - compositeSnapshot, err := a.createCompositeSnapshotsIfConflictExists(a.application, component, a.snapshot) - if err != nil { - a.logger.Error(err, "Failed to determine if a composite snapshot needs to be created because of a conflict", - "snapshot.Name", a.snapshot.Name) - return controller.RequeueWithError(err) - } - - if compositeSnapshot != nil { - a.logger.Info("The global component list has changed in the meantime, marking snapshot as Invalid", - "snapshot.Name", a.snapshot.Name) - if !gitops.IsSnapshotMarkedAsInvalid(a.snapshot) { - err = gitops.MarkSnapshotAsInvalid(a.context, a.client, a.snapshot, - "The global component list has changed in the meantime, superseding with a composite snapshot") - if err != nil { - a.logger.Error(err, "Failed to update the status to Invalid for the snapshot", - "snapshot.Name", a.snapshot.Name) - return controller.RequeueWithError(err) - } - a.logger.LogAuditEvent("Snapshot integration status condition marked as invalid, the global component list has changed in the meantime", - a.snapshot, helpers.LogActionUpdate) - } - return controller.ContinueProcessing() - } - } - // If all Integration Pipeline runs passed, mark the snapshot as succeeded, otherwise mark it as failed // This updates the Snapshot resource on the cluster if allIntegrationTestsPassed { @@ -271,95 +225,6 @@ func (a *Adapter) determineIfAllRequiredIntegrationTestsFinishedAndPassed(integr return allIntegrationTestsFinished, allIntegrationTestsPassed } -// prepareCompositeSnapshot prepares the Composite Snapshot for a given application, -// component, containerImage and containerSource. In case the Snapshot can't be created, an error will be returned. -func (a *Adapter) prepareCompositeSnapshot(application *applicationapiv1alpha1.Application, component *applicationapiv1alpha1.Component, newContainerImage string, newComponentSource *applicationapiv1alpha1.ComponentSource) (*applicationapiv1alpha1.Snapshot, error) { - applicationComponents, err := a.loader.GetAllApplicationComponents(a.context, a.client, application) - if err != nil { - return nil, err - } - - snapshot, err := gitops.PrepareSnapshot(a.context, a.client, application, applicationComponents, component, newContainerImage, newComponentSource) - if err != nil { - return nil, err - } - - if snapshot.Labels == nil { - snapshot.Labels = map[string]string{} - } - snapshot.Labels[gitops.SnapshotTypeLabel] = gitops.SnapshotCompositeType - - return snapshot, nil -} - -// createCompositeSnapshotsIfConflictExists checks if the component Snapshot is good to release by checking if any -// of the other components containerImages changed in the meantime. If any of them changed, it creates a new composite snapshot. -func (a *Adapter) createCompositeSnapshotsIfConflictExists(application *applicationapiv1alpha1.Application, component *applicationapiv1alpha1.Component, testedSnapshot *applicationapiv1alpha1.Snapshot) (*applicationapiv1alpha1.Snapshot, error) { - newContainerImage, err := a.getImagePullSpecFromSnapshotComponent(testedSnapshot, component) - if err != nil { - return nil, err - } - - newComponentSource, err := a.getComponentSourceFromSnapshotComponent(testedSnapshot, component) - if err != nil { - return nil, err - } - - compositeSnapshot, err := a.prepareCompositeSnapshot(application, component, newContainerImage, newComponentSource) - if err != nil { - return nil, err - } - - gitops.CopySnapshotLabelsAndAnnotation(application, compositeSnapshot, component.Name, &testedSnapshot.ObjectMeta, gitops.PipelinesAsCodePrefix, true) - - // Create the new composite snapshot if it doesn't exist already - if !gitops.CompareSnapshots(compositeSnapshot, testedSnapshot) { - allSnapshots, err := a.loader.GetAllSnapshots(a.context, a.client, application) - if err != nil { - return nil, err - } - existingCompositeSnapshot := gitops.FindMatchingSnapshot(a.application, allSnapshots, compositeSnapshot) - - if existingCompositeSnapshot != nil { - a.logger.Info("Found existing composite Snapshot", - "snapshot.Name", existingCompositeSnapshot.Name, - "snapshot.Spec.Components", existingCompositeSnapshot.Spec.Components) - return existingCompositeSnapshot, nil - } else { - err = a.client.Create(a.context, compositeSnapshot) - if err != nil { - return nil, err - } - go metrics.RegisterNewSnapshot() - a.logger.LogAuditEvent("CompositeSnapshot created", compositeSnapshot, helpers.LogActionAdd, - "snapshot.Spec.Components", compositeSnapshot.Spec.Components) - return compositeSnapshot, nil - } - } - - return nil, nil -} - -// getImagePullSpecFromSnapshotComponent gets the full image pullspec from the given Snapshot Component, -func (a *Adapter) getImagePullSpecFromSnapshotComponent(snapshot *applicationapiv1alpha1.Snapshot, component *applicationapiv1alpha1.Component) (string, error) { - for _, snapshotComponent := range snapshot.Spec.Components { - if snapshotComponent.Name == component.Name { - return snapshotComponent.ContainerImage, nil - } - } - return "", fmt.Errorf("couldn't find the requested component info in the given Snapshot") -} - -// getComponentSourceFromSnapshotComponent gets the component source from the given Snapshot for the given Component, -func (a *Adapter) getComponentSourceFromSnapshotComponent(snapshot *applicationapiv1alpha1.Snapshot, component *applicationapiv1alpha1.Component) (*applicationapiv1alpha1.ComponentSource, error) { - for _, snapshotComponent := range snapshot.Spec.Components { - if snapshotComponent.Name == component.Name { - return &snapshotComponent.Source, nil - } - } - return nil, fmt.Errorf("couldn't find the requested component source info in the given Snapshot") -} - // findUntriggeredIntegrationTestFromStatus returns name of integrationTestScenario that is not triggered yet. func (a *Adapter) findUntriggeredIntegrationTestFromStatus(integrationTestScenarios *[]v1beta2.IntegrationTestScenario, testStatuses *intgteststat.SnapshotIntegrationTestStatuses) string { for _, integrationTestScenario := range *integrationTestScenarios { diff --git a/internal/controller/statusreport/statusreport_adapter_test.go b/internal/controller/statusreport/statusreport_adapter_test.go index e0726652c..d24970fb0 100644 --- a/internal/controller/statusreport/statusreport_adapter_test.go +++ b/internal/controller/statusreport/statusreport_adapter_test.go @@ -20,7 +20,6 @@ import ( "bytes" "fmt" "reflect" - "time" "github.com/konflux-ci/integration-service/api/v1beta2" "github.com/tonglil/buflogr" @@ -41,7 +40,6 @@ import ( "github.com/konflux-ci/integration-service/gitops" "github.com/konflux-ci/integration-service/helpers" "k8s.io/apimachinery/pkg/api/errors" - "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe("Snapshot Adapter", Ordered, func() { @@ -385,13 +383,6 @@ var _ = Describe("Snapshot Adapter", Ordered, func() { }) - It("ensures the global component list unchanged and compositeSnapshot shouldn't be created ", func() { - // Check if the global component list changed in the meantime and create a composite snapshot if it did. - compositeSnapshot, err := adapter.createCompositeSnapshotsIfConflictExists(hasApp, hasComp, hasSnapshot) - Expect(err).To(BeNil()) - Expect(compositeSnapshot).To(BeNil()) - }) - }) When("New Adapter is created for a push-type Snapshot that failed one of the tests", func() { @@ -542,95 +533,5 @@ var _ = Describe("Snapshot Adapter", Ordered, func() { }) }) - It("ensures the component Snapshot is marked as invalid when the global component list is changed", func() { - result, err := adapter.EnsureSnapshotFinishedAllTests() - Expect(!result.RequeueRequest && !result.CancelRequest && err == nil).To(BeTrue()) - - condition := meta.FindStatusCondition(hasSnapshot.Status.Conditions, gitops.AppStudioIntegrationStatusCondition) - Expect(condition.Status).NotTo(BeNil()) - Expect(condition.Status).To(Equal(metav1.ConditionFalse)) - Expect(condition.Reason).To(Equal("Invalid")) - - expectedLogEntry := "The global component list has changed in the meantime, marking snapshot as Invalid" - Expect(buf.String()).Should(ContainSubstring(expectedLogEntry)) - expectedLogEntry = "CompositeSnapshot created" - Expect(buf.String()).Should(ContainSubstring(expectedLogEntry)) - - compositeSnapshots := &applicationapiv1alpha1.SnapshotList{} - opts := []client.ListOption{ - client.InNamespace(hasApp.Namespace), - client.MatchingLabels{ - gitops.SnapshotTypeLabel: gitops.SnapshotCompositeType, - }, - } - Eventually(func() error { - if err := k8sClient.List(adapter.context, compositeSnapshots, opts...); err != nil { - return err - } - - if expected, got := 1, len(compositeSnapshots.Items); expected != got { - return fmt.Errorf("found %d composite Snapshots, expected: %d", got, expected) - } - return nil - }, time.Second*10).Should(BeNil()) - - compositeSnapshot := &compositeSnapshots.Items[0] - Expect(compositeSnapshot).NotTo(BeNil()) - - // Check if the composite snapshot that was already created above was correctly detected and returned. - adapter.context = toolkit.GetMockedContext(ctx, []toolkit.MockData{ - { - ContextKey: loader.ApplicationContextKey, - Resource: hasApp, - }, - { - ContextKey: loader.ComponentContextKey, - Resource: hasComp, - }, - { - ContextKey: loader.SnapshotContextKey, - Resource: hasSnapshot, - }, - { - ContextKey: loader.ApplicationComponentsContextKey, - Resource: []applicationapiv1alpha1.Component{*hasComp, *hasComp2}, - }, - { - ContextKey: loader.AllSnapshotsContextKey, - Resource: []applicationapiv1alpha1.Snapshot{*compositeSnapshot}, - }, - }) - existingCompositeSnapshot, err := adapter.createCompositeSnapshotsIfConflictExists(hasApp, hasComp, hasSnapshot) - Expect(err).To(BeNil()) - Expect(existingCompositeSnapshot).NotTo(BeNil()) - Expect(existingCompositeSnapshot.Name).To(Equal(compositeSnapshot.Name)) - - expectedLogEntry = "Found existing composite Snapshot" - Expect(buf.String()).Should(ContainSubstring(expectedLogEntry)) - - componentSource, _ := adapter.getComponentSourceFromSnapshotComponent(existingCompositeSnapshot, hasComp2) - Expect(componentSource.GitSource.Revision).To(Equal("lastbuiltcommit")) - - componentImagePullSpec, _ := adapter.getImagePullSpecFromSnapshotComponent(existingCompositeSnapshot, hasComp2) - Expect(componentImagePullSpec).To(Equal(SampleImage)) - }) - - It("ensures that Labels and Annotations were coppied to composite snapshot from PR snapshot", func() { - copyToCompositeSnapshot, err := adapter.createCompositeSnapshotsIfConflictExists(hasApp, hasComp, hasSnapshot) - - Expect(err).ToNot(HaveOccurred()) - Expect(copyToCompositeSnapshot).NotTo(BeNil()) - - gitops.CopySnapshotLabelsAndAnnotation(hasApp, copyToCompositeSnapshot, hasComp.Name, &hasPRSnapshot.ObjectMeta, gitops.PipelinesAsCodePrefix, true) - Expect(copyToCompositeSnapshot.Labels[gitops.SnapshotTypeLabel]).To(Equal(gitops.SnapshotCompositeType)) - Expect(copyToCompositeSnapshot.Labels[gitops.SnapshotComponentLabel]).To(Equal(hasComp.Name)) - Expect(copyToCompositeSnapshot.Labels[gitops.ApplicationNameLabel]).To(Equal(hasApp.Name)) - Expect(copyToCompositeSnapshot.Labels["pac.test.appstudio.openshift.io/url-org"]).To(Equal("testorg")) - Expect(copyToCompositeSnapshot.Labels["pac.test.appstudio.openshift.io/url-repository"]).To(Equal("testrepo")) - Expect(copyToCompositeSnapshot.Labels["pac.test.appstudio.openshift.io/sha"]).To(Equal("testsha")) - - Expect(copyToCompositeSnapshot.Annotations[gitops.PipelineAsCodeInstallationIDAnnotation]).To(Equal("123")) - - }) }) })