Skip to content

Commit

Permalink
chore(stoneintg-1008): stop referenciing spec.containerImage
Browse files Browse the repository at this point in the history
The integration service has begun using status.lastPromotedImage
as the source of truth for the GCL. Initially the spec.containerImage
field was used as a fallback until components could migrate to the new
field.  Now that all elligible components have been manually migrated
to the new field, we can remove references to the old field in our code.

This will end the errors caused by the integration and build services
both attempting to write to spec.containerImage.

Signed-off-by: Ryan Cole <[email protected]>
  • Loading branch information
14rcole committed Dec 11, 2024
1 parent 392d2aa commit 855052b
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 48 deletions.
2 changes: 1 addition & 1 deletion docs/snapshot-controller.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ flowchart TD
%% Node definitions
ensure2(Process further if: <br>Snapshot was not created by PAC Pull Request Event OR <br>an override snapshot was created <br>& Snapshot wasn't added to Global Candidate List)
update_container_image("<b>Get</b> all components of snapshot and <br><b>Update</b> the '.spec.containerImage' field of the <br>component with the latest value, taken from <br>given Snapshot's .spec.components[x].containerImage field")
update_container_image("<b>Get</b> all components of snapshot and <br><b>Update</b> the '.status.lastPromotedImage' field of the <br>component with the latest value, taken from <br>given Snapshot's .spec.components[x].containerImage field")
update_last_built_commit("<b>Update</b> the '.status.lastBuiltCommit' field of the given <br>component with the latest value, taken from <br>given Snapshot's .spec.components[x].source.git.revision field")
mark_snapshot_added_to_GCL(<b>Mark</b> the Snapshot as AddedToGlobalCandidateList)
continue_processing2(Controller continues processing...)
Expand Down
3 changes: 0 additions & 3 deletions gitops/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,9 +756,6 @@ func PrepareSnapshot(ctx context.Context, adapterClient client.Client, applicati
for _, applicationComponent := range *applicationComponents {
applicationComponent := applicationComponent // G601
containerImage := applicationComponent.Status.LastPromotedImage
if containerImage == "" {
containerImage = applicationComponent.Spec.ContainerImage
}

var componentSource *applicationapiv1alpha1.ComponentSource
if applicationComponent.Name == component.Name {
Expand Down
2 changes: 1 addition & 1 deletion helpers/errorhandlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func IsInvalidImageDigestError(err error) bool {
func NewMissingValidComponentError(componentName string) error {
return &IntegrationError{
Reason: ReasonMissingValidComponentError,
Message: fmt.Sprintf("The only one component %s is invalid, valid .Spec.ContainerImage is missing", componentName),
Message: fmt.Sprintf("The only one component %s is invalid, valid .Status.LastPromotedImage is missing", componentName),
}
}

Expand Down
2 changes: 1 addition & 1 deletion helpers/errorhandlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ var _ = Describe("Helpers for error handlers", Ordered, func() {
It("Can define MissingValidComponentError", func() {
err := helpers.NewMissingValidComponentError("componentName")
Expect(helpers.IsMissingValidComponentError(err)).To(BeTrue())
Expect(err.Error()).To(Equal("The only one component componentName is invalid, valid .Spec.ContainerImage is missing"))
Expect(err.Error()).To(Equal("The only one component componentName is invalid, valid .Status.LastPromotedImage is missing"))
})

It("Can handle non integration error", func() {
Expand Down
3 changes: 0 additions & 3 deletions internal/controller/component/component_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@ func (a *Adapter) EnsureComponentIsCleanedUp() (controller.OperationResult, erro
component := individualComponent
if a.component.Name != component.Name {
containerImage := component.Status.LastPromotedImage
if containerImage == "" {
containerImage = component.Spec.ContainerImage
}
componentSource := gitops.GetComponentSourceFromComponent(&component)
snapshotComponents = append(snapshotComponents, applicationapiv1alpha1.SnapshotComponent{
Name: component.Name,
Expand Down
30 changes: 1 addition & 29 deletions internal/controller/snapshot/snapshot_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,6 @@ func (a *Adapter) EnsureGlobalCandidateImageUpdated() (controller.OperationResul
if err != nil {
return controller.RequeueWithError(err)
}
//update .Spec.ContainerImage for the component included in component snapshot
err = a.updateComponentContainerImage(a.context, a.client, componentToUpdate, &snapshotComponent)
if err != nil {
return controller.RequeueWithError(err)
}

// update .Status.LastBuiltCommit for the component included in component snapshot
err = a.updateComponentSource(a.context, a.client, componentToUpdate, &snapshotComponent)
Expand All @@ -393,7 +388,7 @@ func (a *Adapter) EnsureGlobalCandidateImageUpdated() (controller.OperationResul
}
}
} else if gitops.IsOverrideSnapshot(a.snapshot) {
// update Spec.ContainerImage for each component in override snapshot
// update Status.LastPromotedImage for each component in override snapshot
for _, snapshotComponent := range a.snapshot.Spec.Components {
snapshotComponent := snapshotComponent //G601
// get component for each snapshotComponent in override snapshot
Expand All @@ -419,11 +414,6 @@ func (a *Adapter) EnsureGlobalCandidateImageUpdated() (controller.OperationResul
if err != nil {
return controller.RequeueWithError(err)
}
//update component.Spec.ContainerImage for each snapshotComponent in override snapshot
err = a.updateComponentContainerImage(a.context, a.client, componentToUpdate, &snapshotComponent)
if err != nil {
return controller.RequeueWithError(err)
}
// update .Status.LastBuiltCommit for each snapshotComponent in override snapshot
err = a.updateComponentSource(a.context, a.client, componentToUpdate, &snapshotComponent)
if err != nil {
Expand Down Expand Up @@ -845,21 +835,6 @@ func (a *Adapter) HandlePipelineCreationError(err error, integrationTestScenario
return controller.RequeueWithError(err)
}

func (a *Adapter) updateComponentContainerImage(ctx context.Context, c client.Client, component *applicationapiv1alpha1.Component, snapshotComponent *applicationapiv1alpha1.SnapshotComponent) error {
patch := client.MergeFrom(component.DeepCopy())
component.Spec.ContainerImage = snapshotComponent.ContainerImage
err := a.client.Patch(a.context, component, patch)
if err != nil {
a.logger.Error(err, "Failed to update .Spec.ContainerImage of Global Candidate for the Component",
"component.Name", component.Name)
return err
}
a.logger.LogAuditEvent("Updated .Spec.ContainerImage of Global Candidate for the Component",
component, h.LogActionUpdate,
"containerImage", snapshotComponent.ContainerImage)
return nil
}

func (a *Adapter) updateComponentSource(ctx context.Context, c client.Client, component *applicationapiv1alpha1.Component, snapshotComponent *applicationapiv1alpha1.SnapshotComponent) error {
if reflect.ValueOf(snapshotComponent.Source).IsValid() && snapshotComponent.Source.GitSource != nil && snapshotComponent.Source.GitSource.Revision != "" {
patch := client.MergeFrom(component.DeepCopy())
Expand Down Expand Up @@ -941,9 +916,6 @@ func (a *Adapter) prepareGroupSnapshot(application *applicationapiv1alpha1.Appli
// if there is no component snapshot found for open PR/MR, we get snapshotComponent from gcl
componentSource := gitops.GetComponentSourceFromComponent(&applicationComponent)
containerImage := applicationComponent.Status.LastPromotedImage
if containerImage == "" {
containerImage = applicationComponent.Spec.ContainerImage
}
if containerImage == "" {
a.logger.Info("component cannot be added to snapshot for application due to missing containerImage", "component.Name", applicationComponent.Name)
continue
Expand Down
12 changes: 2 additions & 10 deletions internal/controller/snapshot/snapshot_adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,6 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
return !result.CancelRequest && err == nil
}, time.Second*10).Should(BeTrue())

Expect(hasComp.Spec.ContainerImage).To(Equal(""))
Expect(hasComp.Status.LastBuiltCommit).To(Equal(""))
})

Expand All @@ -776,9 +775,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
Expect(err).ShouldNot(HaveOccurred())
Expect(result.CancelRequest).To(BeFalse())

expectedLogEntry := "Updated .Spec.ContainerImage of Global Candidate for the Component"
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
expectedLogEntry = "Updated .Status.LastBuiltCommit of Global Candidate for the Component"
expectedLogEntry := "Updated .Status.LastBuiltCommit of Global Candidate for the Component"
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
})

Expand All @@ -797,9 +794,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
Expect(err).ShouldNot(HaveOccurred())
Expect(result.CancelRequest).To(BeFalse())

expectedLogEntry := "Updated .Spec.ContainerImage of Global Candidate for the Component"
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
expectedLogEntry = "Updated .Status.LastBuiltCommit of Global Candidate for the Component"
expectedLogEntry := "Updated .Status.LastBuiltCommit of Global Candidate for the Component"
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
expectedLogEntry = "Updated .Status.LastPromotedImage of Global Candidate for the Component"
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
Expand Down Expand Up @@ -842,8 +837,6 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
result, err = adapter.EnsureGlobalCandidateImageUpdated()
Expect(err).ShouldNot(HaveOccurred())
Expect(result.CancelRequest).To(BeFalse())
expectedLogEntry = "Updated .Spec.ContainerImage of Global Candidate for the Component"
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
expectedLogEntry = "Updated .Status.LastBuiltCommit of Global Candidate for the Component"
Expect(buf.String()).Should(ContainSubstring(expectedLogEntry))
// don't update Global Candidate List for the component included in a override snapshot but doesn't existw
Expand Down Expand Up @@ -2273,7 +2266,6 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
Expect(result.CancelRequest).To(BeFalse())
Expect(result.RequeueRequest).To(BeFalse())
Expect(buf.String()).Should(ContainSubstring("component cannot be added to snapshot for application due to invalid digest in containerImage"))
Expect(buf.String()).Should(ContainSubstring("component with containerImage from Global Candidate List will be added to group snapshot"))
Expect(err).NotTo(HaveOccurred())
Eventually(func() bool {
_ = k8sClient.Get(adapter.context, types.NamespacedName{
Expand Down

0 comments on commit 855052b

Please sign in to comment.