diff --git a/pkg/featuregatedetails/feature_gates.go b/pkg/featuregatedetails/feature_gates.go index ffcdd81275..873a1a293e 100644 --- a/pkg/featuregatedetails/feature_gates.go +++ b/pkg/featuregatedetails/feature_gates.go @@ -24,6 +24,14 @@ func GetFeatureGatePhase(fgName string) (enabled featuregates.Phase, fgExists bo return fg.Phase, true } +func ListBetaFeatureGates() []string { + return slices.Sorted(filterFGByPhase(maps.All(featureGatesDetails), featuregates.PhaseBeta)) +} + +func ListAlphaFeatureGates() []string { + return slices.Sorted(filterFGByPhase(maps.All(featureGatesDetails), featuregates.PhaseAlpha)) +} + func init() { if err := setup(featureGateJson); err != nil { panic("unable to setup v1 feature gates;" + err.Error()) @@ -51,3 +59,15 @@ 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 { + if fg.Phase == phase { + if !yield(fgName) { + return + } + } + } + } +} diff --git a/pkg/featuregatedetails/feature_gates_test.go b/pkg/featuregatedetails/feature_gates_test.go new file mode 100644 index 0000000000..b9a8239cc0 --- /dev/null +++ b/pkg/featuregatedetails/feature_gates_test.go @@ -0,0 +1,128 @@ +package featuregatedetails + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/kubevirt/hyperconverged-cluster-operator/pkg/featuregates" +) + +func TestFeatureGateDetails(t *testing.T) { + RegisterFailHandler(Fail) + BeforeSuite(func() { + origFeatureGatesDetails := featureGatesDetails + DeferCleanup(func() { + featureGatesDetails = origFeatureGatesDetails + }) + }) + + RunSpecs(t, "Feature Gate Details Suite") +} + +var _ = Describe("Feature Gate Details", func() { + Context("GetFeatureGatePhase", func() { + It("should return the phase of the feature gate", 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}, + } + + _, exists := GetFeatureGatePhase("notExist") + Expect(exists).To(BeFalse()) + }) + }) + + Context("ListBetaFeatureGates", func() { + It("should return the list of beta feature gates", func() { + featureGatesDetails = map[string]featuregates.FeatureGate{ + "fg1": {Name: "fg1", Phase: featuregates.PhaseGA}, + "alpha1": {Name: "alpha1", Phase: featuregates.PhaseAlpha}, + "beta1": {Name: "beta1", Phase: featuregates.PhaseBeta}, + "fg4": {Name: "fg2", Phase: featuregates.PhaseUnknown}, + "fg5": {Name: "fg3", Phase: featuregates.PhaseDeprecated}, + "fg6": {Name: "fg4", Phase: featuregates.PhaseDiscontinued}, + "alpha2": {Name: "alpha2", Phase: featuregates.PhaseAlpha}, + "beta2": {Name: "beta2", Phase: featuregates.PhaseBeta}, + "beta3": {Name: "beta3", Phase: featuregates.PhaseBeta}, + } + + betaFGs := ListBetaFeatureGates() + Expect(betaFGs).To(HaveLen(3)) + Expect(betaFGs).To(ContainElements("beta1", "beta2", "beta3")) + }) + + It("should return an empty list if no beta FG is defined", func() { + featureGatesDetails = map[string]featuregates.FeatureGate{ + "fg1": {Name: "fg1", Phase: featuregates.PhaseGA}, + "alpha1": {Name: "alpha1", Phase: featuregates.PhaseAlpha}, + "fg4": {Name: "fg2", Phase: featuregates.PhaseUnknown}, + "fg5": {Name: "fg3", Phase: featuregates.PhaseDeprecated}, + "fg6": {Name: "fg4", Phase: featuregates.PhaseDiscontinued}, + "alpha2": {Name: "alpha2", Phase: featuregates.PhaseAlpha}, + } + + betaFGs := ListBetaFeatureGates() + Expect(betaFGs).To(BeEmpty()) + }) + + It("should return an empty list if no FG is defined", func() { + featureGatesDetails = nil + + betaFGs := ListBetaFeatureGates() + Expect(betaFGs).To(BeEmpty()) + }) + }) + + Context("ListAlphaFeatureGates", func() { + It("should return the list of alpha feature gates", func() { + featureGatesDetails = map[string]featuregates.FeatureGate{ + "fg1": {Name: "fg1", Phase: featuregates.PhaseGA}, + "alpha1": {Name: "alpha1", Phase: featuregates.PhaseAlpha}, + "beta1": {Name: "beta1", Phase: featuregates.PhaseBeta}, + "fg4": {Name: "fg2", Phase: featuregates.PhaseUnknown}, + "fg5": {Name: "fg3", Phase: featuregates.PhaseDeprecated}, + "fg6": {Name: "fg4", Phase: featuregates.PhaseDiscontinued}, + "alpha2": {Name: "alpha2", Phase: featuregates.PhaseAlpha}, + "beta2": {Name: "beta2", Phase: featuregates.PhaseBeta}, + "beta3": {Name: "beta2", Phase: featuregates.PhaseBeta}, + } + + alphaFGs := ListAlphaFeatureGates() + Expect(alphaFGs).To(HaveLen(2)) + Expect(alphaFGs).To(ContainElements("alpha1", "alpha2")) + }) + + It("should return an empty list if no alpha FG is defined", func() { + featureGatesDetails = map[string]featuregates.FeatureGate{ + "fg1": {Name: "fg1", Phase: featuregates.PhaseGA}, + "beta1": {Name: "beta1", Phase: featuregates.PhaseBeta}, + "fg4": {Name: "fg2", Phase: featuregates.PhaseUnknown}, + "fg5": {Name: "fg3", Phase: featuregates.PhaseDeprecated}, + "fg6": {Name: "fg4", Phase: featuregates.PhaseDiscontinued}, + "beta2": {Name: "beta2", Phase: featuregates.PhaseBeta}, + "beta3": {Name: "beta2", Phase: featuregates.PhaseBeta}, + } + + alphaFGs := ListAlphaFeatureGates() + Expect(alphaFGs).To(BeEmpty()) + }) + + It("should return an empty list if no FG is defined", func() { + featureGatesDetails = nil + + betaFGs := ListAlphaFeatureGates() + Expect(betaFGs).To(BeEmpty()) + }) + }) +}) diff --git a/tests/func-tests/conversion_test.go b/tests/func-tests/conversion_test.go index 9801018996..0b5f1f34f0 100644 --- a/tests/func-tests/conversion_test.go +++ b/tests/func-tests/conversion_test.go @@ -11,24 +11,29 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" hcov1 "github.com/kubevirt/hyperconverged-cluster-operator/api/v1" "github.com/kubevirt/hyperconverged-cluster-operator/api/v1/featuregates" hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/api/v1beta1" + "github.com/kubevirt/hyperconverged-cluster-operator/pkg/featuregatedetails" hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" tests "github.com/kubevirt/hyperconverged-cluster-operator/tests/func-tests" ) -var _ = Describe("check v1 <=> v1beta1 API conversion", func() { +var _ = Describe("check v1 <=> v1beta1 API conversion", Label("CONVERSION"), func() { var ( hcKey client.ObjectKey cli client.Client ) - BeforeEach(func() { + BeforeEach(func(ctx context.Context) { hcKey = client.ObjectKey{Namespace: tests.InstallNamespace, Name: hcoutil.HyperConvergedName} cli = tests.GetControllerRuntimeClient() + + By("Make sure feature gates are with default values") + restoreFGsToDefault(ctx, cli) }) It("naively read HCO in v1beta1 format", func(ctx context.Context) { @@ -65,33 +70,80 @@ var _ = Describe("check v1 <=> v1beta1 API conversion", func() { }) It("should allow set fields in HyperConverged v1beta1", func(ctx context.Context) { - By("Make sure feature gates are with default values") - restoreFGsToDefault(ctx, cli) + betaFG := getV1beta1FeatureGate(featuregatedetails.ListBetaFeatureGates) + alphaFG := getV1beta1FeatureGate(featuregatedetails.ListAlphaFeatureGates) + + var patchFGs []string + + if betaFG != "" { + GinkgoLogr.Info("found a beta feature gate with a field in v1beta1 API version", "name", betaFG) + patchFGs = []string{fmt.Sprintf(`%q: false`, betaFG)} + } else { + GinkgoLogr.Info("no beta feature gate defined in v1beta1 API version") + } - By("read HyperConverged in v1beta1 format, then update two FGs") - Eventually(func(g Gomega, ctx context.Context) { - hcv1beta1 := &hcov1beta1.HyperConverged{} - g.Expect(cli.Get(ctx, hcKey, hcv1beta1)).To(Succeed()) + if alphaFG != "" { + GinkgoLogr.Info("found an alpha feature gate with a field in v1beta1 API version", "name", alphaFG) + patchFGs = append(patchFGs, fmt.Sprintf(`%q: true`, alphaFG)) + } else { + GinkgoLogr.Info("no alpha feature gate defined in v1beta1 API version") + } - hcv1beta1.Spec.FeatureGates.VideoConfig = new(false) - hcv1beta1.Spec.FeatureGates.DownwardMetrics = new(true) + if betaFG == "" && alphaFG == "" { + // should not happen until dropping the v1beta API version, when we'll drop the whole file anyway + Skip("no alpha or beta feature gates found in v1beta1 API version; skipping this test") + } - g.Expect(cli.Update(ctx, hcv1beta1)).To(Succeed()) + DeferCleanup(func(ctx context.Context) { + By("restore the FGs") + restoreFGsToDefault(ctx, cli) + }) + + By("patch the HyperConverged to modify two FGs, in v1beta1 format") + patch := fmt.Appendf(nil, `{"spec":{"featureGates": {%s}}}`, strings.Join(patchFGs, ",")) + GinkgoLogr.Info("patching v1beta1 Feature gates", "path", string(patch)) + + hcv1beta1 := &hcov1beta1.HyperConverged{ + ObjectMeta: metav1.ObjectMeta{ + Name: hcoutil.HyperConvergedName, + Namespace: tests.InstallNamespace, + }, + } + + Eventually(func(ctx context.Context) error { + + return cli.Patch(ctx, hcv1beta1, client.RawPatch(types.MergePatchType, patch)) }).WithTimeout(60 * time.Second). WithPolling(time.Second). WithContext(ctx). Should(Succeed()) - By("read HyperConverged in v1 format after the v1beta1 update") + By("validate the feature gates in HyperConverged v1 format after the v1beta1 update") hcv1 := &hcov1.HyperConverged{} Expect(cli.Get(ctx, hcKey, hcv1)).To(Succeed()) - Expect(hcv1.Spec.FeatureGates.IsEnabled("downwardMetrics")).To(BeTrueBecause("downwardMetrics was enabled using v1beta1 API. it is expected to be 'true' in v1, but it's not'")) - Expect(hcv1.Spec.FeatureGates.IsEnabled("videoConfig")).To(BeFalseBecause("videoConfig was disabled using v1beta1 API. it is expected to be 'false' in v1, but it's not'")) + if betaFG != "" { + Expect(hcv1.Spec.FeatureGates.IsEnabled(betaFG)).To(BeFalseBecause("the %q beta feature gate was disabled using v1beta1 API. it is expected to be 'false' in v1, but it's not", betaFG)) + } - DeferCleanup(func(ctx context.Context) { - By("restore the FGs") - restoreFGsToDefault(ctx, cli) - }) + if alphaFG != "" { + Expect(hcv1.Spec.FeatureGates.IsEnabled(alphaFG)).To(BeTrueBecause("the %q alpha feature gate was enabled using v1beta1 API. it is expected to be 'true' in v1, but it's not", alphaFG)) + } + + By("Check v1 <==> v1beta1 conversion, with non-empty feature gate list") + Eventually(func(ctx context.Context) error { + return cli.Get(ctx, hcKey, hcv1beta1) + }).WithContext(ctx). + WithTimeout(60 * time.Second). + WithPolling(time.Second). + Should(Succeed()) + + converted := &hcov1.HyperConverged{} + Expect(hcv1beta1.ConvertTo(converted)).To(Succeed()) + diff := cmp.Diff(converted.Spec, hcv1.Spec) + if diff != "" { + GinkgoWriter.Println(diff) + Fail("v1 HyperConverged should be equal to the v1beta1 converted one") + } }) }) @@ -142,7 +194,7 @@ func restoreFGsToDefault(ctx context.Context, cl client.Client) { hco, err := tests.GetHCO(ctx, cl) g.Expect(err).ToNot(HaveOccurred()) - if len(hco.Spec.FeatureGates) == 0 { + if hco.Spec.FeatureGates == nil { return } @@ -174,3 +226,27 @@ func restoreFGsToDefault(ctx context.Context, cl client.Client) { WithContext(ctx). Should(Succeed()) } + +func isFGExistInV1beta1(fgName string) bool { + for fld := range reflect.TypeFor[hcov1beta1.HyperConvergedFeatureGates]().Fields() { + if names := strings.Split(fld.Tag.Get("json"), ","); len(names) > 0 { + if fgName == names[0] { + return true + } + } + } + + return false +} + +// getV1beta1FeatureGate receives a (function that returns a) list of FG names +// it returns the first FG name that is also exists as a field in v1beta1 FG struct. +func getV1beta1FeatureGate(getter func() []string) string { + for _, fg := range getter() { + if isFGExistInV1beta1(fg) { + return fg + } + } + + return "" +}