From 3e84c05378c0807aa4ae5ee1e3341f38be1015ae Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Tue, 7 Jul 2026 12:58:06 +0300 Subject: [PATCH 1/6] add the 'generate-crd' make target to create the CRD Signed-off-by: Nahshon Unna Tsameret --- Makefile | 22 +++++++++++++--------- hack/build-manifests.sh | 7 +------ 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index fe18e1385b..ed43ffe248 100644 --- a/Makefile +++ b/Makefile @@ -37,7 +37,7 @@ DO=eval export JOB_TYPE=prow endif -sanity: generate gogenerate gogenerate-crd-creator generate-doc validate-no-offensive-lang goimport lint-metrics lint-monitoring +sanity: generate gogenerate prepare-tools-crd generate-doc validate-no-offensive-lang goimport lint-metrics lint-monitoring go version go fmt ./... go mod tidy -v @@ -80,7 +80,7 @@ build-manifest-splitter: build-webhook: $(SOURCES) ## Build binary from source go build -ldflags="${LDFLAGS}" -o _out/hyperconverged-cluster-webhook ./cmd/hyperconverged-cluster-webhook -build-manifests: gogenerate-crd-creator build-crd-creator build-csv-merger build-manifest-splitter build-manifest-templator +build-manifests: prepare-tools-crd build-csv-merger build-manifest-splitter build-manifest-templator DUMP_NETWORK_POLICIES=$(DUMP_NETWORK_POLICIES) ./hack/build-manifests.sh build-manifests-prev: @@ -118,10 +118,10 @@ container-build: container-build-operator container-build-webhook container-buil build-push-multi-arch-images: build-push-multi-arch-operator-image build-push-multi-arch-webhook-image build-push-multi-arch-functest-image build-push-multi-arch-artifacts-server -container-build-operator: gogenerate gogenerate-crd-creator +container-build-operator: gogenerate prepare-tools-crd . "hack/cri-bin.sh" && $$CRI_BIN build --platform=linux/$(ARCH) -f build/Dockerfile -t $(IMAGE_REGISTRY)/$(OPERATOR_IMAGE):$(IMAGE_TAG) --build-arg git_sha=$(SHA) . -build-push-multi-arch-operator-image: gogenerate gogenerate-crd-creator +build-push-multi-arch-operator-image: gogenerate prepare-tools-crd IMAGE_NAME=$(IMAGE_REGISTRY)/$(OPERATOR_IMAGE):$(IMAGE_TAG) SHA=SHA DOCKER_FILE=build/Dockerfile ./hack/build-push-multi-arch-images.sh container-build-webhook: @@ -245,9 +245,13 @@ bump-kubevirtci: gogenerate: generate go generate ./pkg/upgradepatch -gogenerate-crd-creator: generate - go generate ./tools/csv-merger - go generate ./tools/manifest-templator +generate-crd: generate build-crd-creator + ./_out/crd-creator --output-file=config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml + @echo "the CRD file was generated in config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml" + +prepare-tools-crd: generate-crd + cp config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml ./tools/csv-merger/generated-crd.yaml + cp config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml ./tools/manifest-templator/generated-crd.yaml generate: generate-feature-gates ./hack/generate.sh @@ -270,10 +274,10 @@ help: ## Show this help screen awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' @echo '' -test-unit: gogenerate gogenerate-crd-creator +test-unit: gogenerate prepare-tools-crd ./hack/unit-test.sh -test-unit-coverage: gogenerate gogenerate-crd-creator +test-unit-coverage: gogenerate prepare-tools-crd ./hack/unit-test-coverage.sh test-fuzz-api-conversion: generate diff --git a/hack/build-manifests.sh b/hack/build-manifests.sh index 8bf075c970..8c07a8a30f 100755 --- a/hack/build-manifests.sh +++ b/hack/build-manifests.sh @@ -258,10 +258,6 @@ function create_inflight_operations_csv() { echo "${operatorName}" } -# Write HCO CRDs -hco_crds=${PROJECT_ROOT}/config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml -${TOOLS}/crd-creator --output-file=${hco_crds} - (cd ${PROJECT_ROOT}/tools/manifest-splitter/ && go build) TEMPDIR=$(mktemp -d) || (echo "Failed to create temp directory" && exit 1) @@ -295,11 +291,10 @@ spec: $keywords EOM -cat ${hco_crds} | ${TOOLS}/manifest-splitter --operator-name="hco" +cat "${PROJECT_ROOT}/config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml" | "${TOOLS}/manifest-splitter" --operator-name="hco" popd - rm -fr "${CSV_DIR}" mkdir -p "${CSV_DIR}/metadata" "${CSV_DIR}/manifests" From 1ae45b6ced2632b01a69a7bf7a193842ab7c0ec1 Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Tue, 7 Jul 2026 13:00:02 +0300 Subject: [PATCH 2/6] make v1 FG to be case-insensitive Also, make sure the feature gate are unique in the FeatureGates list. Signed-off-by: Nahshon Unna Tsameret --- api/v1/featuregates/feature_gates.go | 6 +- api/v1/featuregates/feature_gates_test.go | 12 +++- pkg/featuregatedetails/feature_gates.go | 8 ++- pkg/featuregatedetails/feature_gates_test.go | 10 ++++ pkg/webhooks/mutator/hyperConvergedMutator.go | 5 +- pkg/webhooks/mutator/mutate_mdev_test.go | 57 ++++++++++++++----- 6 files changed, 74 insertions(+), 24 deletions(-) diff --git a/api/v1/featuregates/feature_gates.go b/api/v1/featuregates/feature_gates.go index 9ad475c00b..165d18b4dd 100644 --- a/api/v1/featuregates/feature_gates.go +++ b/api/v1/featuregates/feature_gates.go @@ -23,6 +23,7 @@ const ( // +k8s:openapi-gen=true type FeatureGate struct { // Name is the feature gate name + // +kubebuilder:validation:MaxLength=256 Name string `json:"name"` // State determines if the feature gate is Enabled, or Disabled. The default value is Enabled. @@ -68,6 +69,8 @@ func (fg *FeatureGate) UnmarshalJSON(bytes []byte) error { // +k8s:openapi-gen=true // +k8s:conversion-gen=false // +k8s:deepcopy-gen=false +// +kubebuilder:validation:MaxItems=64 +// +kubebuilder:validation:XValidation:rule="self.all(x, self.exists_one(y, x.name.lowerAscii() == y.name.lowerAscii()))",message="feature gate names must be unique (case-insensitive)" type HyperConvergedFeatureGates []FeatureGate // Enable enables a feature gate by its name @@ -137,7 +140,8 @@ func (fgs *HyperConvergedFeatureGates) IsExplicitlyEnabled(name string) (enabled } func (fgs *HyperConvergedFeatureGates) index(name string) int { + name = strings.ToLower(name) return slices.IndexFunc(*fgs, func(fg FeatureGate) bool { - return fg.Name == name + return strings.ToLower(fg.Name) == name }) } diff --git a/api/v1/featuregates/feature_gates_test.go b/api/v1/featuregates/feature_gates_test.go index 75f6a4a5c4..b4c0328b86 100644 --- a/api/v1/featuregates/feature_gates_test.go +++ b/api/v1/featuregates/feature_gates_test.go @@ -185,15 +185,21 @@ var _ = Describe("Feature Gates", func() { Entry("known alpha FG; in list; enabled", featuregates.HyperConvergedFeatureGates{{Name: "downwardMetrics", State: new(featuregates.Enabled)}}, "downwardMetrics", BeTrue()), Entry("known alpha FG; in list; disabled", featuregates.HyperConvergedFeatureGates{{Name: "downwardMetrics", State: new(featuregates.Disabled)}}, "downwardMetrics", BeFalse()), - Entry("known alpha FG; not in list; disabled", featuregates.HyperConvergedFeatureGates{{Name: "deployKubeSecondaryDNS", State: new(featuregates.Enabled)}}, "downwardMetrics", BeFalse()), + Entry("known alpha FG; not in list;", featuregates.HyperConvergedFeatureGates{{Name: "deployKubeSecondaryDNS", State: new(featuregates.Enabled)}}, "downwardMetrics", BeFalse()), + + Entry("known alpha FG with different casing; in list; enabled", featuregates.HyperConvergedFeatureGates{{Name: "DownwardMetricS", State: new(featuregates.Enabled)}}, "downwardMetrics", BeTrue()), + Entry("known alpha FG with different casing; in list; disabled", featuregates.HyperConvergedFeatureGates{{Name: "DownwardMetricS", State: new(featuregates.Disabled)}}, "downwardMetrics", BeFalse()), Entry("known beta FG; in list; enabled", featuregates.HyperConvergedFeatureGates{{Name: "declarativeHotplugVolumes", State: new(featuregates.Enabled)}}, "declarativeHotplugVolumes", BeTrue()), Entry("known beta FG; in list; disabled", featuregates.HyperConvergedFeatureGates{{Name: "declarativeHotplugVolumes", State: new(featuregates.Disabled)}}, "declarativeHotplugVolumes", BeFalse()), - Entry("known beta FG; not in list; disabled", featuregates.HyperConvergedFeatureGates{{Name: "deployKubeSecondaryDNS", State: new(featuregates.Enabled)}}, "declarativeHotplugVolumes", BeTrue()), + Entry("known beta FG; not in list;", featuregates.HyperConvergedFeatureGates{{Name: "deployKubeSecondaryDNS", State: new(featuregates.Enabled)}}, "declarativeHotplugVolumes", BeTrue()), + + Entry("known beta FG with different casing; in list; enabled", featuregates.HyperConvergedFeatureGates{{Name: "DeclarativeHotplugVolumeS", State: new(featuregates.Enabled)}}, "declarativeHotplugVolumes", BeTrue()), + Entry("known beta FG with different casing; in list; disabled", featuregates.HyperConvergedFeatureGates{{Name: "DeclarativeHotplugVolumeS", State: new(featuregates.Disabled)}}, "declarativeHotplugVolumes", BeFalse()), Entry("known deprecated FG; in list; enabled", featuregates.HyperConvergedFeatureGates{{Name: "withHostPassthroughCPU", State: new(featuregates.Enabled)}}, "withHostPassthroughCPU", BeFalse()), Entry("known deprecated FG; in list; disabled", featuregates.HyperConvergedFeatureGates{{Name: "withHostPassthroughCPU", State: new(featuregates.Disabled)}}, "withHostPassthroughCPU", BeFalse()), - Entry("known deprecated FG; not in list; disabled", featuregates.HyperConvergedFeatureGates{{Name: "deployKubeSecondaryDNS", State: new(featuregates.Enabled)}}, "withHostPassthroughCPU", BeFalse()), + Entry("known deprecated FG; not in list;", featuregates.HyperConvergedFeatureGates{{Name: "deployKubeSecondaryDNS", State: new(featuregates.Enabled)}}, "withHostPassthroughCPU", BeFalse()), ) Context("check Enable", func() { diff --git a/pkg/featuregatedetails/feature_gates.go b/pkg/featuregatedetails/feature_gates.go index 873a1a293e..e5837253db 100644 --- a/pkg/featuregatedetails/feature_gates.go +++ b/pkg/featuregatedetails/feature_gates.go @@ -6,6 +6,7 @@ import ( "iter" "maps" "slices" + "strings" "github.com/kubevirt/hyperconverged-cluster-operator/pkg/featuregates" ) @@ -16,6 +17,7 @@ var featureGateJson []byte var featureGatesDetails map[string]featuregates.FeatureGate func GetFeatureGatePhase(fgName string) (enabled featuregates.Phase, fgExists bool) { + fgName = strings.ToLower(fgName) fg, ok := featureGatesDetails[fgName] if !ok { return featuregates.PhaseUnknown, false @@ -53,7 +55,7 @@ func setup(fgJson []byte) error { func fgsToMap(fgs iter.Seq[featuregates.FeatureGate]) iter.Seq2[string, featuregates.FeatureGate] { return func(yield func(string, featuregates.FeatureGate) bool) { for fg := range fgs { - if !yield(fg.Name, fg) { + if !yield(strings.ToLower(fg.Name), fg) { return } } @@ -62,9 +64,9 @@ func fgsToMap(fgs iter.Seq[featuregates.FeatureGate]) iter.Seq2[string, featureg func filterFGByPhase(fgs iter.Seq2[string, featuregates.FeatureGate], phase featuregates.Phase) iter.Seq[string] { return func(yield func(string) bool) { - for fgName, fg := range fgs { + for _, fg := range fgs { if fg.Phase == phase { - if !yield(fgName) { + if !yield(fg.Name) { return } } diff --git a/pkg/featuregatedetails/feature_gates_test.go b/pkg/featuregatedetails/feature_gates_test.go index b9a8239cc0..5bc527e1c1 100644 --- a/pkg/featuregatedetails/feature_gates_test.go +++ b/pkg/featuregatedetails/feature_gates_test.go @@ -33,6 +33,16 @@ var _ = Describe("Feature Gate Details", func() { Expect(phase).To(Equal(featuregates.PhaseGA)) }) + It("should return the phase of the feature gate, with different casing", func() { + featureGatesDetails = map[string]featuregates.FeatureGate{ + "fg1": {Name: "fg1", Phase: featuregates.PhaseGA}, + } + + phase, exists := GetFeatureGatePhase("FG1") + Expect(exists).To(BeTrue()) + Expect(phase).To(Equal(featuregates.PhaseGA)) + }) + It("should return false if feature gate does not exist", func() { featureGatesDetails = map[string]featuregates.FeatureGate{ "exists": {Name: "exists", Phase: featuregates.PhaseGA}, diff --git a/pkg/webhooks/mutator/hyperConvergedMutator.go b/pkg/webhooks/mutator/hyperConvergedMutator.go index 9ff03e51bc..2bed10b1d3 100644 --- a/pkg/webhooks/mutator/hyperConvergedMutator.go +++ b/pkg/webhooks/mutator/hyperConvergedMutator.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/http" - "slices" "github.com/go-logr/logr" "gomodules.xyz/jsonpatch/v2" @@ -231,9 +230,7 @@ func dropMdevFG(fgs hcov1fg.HyperConvergedFeatureGates, patches []jsonpatch.Json return patches } - idx := slices.IndexFunc(fgs, func(fg hcov1fg.FeatureGate) bool { - return fg.Name == disableMDevConfigurationFGName - }) + idx := fgs.Index(disableMDevConfigurationFGName) if idx < 0 { return patches diff --git a/pkg/webhooks/mutator/mutate_mdev_test.go b/pkg/webhooks/mutator/mutate_mdev_test.go index c70d33d1f5..085ed821d1 100644 --- a/pkg/webhooks/mutator/mutate_mdev_test.go +++ b/pkg/webhooks/mutator/mutate_mdev_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -11,7 +12,6 @@ import ( "gomodules.xyz/jsonpatch/v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -40,7 +40,7 @@ var _ = Describe("test HyperConverged v1 mutator", func() { }, Spec: hcov1.HyperConvergedSpec{ Virtualization: hcov1.VirtualizationConfig{ - EvictionStrategy: ptr.To(kubevirtcorev1.EvictionStrategyLiveMigrate), + EvictionStrategy: new(kubevirtcorev1.EvictionStrategyLiveMigrate), }, }, } @@ -1480,7 +1480,7 @@ var _ = Describe("test HyperConverged v1 mutator", func() { &hcov1.HyperConverged{ Spec: hcov1.HyperConvergedSpec{ FeatureGates: hcov1fg.HyperConvergedFeatureGates{ - {Name: "disableMDevConfiguration"}, + {Name: disableMDevConfigurationFGName}, }, Virtualization: nilField, }, @@ -1488,7 +1488,7 @@ var _ = Describe("test HyperConverged v1 mutator", func() { &hcov1.HyperConverged{ Spec: hcov1.HyperConvergedSpec{ FeatureGates: hcov1fg.HyperConvergedFeatureGates{ - {Name: "disableMDevConfiguration"}, + {Name: disableMDevConfigurationFGName}, }, Virtualization: enabledField, }, @@ -1503,11 +1503,11 @@ var _ = Describe("test HyperConverged v1 mutator", func() { }, ), - Entry("should remove the only the disableMDevConfiguration FG is set, if it's the first FG", + Entry("should remove only the disableMDevConfiguration FG, if it's the first FG", &hcov1.HyperConverged{ Spec: hcov1.HyperConvergedSpec{ FeatureGates: hcov1fg.HyperConvergedFeatureGates{ - {Name: "disableMDevConfiguration"}, + {Name: disableMDevConfigurationFGName}, {Name: "someEnabledFG"}, {Name: "someDisabledFG", State: new(hcov1fg.Disabled)}, }, @@ -1517,7 +1517,7 @@ var _ = Describe("test HyperConverged v1 mutator", func() { &hcov1.HyperConverged{ Spec: hcov1.HyperConvergedSpec{ FeatureGates: hcov1fg.HyperConvergedFeatureGates{ - {Name: "disableMDevConfiguration"}, + {Name: disableMDevConfigurationFGName}, {Name: "someEnabledFG"}, {Name: "someDisabledFG", State: new(hcov1fg.Disabled)}, }, @@ -1534,12 +1534,12 @@ var _ = Describe("test HyperConverged v1 mutator", func() { }, ), - Entry("should remove the only the disableMDevConfiguration FG is set, if it's not the first FG", + Entry("should remove only the disableMDevConfiguration FG, if it's not the first FG", &hcov1.HyperConverged{ Spec: hcov1.HyperConvergedSpec{ FeatureGates: hcov1fg.HyperConvergedFeatureGates{ {Name: "someEnabledFG"}, - {Name: "disableMDevConfiguration"}, + {Name: disableMDevConfigurationFGName}, {Name: "someDisabledFG", State: new(hcov1fg.Disabled)}, }, Virtualization: nilField, @@ -1549,7 +1549,7 @@ var _ = Describe("test HyperConverged v1 mutator", func() { Spec: hcov1.HyperConvergedSpec{ FeatureGates: hcov1fg.HyperConvergedFeatureGates{ {Name: "someEnabledFG"}, - {Name: "disableMDevConfiguration"}, + {Name: disableMDevConfigurationFGName}, {Name: "someDisabledFG", State: new(hcov1fg.Disabled)}, }, Virtualization: enabledField, @@ -1565,13 +1565,13 @@ var _ = Describe("test HyperConverged v1 mutator", func() { }, ), - Entry("should remove the only the disableMDevConfiguration FG is set, if it's the last FG", + Entry("should remove only the disableMDevConfiguration FG, if it's the last FG", &hcov1.HyperConverged{ Spec: hcov1.HyperConvergedSpec{ FeatureGates: hcov1fg.HyperConvergedFeatureGates{ {Name: "someEnabledFG"}, {Name: "someDisabledFG", State: new(hcov1fg.Disabled)}, - {Name: "disableMDevConfiguration"}, + {Name: disableMDevConfigurationFGName}, }, Virtualization: nilField, }, @@ -1581,7 +1581,38 @@ var _ = Describe("test HyperConverged v1 mutator", func() { FeatureGates: hcov1fg.HyperConvergedFeatureGates{ {Name: "someEnabledFG"}, {Name: "someDisabledFG", State: new(hcov1fg.Disabled)}, - {Name: "disableMDevConfiguration"}, + {Name: disableMDevConfigurationFGName}, + }, + Virtualization: enabledField, + }, + }, + &expectedResponse{ + patches: []jsonpatch.JsonPatchOperation{{ + Operation: "remove", + Path: "/spec/featureGates/2", + }}, + checkAllowed: BeTrue(), + checkWarning: BeEmpty(), + }, + ), + + Entry("should remove the disableMDevConfiguration FG, if it's set with different casing", + &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcov1fg.HyperConvergedFeatureGates{ + {Name: "someEnabledFG"}, + {Name: "someDisabledFG", State: new(hcov1fg.Disabled)}, + {Name: strings.ToUpper(disableMDevConfigurationFGName)}, + }, + Virtualization: nilField, + }, + }, + &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcov1fg.HyperConvergedFeatureGates{ + {Name: "someEnabledFG"}, + {Name: "someDisabledFG", State: new(hcov1fg.Disabled)}, + {Name: strings.ToUpper(disableMDevConfigurationFGName)}, }, Virtualization: enabledField, }, From 589a47e1e4442d1f6e6f14d4216f5d74e06cc523 Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Tue, 7 Jul 2026 13:01:41 +0300 Subject: [PATCH 3/6] regenerate CRDs Signed-off-by: Nahshon Unna Tsameret --- config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml | 5 +++++ deploy/crds/hco00.crd.yaml | 5 +++++ .../1.19.0/manifests/hco00.crd.yaml | 5 +++++ .../1.19.0/manifests/hco00.crd.yaml | 5 +++++ tools/csv-merger/generated-crd.yaml | 5 +++++ tools/manifest-templator/generated-crd.yaml | 5 +++++ 6 files changed, 30 insertions(+) diff --git a/config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml b/config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml index 4fea4760ff..d9f1ac918c 100644 --- a/config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml +++ b/config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml @@ -2299,6 +2299,7 @@ spec: properties: name: description: Name is the feature gate name + maxLength: 256 type: string state: description: State determines if the feature gate is Enabled, @@ -2310,7 +2311,11 @@ spec: required: - name type: object + maxItems: 64 type: array + x-kubernetes-validations: + - message: feature gate names must be unique (case-insensitive) + rule: self.all(x, self.exists_one(y, x.name.lowerAscii() == y.name.lowerAscii())) networking: description: Networking contains all the configurations for networking properties: diff --git a/deploy/crds/hco00.crd.yaml b/deploy/crds/hco00.crd.yaml index 4fea4760ff..d9f1ac918c 100644 --- a/deploy/crds/hco00.crd.yaml +++ b/deploy/crds/hco00.crd.yaml @@ -2299,6 +2299,7 @@ spec: properties: name: description: Name is the feature gate name + maxLength: 256 type: string state: description: State determines if the feature gate is Enabled, @@ -2310,7 +2311,11 @@ spec: required: - name type: object + maxItems: 64 type: array + x-kubernetes-validations: + - message: feature gate names must be unique (case-insensitive) + rule: self.all(x, self.exists_one(y, x.name.lowerAscii() == y.name.lowerAscii())) networking: description: Networking contains all the configurations for networking properties: diff --git a/deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml b/deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml index 4fea4760ff..d9f1ac918c 100644 --- a/deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml +++ b/deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml @@ -2299,6 +2299,7 @@ spec: properties: name: description: Name is the feature gate name + maxLength: 256 type: string state: description: State determines if the feature gate is Enabled, @@ -2310,7 +2311,11 @@ spec: required: - name type: object + maxItems: 64 type: array + x-kubernetes-validations: + - message: feature gate names must be unique (case-insensitive) + rule: self.all(x, self.exists_one(y, x.name.lowerAscii() == y.name.lowerAscii())) networking: description: Networking contains all the configurations for networking properties: diff --git a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml index 4fea4760ff..d9f1ac918c 100644 --- a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml +++ b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml @@ -2299,6 +2299,7 @@ spec: properties: name: description: Name is the feature gate name + maxLength: 256 type: string state: description: State determines if the feature gate is Enabled, @@ -2310,7 +2311,11 @@ spec: required: - name type: object + maxItems: 64 type: array + x-kubernetes-validations: + - message: feature gate names must be unique (case-insensitive) + rule: self.all(x, self.exists_one(y, x.name.lowerAscii() == y.name.lowerAscii())) networking: description: Networking contains all the configurations for networking properties: diff --git a/tools/csv-merger/generated-crd.yaml b/tools/csv-merger/generated-crd.yaml index 4fea4760ff..d9f1ac918c 100644 --- a/tools/csv-merger/generated-crd.yaml +++ b/tools/csv-merger/generated-crd.yaml @@ -2299,6 +2299,7 @@ spec: properties: name: description: Name is the feature gate name + maxLength: 256 type: string state: description: State determines if the feature gate is Enabled, @@ -2310,7 +2311,11 @@ spec: required: - name type: object + maxItems: 64 type: array + x-kubernetes-validations: + - message: feature gate names must be unique (case-insensitive) + rule: self.all(x, self.exists_one(y, x.name.lowerAscii() == y.name.lowerAscii())) networking: description: Networking contains all the configurations for networking properties: diff --git a/tools/manifest-templator/generated-crd.yaml b/tools/manifest-templator/generated-crd.yaml index 4fea4760ff..d9f1ac918c 100644 --- a/tools/manifest-templator/generated-crd.yaml +++ b/tools/manifest-templator/generated-crd.yaml @@ -2299,6 +2299,7 @@ spec: properties: name: description: Name is the feature gate name + maxLength: 256 type: string state: description: State determines if the feature gate is Enabled, @@ -2310,7 +2311,11 @@ spec: required: - name type: object + maxItems: 64 type: array + x-kubernetes-validations: + - message: feature gate names must be unique (case-insensitive) + rule: self.all(x, self.exists_one(y, x.name.lowerAscii() == y.name.lowerAscii())) networking: description: Networking contains all the configurations for networking properties: From c83352ee50b5f79d9af2ddadf330ad40337d2acb Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Tue, 7 Jul 2026 17:53:55 +0300 Subject: [PATCH 4/6] add functional tests Signed-off-by: Nahshon Unna Tsameret --- tests/func-tests/feature_gates_test.go | 110 +++++++++++++++++++++++++ tests/func-tests/hyperconverged.go | 12 +++ 2 files changed, 122 insertions(+) create mode 100644 tests/func-tests/feature_gates_test.go diff --git a/tests/func-tests/feature_gates_test.go b/tests/func-tests/feature_gates_test.go new file mode 100644 index 0000000000..771b6d0d2d --- /dev/null +++ b/tests/func-tests/feature_gates_test.go @@ -0,0 +1,110 @@ +package tests_test + +import ( + "context" + "fmt" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kubevirt/hyperconverged-cluster-operator/pkg/featuregatedetails" + "github.com/kubevirt/hyperconverged-cluster-operator/pkg/featuregates" + tests "github.com/kubevirt/hyperconverged-cluster-operator/tests/func-tests" +) + +var _ = Describe("test feature gates", Label("feature-gates"), func() { + var ( + cli client.Client + ) + + BeforeEach(func(ctx context.Context) { + cli = tests.GetControllerRuntimeClient() + + tests.RestoreDefaultFeatureGates(ctx, cli) + DeferCleanup(func(ctx context.Context) { + tests.RestoreDefaultFeatureGates(ctx, cli) + }) + }) + + It("allow enabling a FG with KubeVirt casing", func(ctx context.Context) { + // this is an example of a "proxy feature gate" - a FG that is set in the HyperConverged CR, in order to set a + // FG in the KubeVirt CR. The HCO name starts with lower case, while KubeVirt's FG name starts with upper case + // This test works only if the feature gate is in alpha phase. If it became deprecated, or graduated to beta or GA + // it should be replaced with another FG. + const ( + kvFG = "DownwardMetrics" + hcoFG = "downwardMetrics" + ) + + // make sure it's alpha. If not, choose another alpha proxy FG + phase, exists := featuregatedetails.GetFeatureGatePhase(hcoFG) + Expect(exists).To(BeTrue()) + Expect(phase).To(Equal(featuregates.PhaseAlpha)) + + By("make sure the feature gate is not enabled in KubeVirt") + Eventually(func(g Gomega, ctx context.Context) { + kv := getKubeVirt(ctx, g, cli) + g.Expect(kv.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil()) + g.Expect(kv.Spec.Configuration.DeveloperConfiguration.FeatureGates).ToNot(ContainElement(kvFG)) + }).WithTimeout(time.Minute).WithPolling(10 * time.Second).WithContext(ctx).Should(Succeed()) + + By("set the feature gate using KubeVirt casing") + Expect(tests.EnableFG(ctx, cli, "DownwardMetrics")).To(Succeed()) + + By("make sure the feature gate is enabled in KubeVirt") + Eventually(func(g Gomega, ctx context.Context) { + kv := getKubeVirt(ctx, g, cli) + g.Expect(kv.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil()) + g.Expect(kv.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElement(kvFG)) + }).WithTimeout(time.Minute).WithPolling(10 * time.Second).WithContext(ctx).Should(Succeed()) + }) + + // assuming the spec.featureGate field is not empty. adding this function to the test context because it's + // implemented for this test. To enable a feature gate in another context, use the tests.EnableFG() function. + addFeatureGate := func(ctx context.Context, cli client.Client, fgName string) error { + const ( + appendFGTemplate = `[{"op": "add", "path": "/spec/featureGates/-", "value": {"name": %q}}]` + ) + + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + hc, err := tests.GetHCO(ctx, cli) + if err != nil { + return err + } + + patch := fmt.Appendf(nil, appendFGTemplate, fgName) + return cli.Patch(ctx, hc, client.RawPatch(types.JSONPatchType, patch)) + }) + } + + It("should not allow enabling the same feature gate with different casing", func(ctx context.Context) { + hcoAlphaFGs := featuregatedetails.ListAlphaFeatureGates() + if len(hcoAlphaFGs) == 0 { + Skip("no Alpha feature gates found") + } + + fgName := hcoAlphaFGs[0] + + By(fmt.Sprintf("Add the %q feature gate to the HyperConverged CR", fgName)) + Expect(tests.EnableFG(ctx, cli, fgName)).To(Succeed()) + + By("try adding the feature gate to the HyperConverged CR, with the same name") + Expect(addFeatureGate(ctx, cli, strings.ToUpper(fgName))).To(MatchError(k8serrors.IsInvalid, "check if it's the invalid error")) + + By("try adding the feature gate to the HyperConverged CR, with all lower case") + Expect(addFeatureGate(ctx, cli, strings.ToLower(fgName))).To(MatchError(k8serrors.IsInvalid, "check if it's the invalid error")) + + By("try adding the feature gate to the HyperConverged CR, with all lower case") + Expect(addFeatureGate(ctx, cli, strings.ToLower(fgName))).To(MatchError(k8serrors.IsInvalid, "check if it's the invalid error")) + + By("try adding the feature gate to the HyperConverged CR, in KubeVirt format") + nameInKVFormat := strings.ToUpper(fgName[:1]) + fgName[1:] + Expect(addFeatureGate(ctx, cli, nameInKVFormat)).To(MatchError(k8serrors.IsInvalid, "check if it's the invalid error")) + }) +}) diff --git a/tests/func-tests/hyperconverged.go b/tests/func-tests/hyperconverged.go index 82223d596b..8312413837 100644 --- a/tests/func-tests/hyperconverged.go +++ b/tests/func-tests/hyperconverged.go @@ -163,6 +163,18 @@ func RestoreDefaults(ctx context.Context, cli client.Client) { PatchHCO(ctx, cli, []byte(`[{"op": "replace", "path": "/spec", "value": {}}]`)) } +func RestoreDefaultFeatureGates(ctx context.Context, cli client.Client) { + ginkgo.GinkgoHelper() + + hc, err := GetHCO(ctx, cli) + Expect(err).NotTo(HaveOccurred()) + if hc.Spec.FeatureGates == nil { + return + } + + PatchHCO(ctx, cli, []byte(`[{"op": "remove", "path": "/spec/featureGates"}]`)) +} + func EnableFG(ctx context.Context, cli client.Client, fgName string) error { hc, err := GetHCO(ctx, cli) if err != nil { From c2e957b71ad109971a8d772d8c78c2e5cef00a19 Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Wed, 8 Jul 2026 08:03:39 +0300 Subject: [PATCH 5/6] do marshal the state field if exists and enabled we only marshal the FeatureGate state field if it disabled, but k8s marshling sometimes keeps it even if it Enabled, so for consistency, we now marshaling the state field if exists, no matter its value. Signed-off-by: Nahshon Unna Tsameret --- api/v1/featuregates/feature_gates.go | 6 +++--- api/v1/featuregates/feature_gates_test.go | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/api/v1/featuregates/feature_gates.go b/api/v1/featuregates/feature_gates.go index 165d18b4dd..afce72b93d 100644 --- a/api/v1/featuregates/feature_gates.go +++ b/api/v1/featuregates/feature_gates.go @@ -37,9 +37,9 @@ func (fg FeatureGate) MarshalJSON() ([]byte, error) { builder.WriteString(fg.Name) builder.WriteByte('"') - if fg.State != nil && *fg.State == Disabled { + if fg.State != nil { builder.WriteString(`,"state":"`) - builder.WriteString(string(Disabled)) + builder.WriteString(string(*fg.State)) builder.WriteByte('"') } builder.WriteByte('}') @@ -55,7 +55,7 @@ func (fg *FeatureGate) UnmarshalJSON(bytes []byte) error { } if fg.State == nil { - fg.State = ptr.To(Enabled) + fg.State = new(Enabled) } return nil diff --git a/api/v1/featuregates/feature_gates_test.go b/api/v1/featuregates/feature_gates_test.go index b4c0328b86..8308f71e74 100644 --- a/api/v1/featuregates/feature_gates_test.go +++ b/api/v1/featuregates/feature_gates_test.go @@ -27,7 +27,7 @@ var _ = Describe("FeatureGate", func() { jsonBytes, err := json.Marshal(fg) Expect(err).ToNot(HaveOccurred()) - Expect(jsonBytes).To(MatchJSON(`{"name":"fgName"}`)) + Expect(jsonBytes).To(MatchJSON(`{"name":"fgName", "state": "Enabled"}`)) }) It("should marshal a disabled feature gate", func() { @@ -49,7 +49,7 @@ var _ = Describe("FeatureGate", func() { jsonBytes, err := json.Marshal(fg) Expect(err).ToNot(HaveOccurred()) - Expect(jsonBytes).To(MatchJSON(`{"name":"fgName"}`)) + Expect(jsonBytes).To(MatchJSON(`{"name":"fgName", "state": "Enabled"}`)) }) It("should marshal a disabled feature gate pointer", func() { @@ -90,7 +90,7 @@ var _ = Describe("FeatureGate", func() { jsonBytes, err := json.Marshal(fgs) Expect(err).ToNot(HaveOccurred()) - Expect(jsonBytes).To(MatchJSON(`[{"name":"noEnabledField"}, {"name": "enabledFG"}, {"name": "disabledFG", "state": "Disabled"}]`)) + Expect(jsonBytes).To(MatchJSON(`[{"name":"noEnabledField"}, {"name": "enabledFG", "state": "Enabled"}, {"name": "disabledFG", "state": "Disabled"}]`)) }) It("should yaml marshal a FG array", func() { @@ -112,6 +112,7 @@ var _ = Describe("FeatureGate", func() { Expect(err).ToNot(HaveOccurred()) Expect(yamlBytes).To(MatchYAML(`- name: noEnabledField - name: enabledFG + state: Enabled - state: Disabled name: disabledFG`, )) From 188f957d3a11530d8a4ad39180d917e6dcf748c3 Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Wed, 8 Jul 2026 08:17:52 +0300 Subject: [PATCH 6/6] Preserve v1 only FGs The automatic conversion only handles feature gates that are defined in both v1beta1 and v1 API versions. Any feature gate tat is not defined in v1beta1, will be lost in conversion. This commit preserves all v1 feature gates using the v1-only-fields mechanism. Signed-off-by: Nahshon Unna Tsameret --- api/v1/featuregates/feature_gates.go | 8 +- api/v1beta1/conversion.go | 32 +- api/v1beta1/conversion_test.go | 681 ++++++++++++++++-- .../zz_generated.featuregates_conversion.go | 178 ++++- .../conversion.go.tmpl | 26 +- 5 files changed, 804 insertions(+), 121 deletions(-) diff --git a/api/v1/featuregates/feature_gates.go b/api/v1/featuregates/feature_gates.go index afce72b93d..c079d8523f 100644 --- a/api/v1/featuregates/feature_gates.go +++ b/api/v1/featuregates/feature_gates.go @@ -84,7 +84,7 @@ func (fgs *HyperConvergedFeatureGates) Disable(name string) { } func (fgs *HyperConvergedFeatureGates) set(name string, enabled State) { - idx := fgs.index(name) + idx := fgs.Index(name) if idx == -1 { *fgs = append(*fgs, FeatureGate{Name: name, State: &enabled}) @@ -121,7 +121,7 @@ func (fgs *HyperConvergedFeatureGates) IsEnabled(name string) bool { return false } - if idx := fgs.index(name); idx > -1 { + if idx := fgs.Index(name); idx > -1 { state = ptr.Deref((*fgs)[idx].State, Enabled) } @@ -130,7 +130,7 @@ func (fgs *HyperConvergedFeatureGates) IsEnabled(name string) bool { // IsExplicitlyEnabled checks if a feature gate is explicitly set in the feature gate list func (fgs *HyperConvergedFeatureGates) IsExplicitlyEnabled(name string) (enabled bool, found bool) { - idx := fgs.index(name) + idx := fgs.Index(name) if idx < 0 { return false, false @@ -139,7 +139,7 @@ func (fgs *HyperConvergedFeatureGates) IsExplicitlyEnabled(name string) (enabled return ptr.Deref((*fgs)[idx].State, Enabled) == Enabled, true } -func (fgs *HyperConvergedFeatureGates) index(name string) int { +func (fgs *HyperConvergedFeatureGates) Index(name string) int { name = strings.ToLower(name) return slices.IndexFunc(*fgs, func(fg FeatureGate) bool { return strings.ToLower(fg.Name) == name diff --git a/api/v1beta1/conversion.go b/api/v1beta1/conversion.go index 46f44f7761..5d5a526919 100644 --- a/api/v1beta1/conversion.go +++ b/api/v1beta1/conversion.go @@ -14,6 +14,7 @@ import ( kubevirtv1 "kubevirt.io/api/core/v1" hcov1 "github.com/kubevirt/hyperconverged-cluster-operator/api/v1" + hcov1fg "github.com/kubevirt/hyperconverged-cluster-operator/api/v1/featuregates" ) const v1OnlyFieldAnnotation = APIVersionGroup + "/v1-only-fields" @@ -21,9 +22,17 @@ const v1OnlyFieldAnnotation = APIVersionGroup + "/v1-only-fields" const DisableMDevConfigurationFG = "disableMDevConfiguration" type v1OnlyFields struct { - DeployNetworkResourcesInjector *bool `json:"deployNetworkResourcesInjector,omitempty"` - MDevConfigEnable *bool `json:"mdevConfigEnable,omitempty"` - DisableMDevConfigurationFG *bool `json:"disableMDevConfigurationFG,omitempty"` + DeployNetworkResourcesInjector *bool `json:"deployNetworkResourcesInjector,omitempty"` + MDevConfigEnable *bool `json:"mdevConfigEnable,omitempty"` + DisableMDevConfigurationFG *bool `json:"disableMDevConfigurationFG,omitempty"` + FeatureGates hcov1fg.HyperConvergedFeatureGates `json:"featureGates,omitempty"` +} + +func (fields *v1OnlyFields) isEmpty() bool { + return fields.DeployNetworkResourcesInjector == nil && + fields.MDevConfigEnable == nil && + fields.DisableMDevConfigurationFG == nil && + fields.FeatureGates == nil } // Implement the conversion.Convertible interface, to be used in the conversion webhook. @@ -547,12 +556,8 @@ func restoreV1OnlyFields(src *HyperConverged, dst *hcov1.HyperConverged) error { dst.Spec.Virtualization.MediatedDevicesConfiguration.Enabled = new(*v1Fields.MDevConfigEnable) } - if v1Fields.DisableMDevConfigurationFG != nil { - if *v1Fields.DisableMDevConfigurationFG { - dst.Spec.FeatureGates.Enable(DisableMDevConfigurationFG) - } else { - dst.Spec.FeatureGates.Disable(DisableMDevConfigurationFG) - } + for _, fg := range v1Fields.FeatureGates { + dst.Spec.FeatureGates = append(dst.Spec.FeatureGates, *fg.DeepCopy()) } return nil @@ -567,11 +572,14 @@ func storeV1OnlyFields(src *hcov1.HyperConverged, dst *HyperConverged) error { v1Fields.MDevConfigEnable = src.Spec.Virtualization.MediatedDevicesConfiguration.Enabled } - if fgEnabled, fgFound := src.Spec.FeatureGates.IsExplicitlyEnabled(DisableMDevConfigurationFG); fgFound { - v1Fields.DisableMDevConfigurationFG = new(fgEnabled) + if len(src.Spec.FeatureGates) > 0 { + v1Fields.FeatureGates = make(hcov1fg.HyperConvergedFeatureGates, len(src.Spec.FeatureGates)) + for i, fg := range src.Spec.FeatureGates { + v1Fields.FeatureGates[i] = *fg.DeepCopy() + } } - if v1Fields == (v1OnlyFields{}) { + if v1Fields.isEmpty() { return nil } diff --git a/api/v1beta1/conversion_test.go b/api/v1beta1/conversion_test.go index fe61b54851..180c9cc03c 100644 --- a/api/v1beta1/conversion_test.go +++ b/api/v1beta1/conversion_test.go @@ -520,6 +520,540 @@ var _ = Describe("api/v1beta1", func() { Expect(result.DecentralizedLiveMigration).To(HaveValue(BeTrue())) Expect(result.DeclarativeHotplugVolumes).To(HaveValue(BeTrue())) }) + + It("v1-only feature gates should survive roundtrip", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: "implicitlyEnabled"}, + {Name: "explicitlyEnabled", State: new(hcofg.Enabled)}, + {Name: "disabled", State: new(hcofg.Disabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + enabled, found := result.Spec.FeatureGates.IsExplicitlyEnabled("implicitlyEnabled") + Expect(found).To(BeTrue()) + Expect(enabled).To(BeTrue()) + + enabled, found = result.Spec.FeatureGates.IsExplicitlyEnabled("explicitlyEnabled") + Expect(found).To(BeTrue()) + Expect(enabled).To(BeTrue()) + + enabled, found = result.Spec.FeatureGates.IsExplicitlyEnabled("disabled") + Expect(found).To(BeTrue()) + Expect(enabled).To(BeFalse()) + }) + + It("v1beta1 feature gates with different cases should survive roundtrip", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + // enable alphas, disable betas + {Name: "ALIGNCPUS"}, // implicitly enable alpha + {Name: "DOWNWARDMETRICS", State: new(hcofg.Enabled)}, // explicitly enable alpha + {Name: "DECENTRALIZEDLIVEMIGRATION", State: new(hcofg.Disabled)}, // disable beta + {Name: "DECLARATIVEHOTPLUGVOLUMES", State: new(hcofg.Disabled)}, // disable beta + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + Expect(v1beta1HC.Spec.FeatureGates.AlignCPUs).To(HaveValue(BeTrue())) + Expect(v1beta1HC.Spec.FeatureGates.DownwardMetrics).To(HaveValue(BeTrue())) + Expect(v1beta1HC.Spec.FeatureGates.DecentralizedLiveMigration).To(HaveValue(BeFalse())) + Expect(v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes).To(HaveValue(BeFalse())) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(4)) + Expect(result.Spec.FeatureGates).To(ContainElements( + hcofg.FeatureGate{Name: "ALIGNCPUS", State: new(hcofg.Enabled)}, + hcofg.FeatureGate{Name: "DOWNWARDMETRICS", State: new(hcofg.Enabled)}, + hcofg.FeatureGate{Name: "DECENTRALIZEDLIVEMIGRATION", State: new(hcofg.Disabled)}, + hcofg.FeatureGate{Name: "DECLARATIVEHOTPLUGVOLUMES", State: new(hcofg.Disabled)}, + )) + + Expect(result.Spec.FeatureGates.IsEnabled("alignCPUs")).To(BeTrue()) + Expect(result.Spec.FeatureGates.IsEnabled("downwardMetrics")).To(BeTrue()) + Expect(result.Spec.FeatureGates.IsEnabled("decentralizedLiveMigration")).To(BeFalse()) + Expect(result.Spec.FeatureGates.IsEnabled("DeclarativeHotplugVolumes")).To(BeFalse()) + }) + + Context("v1beta1 feature gates with different casing can be modified using v1beta1 API", func() { + Context("beta feature gate", func() { + const ( + betaFGName = "declarativeHotplugVolumes" + betaFGNameAllUpper = "DECLARATIVEHOTPLUGVOLUMES" + ) + It("should allow enabling a FG when FGs are nil in v1, using v1beta1", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{}, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = new(true) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(BeEmpty()) + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeTrue()) + }) + + It("should allow disabling a FG when FGs are nil in v1, using v1beta1", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{}, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = new(false) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement( + hcofg.FeatureGate{Name: betaFGName, State: new(hcofg.Disabled)}, + )) + + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeFalse()) + }) + + It("should allow enabling a FG when the FG is not set in v1, using v1beta1", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: "someOtherFG", State: new(hcofg.Enabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = new(true) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + _, found := result.Spec.FeatureGates.IsExplicitlyEnabled(betaFGName) + Expect(found).To(BeFalse()) + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeTrue()) + }) + + It("should allow disabling a FG when the FG is not set in v1, using v1beta1", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: "someOtherFG", State: new(hcofg.Enabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = new(false) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(2)) + Expect(result.Spec.FeatureGates).To(ContainElement( + hcofg.FeatureGate{Name: betaFGName, State: new(hcofg.Disabled)}, + )) + + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeFalse()) + }) + + It("should allow disabling a FG when the FG already disabled in v1, with different casing", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: betaFGNameAllUpper, State: new(hcofg.Disabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = new(false) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: betaFGNameAllUpper, State: new(hcofg.Disabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeFalse()) + }) + + It("should allow enabling a FG when the FG already disabled in v1, with different casing", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: betaFGNameAllUpper, State: new(hcofg.Disabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = new(true) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: betaFGNameAllUpper, State: new(hcofg.Enabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeTrue()) + }) + + It("should allow disabling a FG when the FG already enabled in v1, with different casing", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: betaFGNameAllUpper, State: new(hcofg.Enabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = new(false) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: betaFGNameAllUpper, State: new(hcofg.Disabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeFalse()) + }) + + It("should allow enabling a FG when the FG already enabled in v1, with different casing", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: betaFGNameAllUpper, State: new(hcofg.Enabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = new(true) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: betaFGNameAllUpper, State: new(hcofg.Enabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeTrue()) + }) + + It("should remove an enabled FG if was removed in v1beta", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: "enabledFG"}, + {Name: betaFGNameAllUpper, State: new(hcofg.Enabled)}, + {Name: "disabledFG", State: new(hcofg.Disabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = nil + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(2)) + _, found := result.Spec.FeatureGates.IsExplicitlyEnabled(betaFGNameAllUpper) + Expect(found).To(BeFalse()) + _, found = result.Spec.FeatureGates.IsExplicitlyEnabled(betaFGName) + Expect(found).To(BeFalse()) + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeTrue()) + }) + + It("should remove an disabled FG if was removed in v1beta", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: "enabledFG"}, + {Name: betaFGNameAllUpper, State: new(hcofg.Disabled)}, + {Name: "disabledFG", State: new(hcofg.Disabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.DeclarativeHotplugVolumes = nil + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(2)) + _, found := result.Spec.FeatureGates.IsExplicitlyEnabled(betaFGNameAllUpper) + Expect(found).To(BeFalse()) + _, found = result.Spec.FeatureGates.IsExplicitlyEnabled(betaFGName) + Expect(found).To(BeFalse()) + Expect(result.Spec.FeatureGates.IsEnabled(betaFGName)).To(BeTrue()) + }) + }) + + Context("alpha feature gate", func() { + const ( + alphaFGName = "alignCPUs" + alphaFGNameAllUpper = "ALIGNCPUS" + ) + It("should allow enabling a FG when FGs are nil in v1, using v1beta1", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{}, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = new(true) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: alphaFGName, State: new(hcofg.Enabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeTrue()) + }) + + It("should allow disabling a FG when FGs are nil in v1, using v1beta1", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{}, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = new(false) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(BeEmpty()) + + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeFalse()) + }) + + It("should allow enabling a FG when the FG is not set in v1, using v1beta1", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: "someOtherFG", State: new(hcofg.Enabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = new(true) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(2)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: alphaFGName, State: new(hcofg.Enabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeTrue()) + }) + + It("should allow disabling a FG when the FG is not set in v1, using v1beta1", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: "someOtherFG", State: new(hcofg.Enabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = new(false) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + _, found := result.Spec.FeatureGates.IsExplicitlyEnabled(alphaFGName) + Expect(found).To(BeFalse()) + + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeFalse()) + }) + + It("should allow disabling a FG when the FG already disabled in v1, with different casing", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: alphaFGNameAllUpper, State: new(hcofg.Disabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = new(false) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: alphaFGNameAllUpper, State: new(hcofg.Disabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeFalse()) + }) + + It("should allow enabling a FG when the FG already disabled in v1, with different casing", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: alphaFGNameAllUpper, State: new(hcofg.Disabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = new(true) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: alphaFGNameAllUpper, State: new(hcofg.Enabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeTrue()) + }) + + It("should allow disabling a FG when the FG already enabled in v1, with different casing", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: alphaFGNameAllUpper, State: new(hcofg.Enabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = new(false) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: alphaFGNameAllUpper, State: new(hcofg.Disabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeFalse()) + }) + + It("should allow enabling a FG when the FG already enabled in v1, with different casing", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: alphaFGNameAllUpper, State: new(hcofg.Enabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = new(true) + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(1)) + Expect(result.Spec.FeatureGates).To(ContainElement(hcofg.FeatureGate{Name: alphaFGNameAllUpper, State: new(hcofg.Enabled)})) + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeTrue()) + }) + + It("should remove an enabled FG if was removed in v1beta", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: "enabledFG"}, + {Name: alphaFGNameAllUpper, State: new(hcofg.Enabled)}, + {Name: "disabledFG", State: new(hcofg.Disabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = nil + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(2)) + _, found := result.Spec.FeatureGates.IsExplicitlyEnabled(alphaFGNameAllUpper) + Expect(found).To(BeFalse()) + _, found = result.Spec.FeatureGates.IsExplicitlyEnabled(alphaFGName) + Expect(found).To(BeFalse()) + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeFalse()) + }) + + It("should remove an disabled FG if was removed in v1beta", func() { + original := &hcov1.HyperConverged{ + Spec: hcov1.HyperConvergedSpec{ + FeatureGates: hcofg.HyperConvergedFeatureGates{ + {Name: "enabledFG"}, + {Name: alphaFGNameAllUpper, State: new(hcofg.Disabled)}, + {Name: "disabledFG", State: new(hcofg.Disabled)}, + }, + }, + } + + v1beta1HC := &HyperConverged{} + Expect(v1beta1HC.ConvertFrom(original)).To(Succeed()) + + v1beta1HC.Spec.FeatureGates.AlignCPUs = nil + + result := &hcov1.HyperConverged{} + Expect(v1beta1HC.ConvertTo(result)).To(Succeed()) + + Expect(result.Spec.FeatureGates).To(HaveLen(2)) + _, found := result.Spec.FeatureGates.IsExplicitlyEnabled(alphaFGNameAllUpper) + Expect(found).To(BeFalse()) + _, found = result.Spec.FeatureGates.IsExplicitlyEnabled(alphaFGName) + Expect(found).To(BeFalse()) + Expect(result.Spec.FeatureGates.IsEnabled(alphaFGName)).To(BeFalse()) + }) + }) + }) }) }) @@ -757,8 +1291,8 @@ var _ = Describe("api/v1beta1", func() { Entry("when the v1 field is false and the v1beta1 FG is false", false, false), ) - DescribeTableSubtree("should convert v1beta1 disableMDevConfiguration to v1 FG, if FG set", func(v1FG, v1beta1FG bool) { - annotation := fmt.Sprintf(`{"disableMDevConfigurationFG": %t}`, v1FG) + DescribeTableSubtree("should convert the v1beta1 disableMDevConfiguration FG, to v1 FG, if FG set", func(v1FG hcofg.State, v1beta1FG bool) { + annotation := fmt.Sprintf(`{"featureGates": [{"name": %q, "state": %q}]}`, DisableMDevConfigurationFG, v1FG) It("just make sure the annotation works", func() { src := &HyperConverged{ @@ -774,7 +1308,7 @@ var _ = Describe("api/v1beta1", func() { enabled, found := dst.Spec.FeatureGates.IsExplicitlyEnabled(DisableMDevConfigurationFG) Expect(found).To(BeTrue()) - Expect(enabled).To(Equal(v1FG)) + Expect(enabled).To(Equal(v1FG == hcofg.Enabled)) }) It("should modify the v1 FG", func() { @@ -802,14 +1336,14 @@ var _ = Describe("api/v1beta1", func() { Expect(enabled).To(Equal(v1beta1FG)) }) }, - Entry("when the v1 field is true and the v1beta1 FG is true", true, true), - Entry("when the v1 field is false and the v1beta1 FG is true", false, true), - Entry("when the v1 field is true and the v1beta1 FG is false", true, false), - Entry("when the v1 field is false and the v1beta1 FG is false", false, false), + Entry("when the v1 field is true and the v1beta1 FG is true", hcofg.Enabled, true), + Entry("when the v1 field is false and the v1beta1 FG is true", hcofg.Disabled, true), + Entry("when the v1 field is true and the v1beta1 FG is false", hcofg.Enabled, false), + Entry("when the v1 field is false and the v1beta1 FG is false", hcofg.Disabled, false), ) - DescribeTableSubtree("should convert v1beta1 FG to v1 FG, if set, and override the v1 enabled field", func(v1Enabled, v1FG, v1beta1FG bool) { - annotation := fmt.Sprintf(`{"mdevConfigEnable": %t, "disableMDevConfigurationFG": %t}`, v1Enabled, v1FG) + DescribeTableSubtree("should convert v1beta1 FG to v1 FG, if set, and override the v1 enabled field", func(v1Enabled bool, v1FG hcofg.State, v1beta1FG bool) { + annotation := fmt.Sprintf(`{"mdevConfigEnable": %t, "featureGates": [{"name": "disableMDevConfiguration", "state": %q}]}`, v1Enabled, v1FG) It("just make sure the annotation works", func() { src := &HyperConverged{ @@ -828,7 +1362,7 @@ var _ = Describe("api/v1beta1", func() { enabled, found := dst.Spec.FeatureGates.IsExplicitlyEnabled(DisableMDevConfigurationFG) Expect(found).To(BeTrue()) - Expect(enabled).To(Equal(v1FG)) + Expect(enabled).To(Equal(v1FG == hcofg.Enabled)) }) It("should modify the v1 FG and the v1 Enabled field", func() { @@ -856,65 +1390,14 @@ var _ = Describe("api/v1beta1", func() { Expect(enabled).To(Equal(v1beta1FG)) }) }, - Entry("when the v1 field is true, v1 enabled is true, and the v1beta1 FG is true", true, true, true), - Entry("when the v1 field is false, v1 enabled is true, and the v1beta1 FG is true", false, true, true), - Entry("when the v1 field is true, v1 enabled is false, and the v1beta1 FG is true", true, false, true), - Entry("when the v1 field is false, v1 enabled is false, and the v1beta1 FG is true", false, false, true), - Entry("when the v1 field is true, v1 enabled is true, and the v1beta1 FG is false", true, true, false), - Entry("when the v1 field is false, v1 enabled is true, and the v1beta1 FG is false", false, true, false), - Entry("when the v1 field is true, v1 enabled is false, and the v1beta1 FG is false", true, false, false), - Entry("when the v1 field is false, v1 enabled is false, and the v1beta1 FG is false", false, false, false), - ) - - DescribeTableSubtree("should convert v1beta1 FG to v1 enabled, if FG set", func(v1FG bool, v1beta1FG bool) { - annotation := fmt.Sprintf(`{"disableMDevConfigurationFG": %t}`, v1FG) - - It("just make sure the annotation works", func() { - src := &HyperConverged{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1OnlyFieldAnnotation: annotation, - }, - }, - } - dst := &hcov1.HyperConverged{} - - Expect(src.ConvertTo(dst)).To(Succeed()) - - enabled, found := dst.Spec.FeatureGates.IsExplicitlyEnabled(DisableMDevConfigurationFG) - Expect(found).To(BeTrue()) - Expect(enabled).To(Equal(v1FG)) - }) - - It("should modify the v1 FG", func() { - src := &HyperConverged{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1OnlyFieldAnnotation: annotation, - }, - }, - Spec: HyperConvergedSpec{ - FeatureGates: HyperConvergedFeatureGates{ - DisableMDevConfiguration: new(v1beta1FG), - }, - }, - } - dst := &hcov1.HyperConverged{} - - Expect(src.ConvertTo(dst)).To(Succeed()) - - Expect(dst.Spec.Virtualization.MediatedDevicesConfiguration).ToNot(BeNil()) - Expect(dst.Spec.Virtualization.MediatedDevicesConfiguration.Enabled).To(HaveValue(Equal(!v1beta1FG))) - - enabled, found := dst.Spec.FeatureGates.IsExplicitlyEnabled(DisableMDevConfigurationFG) - Expect(found).To(BeTrue()) - Expect(enabled).To(Equal(v1beta1FG)) - }) - }, - Entry("when the v1 field is true and the v1beta1 FG is true", true, true), - Entry("when the v1 field is false and the v1beta1 FG is true", false, true), - Entry("when the v1 field is true and the v1beta1 FG is false", true, false), - Entry("when the v1 field is false and the v1beta1 FG is false", false, false), + Entry("when the v1 field is true, v1 enabled is true, and the v1beta1 FG is true", true, hcofg.Enabled, true), + Entry("when the v1 field is false, v1 enabled is true, and the v1beta1 FG is true", false, hcofg.Enabled, true), + Entry("when the v1 field is true, v1 enabled is false, and the v1beta1 FG is true", true, hcofg.Disabled, true), + Entry("when the v1 field is false, v1 enabled is false, and the v1beta1 FG is true", false, hcofg.Disabled, true), + Entry("when the v1 field is true, v1 enabled is true, and the v1beta1 FG is false", true, hcofg.Enabled, false), + Entry("when the v1 field is false, v1 enabled is true, and the v1beta1 FG is false", false, hcofg.Enabled, false), + Entry("when the v1 field is true, v1 enabled is false, and the v1beta1 FG is false", true, hcofg.Disabled, false), + Entry("when the v1 field is false, v1 enabled is false, and the v1beta1 FG is false", false, hcofg.Disabled, false), ) }) @@ -3305,32 +3788,42 @@ var _ = Describe("api/v1beta1", func() { Entry("when the field is false, and FG is true (implicit)", false, hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG}}, - `{"mdevConfigEnable": false, "disableMDevConfigurationFG": true}`, + `{"mdevConfigEnable": false, "featureGates": [{"name": "disableMDevConfiguration"}]}`, ), Entry("when the field is false, and FG is true", false, hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG, State: new(hcofg.Enabled)}}, - `{"mdevConfigEnable": false, "disableMDevConfigurationFG": true}`, + `{"mdevConfigEnable": false, "featureGates": [{"name": "disableMDevConfiguration", "state": "Enabled"}]}`, ), Entry("when the field is true, and FG is false", true, hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG, State: new(hcofg.Disabled)}}, - `{"mdevConfigEnable": true, "disableMDevConfigurationFG": false}`, + `{"mdevConfigEnable": true, "featureGates": [{"name": "disableMDevConfiguration", "state": "Disabled"}]}`, ), Entry("when the field is true, and FG is true (implicit)", true, hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG}}, - `{"mdevConfigEnable": true, "disableMDevConfigurationFG": true}`, + `{"mdevConfigEnable": true, "featureGates": [{"name": "disableMDevConfiguration"}]}`, ), Entry("when the field is true, and FG is true", true, hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG, State: new(hcofg.Enabled)}}, - `{"mdevConfigEnable": true, "disableMDevConfigurationFG": true}`, + `{"mdevConfigEnable": true, "featureGates": [{"name": "disableMDevConfiguration", "state": "Enabled"}]}`, ), Entry("when the field is false, and FG is false", false, hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG, State: new(hcofg.Disabled)}}, - `{"mdevConfigEnable": false, "disableMDevConfigurationFG": false}`, + `{"mdevConfigEnable": false, "featureGates": [{"name": "disableMDevConfiguration", "state": "Disabled"}]}`, + ), + Entry("when the field is true, and FG is true, with different casing", + true, + hcofg.HyperConvergedFeatureGates{{Name: "DISABLEMDEVCONFIGURATIONFG", State: new(hcofg.Enabled)}}, + `{"mdevConfigEnable": true, "featureGates": [{"name": "DISABLEMDEVCONFIGURATIONFG", "state": "Enabled"}]}`, + ), + Entry("when the field is false, and FG is false, with different casing", + false, + hcofg.HyperConvergedFeatureGates{{Name: "DisableMDevConfiguratioN", State: new(hcofg.Disabled)}}, + `{"mdevConfigEnable": false, "featureGates": [{"name": "DisableMDevConfiguratioN", "state": "Disabled"}]}`, ), ) @@ -3372,25 +3865,31 @@ var _ = Describe("api/v1beta1", func() { Entry("when FG list is empty", hcofg.HyperConvergedFeatureGates{}, Not(HaveKey(v1OnlyFieldAnnotation)), false, false), Entry("when the disableMDevConfiguration FG is not set", hcofg.HyperConvergedFeatureGates{{Name: "somethingElse"}}, + HaveKeyWithValue(v1OnlyFieldAnnotation, MatchJSON(`{"featureGates": [{"name": "somethingElse"}]}`)), + false, + false, + ), + Entry("when the no FG is not set", + nil, Not(HaveKey(v1OnlyFieldAnnotation)), false, false, ), Entry("when the disableMDevConfiguration FG is implicitly enabled", - hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG}}, // `{"deployNetworkResourcesInjector": false}`, - HaveKeyWithValue(v1OnlyFieldAnnotation, MatchJSON(`{"disableMDevConfigurationFG": true}`)), + hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG}}, + HaveKeyWithValue(v1OnlyFieldAnnotation, MatchJSON(`{"featureGates": [{"name": "disableMDevConfiguration"}]}`)), true, true, ), Entry("when the disableMDevConfiguration FG is explicitly enabled", hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG, State: new(hcofg.Enabled)}}, - HaveKeyWithValue(v1OnlyFieldAnnotation, MatchJSON(`{"disableMDevConfigurationFG": true}`)), + HaveKeyWithValue(v1OnlyFieldAnnotation, MatchJSON(`{"featureGates": [{"name": "disableMDevConfiguration", "state": "Enabled"}]}`)), true, true, ), Entry("when the disableMDevConfiguration FG is disabled", hcofg.HyperConvergedFeatureGates{{Name: DisableMDevConfigurationFG, State: new(hcofg.Disabled)}}, - HaveKeyWithValue(v1OnlyFieldAnnotation, MatchJSON(`{"disableMDevConfigurationFG": false}`)), + HaveKeyWithValue(v1OnlyFieldAnnotation, MatchJSON(`{"featureGates": [{"name": "disableMDevConfiguration", "state": "Disabled"}]}`)), false, true, ), @@ -3405,13 +3904,21 @@ var _ = Describe("api/v1beta1", func() { Enabled: new(false), } v1HC.Spec.FeatureGates.Enable(DisableMDevConfigurationFG) + v1HC.Spec.FeatureGates.Disable("aDisabledFG") + v1HC.Spec.FeatureGates.Enable("explicitlyEnabledFG") + v1HC.Spec.FeatureGates = append(v1HC.Spec.FeatureGates, hcofg.FeatureGate{Name: "implicitlyEnabledFG"}) v1beta1HC := &HyperConverged{} Expect(v1beta1HC.ConvertFrom(v1HC)).To(Succeed()) const expectedJSONAnnotation = `{ "deployNetworkResourcesInjector": false, "mdevConfigEnable": false, - "disableMDevConfigurationFG": true + "featureGates": [ + {"name": "disableMDevConfiguration", "state": "Enabled"}, + {"name": "aDisabledFG", "state": "Disabled"}, + {"name": "explicitlyEnabledFG", "state": "Enabled"}, + {"name": "implicitlyEnabledFG"} + ] }` Expect(v1beta1HC.Annotations).To(HaveKeyWithValue(v1OnlyFieldAnnotation, MatchJSON(expectedJSONAnnotation))) @@ -3422,6 +3929,20 @@ var _ = Describe("api/v1beta1", func() { Expect(roundTripHC.Spec.Deployment.DeployNetworkResourcesInjector).To(HaveValue(BeFalse())) Expect(roundTripHC.Spec.Virtualization.MediatedDevicesConfiguration).ToNot(BeNil()) Expect(roundTripHC.Spec.Virtualization.MediatedDevicesConfiguration.Enabled).To(HaveValue(BeFalse())) + + Expect(roundTripHC.Spec.FeatureGates.IsEnabled(DisableMDevConfigurationFG)).To(BeTrue()) + + enabled, found := roundTripHC.Spec.FeatureGates.IsExplicitlyEnabled("aDisabledFG") + Expect(found).To(BeTrue()) + Expect(enabled).To(BeFalse()) + + enabled, found = roundTripHC.Spec.FeatureGates.IsExplicitlyEnabled("explicitlyEnabledFG") + Expect(found).To(BeTrue()) + Expect(enabled).To(BeTrue()) + + enabled, found = roundTripHC.Spec.FeatureGates.IsExplicitlyEnabled("implicitlyEnabledFG") + Expect(found).To(BeTrue()) + Expect(enabled).To(BeTrue()) }) }) }) diff --git a/api/v1beta1/zz_generated.featuregates_conversion.go b/api/v1beta1/zz_generated.featuregates_conversion.go index 021129e688..a5a096a408 100644 --- a/api/v1beta1/zz_generated.featuregates_conversion.go +++ b/api/v1beta1/zz_generated.featuregates_conversion.go @@ -12,60 +12,196 @@ package v1beta1 import ( - "k8s.io/utils/ptr" + "slices" hcofg "github.com/kubevirt/hyperconverged-cluster-operator/api/v1/featuregates" ) func convert_v1beta1_FeatureGates_To_v1(in *HyperConvergedFeatureGates, out *hcofg.HyperConvergedFeatureGates) { + var ( + v1Idx int + v1Found bool + v1Enabled bool + ) + // converting the DecentralizedLiveMigration v1beta1 beta feature gate to v1 - if !ptr.Deref(in.DecentralizedLiveMigration, true) { - out.Disable("decentralizedLiveMigration") + v1Idx = out.Index("decentralizedLiveMigration") + v1Found = v1Idx >= 0 + if in.DecentralizedLiveMigration == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && !*in.DecentralizedLiveMigration) || (v1Found && v1Enabled != *in.DecentralizedLiveMigration) { + if *in.DecentralizedLiveMigration { + out.Enable("decentralizedLiveMigration") + } else { + out.Disable("decentralizedLiveMigration") + } + } } // converting the DeclarativeHotplugVolumes v1beta1 beta feature gate to v1 - if !ptr.Deref(in.DeclarativeHotplugVolumes, true) { - out.Disable("declarativeHotplugVolumes") + v1Idx = out.Index("declarativeHotplugVolumes") + v1Found = v1Idx >= 0 + if in.DeclarativeHotplugVolumes == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && !*in.DeclarativeHotplugVolumes) || (v1Found && v1Enabled != *in.DeclarativeHotplugVolumes) { + if *in.DeclarativeHotplugVolumes { + out.Enable("declarativeHotplugVolumes") + } else { + out.Disable("declarativeHotplugVolumes") + } + } } // converting the AlignCPUs v1beta1 alpha feature gate to v1 - if ptr.Deref(in.AlignCPUs, false) { - out.Enable("alignCPUs") + v1Idx = out.Index("alignCPUs") + v1Found = v1Idx >= 0 + if in.AlignCPUs == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && *in.AlignCPUs) || (v1Found && v1Enabled != *in.AlignCPUs) { + if *in.AlignCPUs { + out.Enable("alignCPUs") + } else { + out.Disable("alignCPUs") + } + } } // converting the ContainerPathVolumes v1beta1 alpha feature gate to v1 - if ptr.Deref(in.ContainerPathVolumes, false) { - out.Enable("containerPathVolumes") + v1Idx = out.Index("containerPathVolumes") + v1Found = v1Idx >= 0 + if in.ContainerPathVolumes == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && *in.ContainerPathVolumes) || (v1Found && v1Enabled != *in.ContainerPathVolumes) { + if *in.ContainerPathVolumes { + out.Enable("containerPathVolumes") + } else { + out.Disable("containerPathVolumes") + } + } } // converting the DeployKubeSecondaryDNS v1beta1 alpha feature gate to v1 - if ptr.Deref(in.DeployKubeSecondaryDNS, false) { - out.Enable("deployKubeSecondaryDNS") + v1Idx = out.Index("deployKubeSecondaryDNS") + v1Found = v1Idx >= 0 + if in.DeployKubeSecondaryDNS == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && *in.DeployKubeSecondaryDNS) || (v1Found && v1Enabled != *in.DeployKubeSecondaryDNS) { + if *in.DeployKubeSecondaryDNS { + out.Enable("deployKubeSecondaryDNS") + } else { + out.Disable("deployKubeSecondaryDNS") + } + } } // converting the DownwardMetrics v1beta1 alpha feature gate to v1 - if ptr.Deref(in.DownwardMetrics, false) { - out.Enable("downwardMetrics") + v1Idx = out.Index("downwardMetrics") + v1Found = v1Idx >= 0 + if in.DownwardMetrics == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && *in.DownwardMetrics) || (v1Found && v1Enabled != *in.DownwardMetrics) { + if *in.DownwardMetrics { + out.Enable("downwardMetrics") + } else { + out.Disable("downwardMetrics") + } + } } // converting the EnableMultiArchBootImageImport v1beta1 alpha feature gate to v1 - if ptr.Deref(in.EnableMultiArchBootImageImport, false) { - out.Enable("enableMultiArchBootImageImport") + v1Idx = out.Index("enableMultiArchBootImageImport") + v1Found = v1Idx >= 0 + if in.EnableMultiArchBootImageImport == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && *in.EnableMultiArchBootImageImport) || (v1Found && v1Enabled != *in.EnableMultiArchBootImageImport) { + if *in.EnableMultiArchBootImageImport { + out.Enable("enableMultiArchBootImageImport") + } else { + out.Disable("enableMultiArchBootImageImport") + } + } } // converting the IncrementalBackup v1beta1 alpha feature gate to v1 - if ptr.Deref(in.IncrementalBackup, false) { - out.Enable("incrementalBackup") + v1Idx = out.Index("incrementalBackup") + v1Found = v1Idx >= 0 + if in.IncrementalBackup == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && *in.IncrementalBackup) || (v1Found && v1Enabled != *in.IncrementalBackup) { + if *in.IncrementalBackup { + out.Enable("incrementalBackup") + } else { + out.Disable("incrementalBackup") + } + } } // converting the ObjectGraph v1beta1 alpha feature gate to v1 - if ptr.Deref(in.ObjectGraph, false) { - out.Enable("objectGraph") + v1Idx = out.Index("objectGraph") + v1Found = v1Idx >= 0 + if in.ObjectGraph == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && *in.ObjectGraph) || (v1Found && v1Enabled != *in.ObjectGraph) { + if *in.ObjectGraph { + out.Enable("objectGraph") + } else { + out.Disable("objectGraph") + } + } } // converting the PersistentReservation v1beta1 alpha feature gate to v1 - if ptr.Deref(in.PersistentReservation, false) { - out.Enable("persistentReservation") + v1Idx = out.Index("persistentReservation") + v1Found = v1Idx >= 0 + if in.PersistentReservation == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && *in.PersistentReservation) || (v1Found && v1Enabled != *in.PersistentReservation) { + if *in.PersistentReservation { + out.Enable("persistentReservation") + } else { + out.Disable("persistentReservation") + } + } } // the DisableMDevConfiguration feature gate is deprecated and should have a custom conversion logic in api/v1beta1/conversion.go diff --git a/tools/fg-conversion-generator/conversion.go.tmpl b/tools/fg-conversion-generator/conversion.go.tmpl index 388e0d81cd..220149aa6e 100644 --- a/tools/fg-conversion-generator/conversion.go.tmpl +++ b/tools/fg-conversion-generator/conversion.go.tmpl @@ -12,20 +12,38 @@ package v1beta1 import ( - "k8s.io/utils/ptr" + "slices" hcofg "github.com/kubevirt/hyperconverged-cluster-operator/api/v1/featuregates" ) func convert_v1beta1_FeatureGates_To_v1(in *HyperConvergedFeatureGates, out *hcofg.HyperConvergedFeatureGates) { -{{- range .}} + var ( + v1Idx int + v1Found bool + v1Enabled bool + ) +{{ range . -}} {{- if .IsDeprecated }} // the {{.FieldName}} feature gate is deprecated and should have a custom conversion logic in api/v1beta1/conversion.go {{- else }} // converting the {{.FieldName}} v1beta1 {{.Phase.String}} feature gate to v1 {{- $IsBeta := .IsBeta}} - if {{if $IsBeta}}!{{end}}ptr.Deref(in.{{.FieldName}}, {{if $IsBeta}}true{{else}}false{{end}}) { - out.{{if $IsBeta}}Disable{{else}}Enable{{end}}("{{.JSONName}}") + v1Idx = out.Index("{{.JSONName}}") + v1Found = v1Idx >= 0 + if in.{{.FieldName}} == nil { + if v1Found { + *out = slices.Delete(*out, v1Idx, v1Idx+1) + } + } else { + v1Enabled = v1Found && ((*out)[v1Idx].State == nil || *((*out)[v1Idx].State) == "Enabled") + if (!v1Found && {{if $IsBeta}}!{{end}}*in.{{.FieldName}}) || (v1Found && v1Enabled != *in.{{.FieldName}}) { + if *in.{{.FieldName}} { + out.Enable("{{.JSONName}}") + } else { + out.Disable("{{.JSONName}}") + } + } } {{- end}} {{end -}}