From c86af27bedd24d4f336c37bbd56663a5e9713177 Mon Sep 17 00:00:00 2001 From: Michael Henriksen Date: Fri, 19 Jan 2024 17:07:19 -0500 Subject: [PATCH] GetActiveCDI() should return success if single resource in error state Signed-off-by: Michael Henriksen --- WORKSPACE | 12 ++--- pkg/controller/common/BUILD.bazel | 1 + pkg/controller/common/util.go | 16 ++++--- pkg/controller/common/util_test.go | 64 +++++++++++++++++++++++++++ pkg/operator/controller/cr-manager.go | 2 +- rpm/BUILD.bazel | 4 +- 6 files changed, 84 insertions(+), 15 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 77d80812d4..bca9dafb7c 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -606,15 +606,15 @@ rpm( ) rpm( - name = "crun-0__1.11.2-1.el9.aarch64", - sha256 = "035153ee04d00def11428c1440eb35b9f7ff3251a18649685e8d078e6266d3b5", - urls = ["http://mirror.stream.centos.org/9-stream/AppStream/aarch64/os/Packages/crun-1.11.2-1.el9.aarch64.rpm"], + name = "crun-0__1.12-1.el9.aarch64", + sha256 = "a2d1a41b79d5da007aa0ebcbca2d2eec8e018cdab7474954ac0168b5c53ed8af", + urls = ["https://mirror.stream.centos.org/9-stream/AppStream/aarch64/os/Packages/crun-1.12-1.el9.aarch64.rpm"], ) rpm( - name = "crun-0__1.11.2-1.el9.x86_64", - sha256 = "0f6f281969df7c2aa7c8d705d4072344f15a035634e0561d15000dc5a0edb46a", - urls = ["http://mirror.stream.centos.org/9-stream/AppStream/x86_64/os/Packages/crun-1.11.2-1.el9.x86_64.rpm"], + name = "crun-0__1.12-1.el9.x86_64", + sha256 = "08bf93254eee3c9a612f77f0fecd58af30bf096ac7589ec797b4c6fb57e64764", + urls = ["https://mirror.stream.centos.org/9-stream/AppStream/x86_64/os/Packages/crun-1.12-1.el9.x86_64.rpm"], ) rpm( diff --git a/pkg/controller/common/BUILD.bazel b/pkg/controller/common/BUILD.bazel index ad259b06cf..0b1a37f9d1 100644 --- a/pkg/controller/common/BUILD.bazel +++ b/pkg/controller/common/BUILD.bazel @@ -53,6 +53,7 @@ go_test( "//vendor/github.com/onsi/gomega:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/kubevirt.io/controller-lifecycle-operator-sdk/api:go_default_library", "//vendor/sigs.k8s.io/controller-runtime/pkg/log:go_default_library", "//vendor/sigs.k8s.io/controller-runtime/pkg/log/zap:go_default_library", ], diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go index b08181608d..57e4dd4cf4 100644 --- a/pkg/controller/common/util.go +++ b/pkg/controller/common/util.go @@ -507,6 +507,14 @@ func GetActiveCDI(c client.Client) (*cdiv1.CDI, error) { return nil, err } + if len(crList.Items) == 0 { + return nil, nil + } + + if len(crList.Items) == 1 { + return &crList.Items[0], nil + } + var activeResources []cdiv1.CDI for _, cr := range crList.Items { if cr.Status.Phase != sdkapi.PhaseError { @@ -514,12 +522,8 @@ func GetActiveCDI(c client.Client) (*cdiv1.CDI, error) { } } - if len(activeResources) == 0 { - return nil, nil - } - - if len(activeResources) > 1 { - return nil, fmt.Errorf("number of active CDI CRs > 1") + if len(activeResources) != 1 { + return nil, fmt.Errorf("invalid number of active CDI resources: %d", len(activeResources)) } return &activeResources[0], nil diff --git a/pkg/controller/common/util_test.go b/pkg/controller/common/util_test.go index 8dc5746ccd..c186ab0508 100644 --- a/pkg/controller/common/util_test.go +++ b/pkg/controller/common/util_test.go @@ -7,6 +7,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" + sdkapi "kubevirt.io/controller-lifecycle-operator-sdk/api" ) var _ = Describe("GetRequestedImageSize", func() { @@ -73,6 +74,69 @@ var _ = Describe("GetDefaultStorageClass", func() { sc, _ := GetDefaultStorageClass(client) Expect(sc).To(BeNil()) }) + + Context("GetActiveCDI tests", func() { + createCDI := func(name string, phase sdkapi.Phase) *cdiv1.CDI { + return &cdiv1.CDI{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Status: cdiv1.CDIStatus{ + Status: sdkapi.Status{ + Phase: phase, + }, + }, + } + } + + It("Should return nil if no CDI", func() { + client := CreateClient() + cdi, err := GetActiveCDI(client) + Expect(err).ToNot(HaveOccurred()) + Expect(cdi).To(BeNil()) + }) + + It("Should return single active", func() { + client := CreateClient( + createCDI("cdi1", sdkapi.PhaseDeployed), + ) + cdi, err := GetActiveCDI(client) + Expect(err).ToNot(HaveOccurred()) + Expect(cdi).ToNot(BeNil()) + }) + + It("Should return success with single active one error", func() { + client := CreateClient( + createCDI("cdi1", sdkapi.PhaseDeployed), + createCDI("cdi2", sdkapi.PhaseError), + ) + cdi, err := GetActiveCDI(client) + Expect(err).ToNot(HaveOccurred()) + Expect(cdi).ToNot(BeNil()) + Expect(cdi.Name).To(Equal("cdi1")) + }) + + It("Should return error if multiple CDIs are active", func() { + client := CreateClient( + createCDI("cdi1", sdkapi.PhaseDeployed), + createCDI("cdi2", sdkapi.PhaseDeployed), + ) + cdi, err := GetActiveCDI(client) + Expect(err).To(HaveOccurred()) + Expect(cdi).To(BeNil()) + }) + + It("Should return error if multiple CDIs are error", func() { + client := CreateClient( + createCDI("cdi1", sdkapi.PhaseError), + createCDI("cdi2", sdkapi.PhaseError), + ) + cdi, err := GetActiveCDI(client) + Expect(err).To(HaveOccurred()) + Expect(cdi).To(BeNil()) + }) + + }) }) func createPvcNoSize(name, ns string, annotations, labels map[string]string) *v1.PersistentVolumeClaim { diff --git a/pkg/operator/controller/cr-manager.go b/pkg/operator/controller/cr-manager.go index cbdfe9854a..d5c97805ca 100644 --- a/pkg/operator/controller/cr-manager.go +++ b/pkg/operator/controller/cr-manager.go @@ -71,7 +71,7 @@ func (r *ReconcileCDI) GetDependantResourcesListObjects() []client.ObjectList { func (r *ReconcileCDI) IsCreating(_ client.Object) (bool, error) { configMap, err := r.getConfigMap() if err != nil { - return true, nil + return false, err } return configMap == nil, nil } diff --git a/rpm/BUILD.bazel b/rpm/BUILD.bazel index b75d0a00a6..7ac3b99364 100644 --- a/rpm/BUILD.bazel +++ b/rpm/BUILD.bazel @@ -800,7 +800,7 @@ rpmtree( "@coreutils-single-0__8.32-34.el9.aarch64//rpm", "@cracklib-0__2.9.6-27.el9.aarch64//rpm", "@cracklib-dicts-0__2.9.6-27.el9.aarch64//rpm", - "@crun-0__1.11.2-1.el9.aarch64//rpm", + "@crun-0__1.12-1.el9.aarch64//rpm", "@crypto-policies-0__20231113-1.gite9247c2.el9.aarch64//rpm", "@crypto-policies-scripts-0__20231113-1.gite9247c2.el9.aarch64//rpm", "@curl-0__7.76.1-28.el9.aarch64//rpm", @@ -942,7 +942,7 @@ rpmtree( "@coreutils-single-0__8.32-34.el9.x86_64//rpm", "@cracklib-0__2.9.6-27.el9.x86_64//rpm", "@cracklib-dicts-0__2.9.6-27.el9.x86_64//rpm", - "@crun-0__1.11.2-1.el9.x86_64//rpm", + "@crun-0__1.12-1.el9.x86_64//rpm", "@crypto-policies-0__20231113-1.gite9247c2.el9.x86_64//rpm", "@crypto-policies-scripts-0__20231113-1.gite9247c2.el9.x86_64//rpm", "@curl-0__7.76.1-28.el9.x86_64//rpm",