fix flaky functional test

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
+				}
+			}
+		}
+	}
+}

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())
+		})
+	})
+})

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 ""
+}