Skip to content

Commit 5a5e1b2

Browse files
authored
Merge pull request #1471 from hongweiliu17/STONEINTG-1501
fix(STONEINTG-1501): remove updating GCL from component snapshot
2 parents 0d5df4f + 91f5144 commit 5a5e1b2

4 files changed

Lines changed: 11 additions & 75 deletions

File tree

gitops/snapshot.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1505,7 +1505,11 @@ func UpdateComponentImageAndSource(ctx context.Context, adapterClient client.Cli
15051505
buildTimeStr = strconv.FormatInt(pr.Status.StartTime.Unix(), 10)
15061506
}
15071507
}
1508-
if _, ok := object.(*applicationapiv1alpha1.Snapshot); ok {
1508+
// set the last build time annotation according to the time.Now() when the GCL is updated by override snapshot not build pipelinerun
1509+
if snapshot, ok := object.(*applicationapiv1alpha1.Snapshot); ok {
1510+
if !IsOverrideSnapshot(snapshot) {
1511+
return fmt.Errorf("snapshot %s/%s is not an override snapshot, GCL can be updated from build pipelinerun for push event or override snapshot only", snapshot.Namespace, snapshot.Name)
1512+
}
15091513
buildTimeStr = strconv.FormatInt(time.Now().Unix(), 10)
15101514
}
15111515
return AnnotateComponent(ctx, component, BuildPipelineLastBuiltTime, buildTimeStr, adapterClient)

gitops/snapshot_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,7 @@ var _ = Describe("Gitops functions for managing Snapshots", Ordered, func() {
898898
})
899899

900900
It("Ensure UpdateComponentImageAndSource can update component containerImage and source", func() {
901+
hasSnapshot.Labels[gitops.SnapshotTypeLabel] = "override"
901902
componentSource := applicationapiv1alpha1.ComponentSource{
902903
ComponentSourceUnion: applicationapiv1alpha1.ComponentSourceUnion{
903904
GitSource: &applicationapiv1alpha1.GitSource{

internal/controller/snapshot/snapshot_adapter.go

Lines changed: 3 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -432,18 +432,7 @@ func (a *Adapter) EnsureGlobalCandidateImageUpdated() (controller.OperationResul
432432
return controller.ContinueProcessing()
433433
}
434434

435-
var err error
436-
437-
if gitops.IsComponentSnapshot(a.snapshot) {
438-
if a.isSnapshotOlderThanLastBuild(a.snapshot) {
439-
a.logger.Info("The Glocal Candidate list was updated in newer build pipelinerun or snapshot")
440-
} else {
441-
err = a.updateGCLForComponentSnapshot()
442-
}
443-
} else if gitops.IsOverrideSnapshot(a.snapshot) {
444-
err = a.updateGCLForOverrideSnapshot()
445-
}
446-
435+
err := a.updateGCLForOverrideSnapshot()
447436
if err != nil {
448437
return controller.RequeueWithError(err)
449438
}
@@ -472,8 +461,8 @@ func (a *Adapter) EnsureGlobalCandidateImageUpdated() (controller.OperationResul
472461

473462
// shouldUpdateGlobalCandidateList checks if the snapshot should update the global candidate list
474463
func (a *Adapter) shouldUpdateGlobalCandidateList() bool {
475-
if !gitops.IsComponentSnapshotCreatedByPACPushEvent(a.snapshot) && !gitops.IsOverrideSnapshot(a.snapshot) {
476-
a.logger.Info("The Snapshot was neither created for a single component push event nor override type, not updating the global candidate list.")
464+
if !gitops.IsOverrideSnapshot(a.snapshot) {
465+
a.logger.Info("The Snapshot was not override type, will not update the global candidate list.")
477466
return false
478467
}
479468

@@ -486,34 +475,6 @@ func (a *Adapter) shouldUpdateGlobalCandidateList() bool {
486475
return true
487476
}
488477

489-
// updateGCLForComponentSnapshot updates global candidate list for component snapshots
490-
func (a *Adapter) updateGCLForComponentSnapshot() error {
491-
var componentToUpdate *applicationapiv1alpha1.Component
492-
var err error
493-
494-
err = retry.OnError(retry.DefaultRetry, func(_ error) bool { return true }, func() error {
495-
componentToUpdate, err = a.loader.GetComponentFromSnapshot(a.context, a.client, a.snapshot)
496-
return err
497-
})
498-
if err != nil {
499-
_, loaderError := h.HandleLoaderError(a.logger, err, fmt.Sprintf("Component or '%s' label", tektonconsts.ComponentNameLabel), "Snapshot")
500-
if loaderError != nil {
501-
return loaderError
502-
}
503-
return nil // Continue processing if no loader error
504-
}
505-
506-
// Find and update the matching snapshot component
507-
for _, snapshotComponent := range a.snapshot.Spec.Components {
508-
snapshotComponent := snapshotComponent //G601
509-
if snapshotComponent.Name == componentToUpdate.Name {
510-
return gitops.UpdateComponentImageAndSource(a.context, a.client, a.snapshot, componentToUpdate, snapshotComponent.Source, snapshotComponent.ContainerImage)
511-
}
512-
}
513-
514-
return nil
515-
}
516-
517478
// updateGCLForOverrideSnapshot handles updating global candidate list for override snapshots
518479
func (a *Adapter) updateGCLForOverrideSnapshot() error {
519480
for _, snapshotComponent := range a.snapshot.Spec.Components {

internal/controller/snapshot/snapshot_adapter_test.go

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,7 +1178,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
11781178
adapter.logger = log
11791179
adapter.snapshot = hasOverRideSnapshot
11801180

1181-
// don't update Global Candidate List for a snapshot if it is neither component snapshot nor override snapshot
1181+
// don't update Global Candidate List for a snapshot if it is not override snapshot
11821182
hasOverRideSnapshot.Labels[gitops.SnapshotTypeLabel] = ""
11831183
hasOverRideSnapshot.Spec.Components = []applicationapiv1alpha1.SnapshotComponent{
11841184
{
@@ -1202,7 +1202,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
12021202
Expect(err).ShouldNot(HaveOccurred())
12031203
Expect(result.CancelRequest).To(BeFalse())
12041204
Expect(result.RequeueRequest).To(BeFalse())
1205-
expectedLogEntry := "The Snapshot was neither created for a single component push event nor override type, not updating the global candidate list."
1205+
expectedLogEntry := "The Snapshot was not override type, will not update the global candidate list"
12061206
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
12071207

12081208
// update Glocal Candidate List for the component in a override snapshot
@@ -1217,36 +1217,6 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
12171217
expectedLogEntry = "containerImage cannot be updated to component Global Candidate List due to invalid digest in containerImage"
12181218
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
12191219
})
1220-
1221-
It("ensures global Component Image and lastPromotedImage updated when push snapshot is created", func() {
1222-
// ensure last promoted image is empty string
1223-
hasComp.Status.LastPromotedImage = ""
1224-
var buf bytes.Buffer
1225-
log := helpers.IntegrationLogger{Logger: buflogr.NewWithBuffer(&buf)}
1226-
adapter.logger = log
1227-
adapter.snapshot = hasSnapshot
1228-
Expect(gitops.IsSnapshotMarkedAsAddedToGlobalCandidateList(hasSnapshot)).To(BeFalse())
1229-
result, err := adapter.EnsureGlobalCandidateImageUpdated()
1230-
Expect(err).ShouldNot(HaveOccurred())
1231-
Expect(result.CancelRequest).To(BeFalse())
1232-
1233-
Eventually(func() bool {
1234-
err := k8sClient.Get(ctx, types.NamespacedName{
1235-
Name: hasSnapshot.Name,
1236-
Namespace: "default",
1237-
}, hasSnapshot)
1238-
return err == nil && gitops.IsSnapshotMarkedAsAddedToGlobalCandidateList(hasSnapshot)
1239-
}, time.Second*10).Should(BeTrue())
1240-
1241-
// Check if the adapter function detects that it already promoted the snapshot component
1242-
result, err = adapter.EnsureGlobalCandidateImageUpdated()
1243-
Expect(err).ShouldNot(HaveOccurred())
1244-
Expect(result.CancelRequest).To(BeFalse())
1245-
1246-
expectedLogEntry := "The Snapshot's component was previously added to the global candidate list, skipping adding it."
1247-
1248-
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
1249-
})
12501220
})
12511221

12521222
When("pull request updates repo with integration test", func() {

0 commit comments

Comments
 (0)