Skip to content

Commit

Permalink
Merge pull request #794 from jsztuka/STONE-961
Browse files Browse the repository at this point in the history 
feat(STONEINTG-961): remove composite snapshot
  • Loading branch information
@jsztuka
jsztuka authored Jul 4, 2024
2 parents 2f08ad5 + 3d51925 commit 904b28c
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 258 deletions.
8 changes: 1 addition & 7 deletions docs/statusreport-controller.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ flowchart TD
get_required_scenarios(Get all required <br> IntegrationTestScenarios)
parse_snapshot_status(Parse the Snapshot's <br> status annotation)
check_finished_tests{Did Snapshot <br> finish all required <br> integration tests?}
check_supersede{Does Snapshot need <br> to be superseded <br> with a composite Snapshot?}
check_passed_tests{Did Snapshot <br> pass all required <br> integration tests?}
create_snapshot(Create composite Snapshot)
update_status(Update Snapshot status accordingly)
continue_processing_tests(Controller continues processing)
Expand All @@ -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
Expand Down
13 changes: 3 additions & 10 deletions gitops/snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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{}
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/buildpipeline/buildpipeline_adapter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/buildpipeline/buildpipeline_adapter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
5 changes: 0 additions & 5 deletions internal/controller/component/component_adapter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
135 changes: 0 additions & 135 deletions internal/controller/statusreport/statusreport_adapter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,14 @@ 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"
"github.com/konflux-ci/integration-service/gitops"
"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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
99 changes: 0 additions & 99 deletions internal/controller/statusreport/statusreport_adapter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"bytes"
"fmt"
"reflect"
"time"

"github.com/konflux-ci/integration-service/api/v1beta2"
"github.com/tonglil/buflogr"
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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"))

})
})
})

0 comments on commit 904b28c

Please sign in to comment.