Skip to content

Commit

Permalink
Fix bug with ImageStreams in relatedObjects (kubevirt#3061)
Browse files Browse the repository at this point in the history
When an ImageStream is deleted for any reason, it stays in the
relatedObjects list in the HypeConverged Status.

This PR fixes this issue, and remove the deleted imageStreams from the
relatedObject list.

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
  • Loading branch information
nunnatsa authored Aug 21, 2024
1 parent e411afd commit 55d0ab9
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 4 deletions.
11 changes: 11 additions & 0 deletions controllers/operands/imageStream.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,17 @@ func (iso imageStreamOperand) deleteImageStream(req *common.HcoRequest) *EnsureR
if err != nil {
return NewEnsureResult(h.required).Error(fmt.Errorf("failed to delete imagestream %s/%s; %w", h.required.Namespace, h.required.Name, err))
}

objectRef, err := reference.GetReference(iso.operand.Scheme, h.required)
if err != nil {
return NewEnsureResult(req.Instance).Error(err)
}

if err = objectreferencesv1.RemoveObjectReference(&req.Instance.Status.RelatedObjects, *objectRef); err != nil {
return NewEnsureResult(req.Instance).Error(err)
}
req.StatusDirty = true

return nil
}

Expand Down
37 changes: 33 additions & 4 deletions controllers/operands/imageStream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ var _ = Describe("imageStream tests", func() {
ObjectMeta: metav1.ObjectMeta{
Name: "test-image-stream",
Namespace: "test-image-stream-ns",
UID: types.UID("0987654321"),
},

Spec: imagev1.ImageStreamSpec{
Expand All @@ -101,6 +102,10 @@ var _ = Describe("imageStream tests", func() {
}
exists.Labels = getLabels(hco, util.AppComponentCompute)

ref, err := reference.GetReference(commontestutils.GetScheme(), exists)
Expect(err).ToNot(HaveOccurred())
Expect(objectreferencesv1.SetObjectReference(&hco.Status.RelatedObjects, *ref)).To(Succeed())

cli := commontestutils.InitClient([]client.Object{exists})
handlers, err := getImageStreamHandlers(logger, cli, schemeForTest, hco)
Expect(err).ToNot(HaveOccurred())
Expand All @@ -115,6 +120,10 @@ var _ = Describe("imageStream tests", func() {
imageStreamObjects := &imagev1.ImageStreamList{}
Expect(cli.List(context.TODO(), imageStreamObjects)).To(Succeed())
Expect(imageStreamObjects.Items).To(BeEmpty())

newRef, err := objectreferencesv1.FindObjectReference(hco.Status.RelatedObjects, *ref)
Expect(err).ToNot(HaveOccurred())
Expect(newRef).To(BeNil())
})

It("should delete the ImageStream resource if the FG is not set, and emit event", func() {
Expand Down Expand Up @@ -770,10 +779,6 @@ var _ = Describe("imageStream tests", func() {
return testFilesLocation
}

getImageStreamFileLocation = func() string {
return testFilesLocation
}

By("create imagestream in the default namespace")
hco := commontestutils.NewHco()
hco.Spec.FeatureGates.EnableCommonBootImageImport = ptr.To(true)
Expand All @@ -794,10 +799,14 @@ var _ = Describe("imageStream tests", func() {
Expect(ImageStreamObjects.Items[0].Name).To(Equal("test-image-stream"))
Expect(ImageStreamObjects.Items[0].Namespace).To(Equal("test-image-stream-ns"))

ref, err := reference.GetReference(commontestutils.GetScheme(), &ImageStreamObjects.Items[0])
Expect(err).ToNot(HaveOccurred())

By("replace the image stream with a new one in the custom namespace")
hco = commontestutils.NewHco()
hco.Spec.FeatureGates.EnableCommonBootImageImport = ptr.To(true)
hco.Spec.CommonBootImageNamespace = ptr.To(customNS)
Expect(objectreferencesv1.SetObjectReference(&hco.Status.RelatedObjects, *ref)).To(Succeed())

req = commontestutils.NewReq(hco)
res = handlers[0].ensure(req)
Expand All @@ -809,6 +818,10 @@ var _ = Describe("imageStream tests", func() {
Expect(ImageStreamObjects.Items).To(HaveLen(1))
Expect(ImageStreamObjects.Items[0].Name).To(Equal("test-image-stream"))
Expect(ImageStreamObjects.Items[0].Namespace).To(Equal(customNS))

newRef, err := objectreferencesv1.FindObjectReference(hco.Status.RelatedObjects, *ref)
Expect(err).ToNot(HaveOccurred())
Expect(newRef).To(BeNil())
})

It("should remove an imagestream from a custom namespace, and create it in the default one", func() {
Expand Down Expand Up @@ -842,9 +855,13 @@ var _ = Describe("imageStream tests", func() {
Expect(ImageStreamObjects.Items[0].Name).To(Equal("test-image-stream"))
Expect(ImageStreamObjects.Items[0].Namespace).To(Equal(customNS))

ref, err := reference.GetReference(commontestutils.GetScheme(), &ImageStreamObjects.Items[0])
Expect(err).ToNot(HaveOccurred())

By("replace the image stream with a new one in the default namespace")
hco = commontestutils.NewHco()
hco.Spec.FeatureGates.EnableCommonBootImageImport = ptr.To(true)
Expect(objectreferencesv1.SetObjectReference(&hco.Status.RelatedObjects, *ref)).To(Succeed())

req = commontestutils.NewReq(hco)
res = handlers[0].ensure(req)
Expand All @@ -856,6 +873,10 @@ var _ = Describe("imageStream tests", func() {
Expect(ImageStreamObjects.Items).To(HaveLen(1))
Expect(ImageStreamObjects.Items[0].Name).To(Equal("test-image-stream"))
Expect(ImageStreamObjects.Items[0].Namespace).To(Equal("test-image-stream-ns"))

newRef, err := objectreferencesv1.FindObjectReference(hco.Status.RelatedObjects, *ref)
Expect(err).ToNot(HaveOccurred())
Expect(newRef).To(BeNil())
})

It("should remove an imagestream from a custom namespace, and create it in the new custom namespace", func() {
Expand Down Expand Up @@ -889,10 +910,14 @@ var _ = Describe("imageStream tests", func() {
Expect(ImageStreamObjects.Items[0].Name).To(Equal("test-image-stream"))
Expect(ImageStreamObjects.Items[0].Namespace).To(Equal(customNS))

ref, err := reference.GetReference(commontestutils.GetScheme(), &ImageStreamObjects.Items[0])
Expect(err).ToNot(HaveOccurred())

By("replace the image stream with a new one in another custom namespace")
hco = commontestutils.NewHco()
hco.Spec.FeatureGates.EnableCommonBootImageImport = ptr.To(true)
hco.Spec.CommonBootImageNamespace = ptr.To(customNS + "1")
Expect(objectreferencesv1.SetObjectReference(&hco.Status.RelatedObjects, *ref)).To(Succeed())

req = commontestutils.NewReq(hco)
res = handlers[0].ensure(req)
Expand All @@ -904,6 +929,10 @@ var _ = Describe("imageStream tests", func() {
Expect(ImageStreamObjects.Items).To(HaveLen(1))
Expect(ImageStreamObjects.Items[0].Name).To(Equal("test-image-stream"))
Expect(ImageStreamObjects.Items[0].Namespace).To(Equal(customNS + "1"))

newRef, err := objectreferencesv1.FindObjectReference(hco.Status.RelatedObjects, *ref)
Expect(err).ToNot(HaveOccurred())
Expect(newRef).To(BeNil())
})
})
})
Expand Down

0 comments on commit 55d0ab9

Please sign in to comment.