update:

- fix order for kueue, sinde the deploy need to be called after we get all manifests
- testcase to explictly check error matching no CRD of VAP/VAPB found for < 4.17
- cannot use cluster.GetClusterInfo since that is init/set in the main, tc wont have that value

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

controllers/components/kueue/kueue_controller.go

@@ -44,9 +44,15 @@ import (
 )
 
 func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
-	b := reconciler.ReconcilerFor(mgr, &componentApi.Kueue{}).
-		// customized Owns() for Component with new predicates
-		Owns(&corev1.ConfigMap{}).
+	b := reconciler.ReconcilerFor(mgr, &componentApi.Kueue{})
+
+	if cluster.GetClusterInfo().Version.GTE(semver.MustParse("4.17.0")) {
+		b = b.OwnsGVK(gvk.ValidatingAdmissionPolicy). // "own" VAP, because we want it has owner so when kueue is removed it gets cleaned.
+								WatchesGVK(gvk.ValidatingAdmissionPolicyBinding). // "watch" VAPB, because we want it to be configable by user and it can be left behind when kueue is remov
+								WithAction(extraInitialize)
+	}
+	// customized Owns() for Component with new predicates
+	b.Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.Secret{}).
 		Owns(&rbacv1.ClusterRoleBinding{}).
 		Owns(&rbacv1.ClusterRole{}).
@@ -83,12 +89,6 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		// must be the final action
 		WithAction(gc.NewAction())
 
-	if cluster.GetClusterInfo().Version.GTE(semver.MustParse("4.17.0")) {
-		b = b.OwnsGVK(gvk.ValidatingAdmissionPolicy)           // "own" VAP, because we want it has owner so when kueue is removed it gets cleaned.
-		b = b.WatchesGVK(gvk.ValidatingAdmissionPolicyBinding) // "watch" VAPB, because we want it to be configable by user and it can be left behind when kueue is remov
-		b = b.WithAction(extraInitialize)
-	}
-
 	if _, err := b.Build(ctx); err != nil {
 		return err // no need customize error, it is done in the caller main
 	}

tests/e2e/components_test.go

@@ -6,6 +6,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/blang/semver/v4"
+	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -15,6 +17,7 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/apis/common"
 	dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
@@ -278,3 +281,13 @@ func (c *ComponentTestCtx) GetDSCI() (*dsciv1.DSCInitialization, error) {
 
 	return &obj, nil
 }
+
+func (c *ComponentTestCtx) GetClusterVersion() (semver.Version, error) {
+	clusterVersion := &configv1.ClusterVersion{}
+	if err := c.Client().Get(c.Context(), client.ObjectKey{
+		Name: cluster.OpenShiftVersionObj,
+	}, clusterVersion); err != nil {
+		return semver.Version{}, err
+	}
+	return semver.ParseTolerant(clusterVersion.Status.History[0].Version)
+}

tests/e2e/kueue_test.go

@@ -2,17 +2,15 @@ package e2e_test
 
 import (
 	"testing"
+	"time"
 
 	"github.com/blang/semver/v4"
 	"github.com/stretchr/testify/require"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/types"
 
 	componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq"
 
 	. "github.com/onsi/gomega"
@@ -41,25 +39,20 @@ type KueueTestCtx struct {
 
 func (tc *KueueTestCtx) validateKueueVAPReady(t *testing.T) {
 	g := tc.NewWithT(t)
-	if cluster.GetClusterInfo().Version.GTE(semver.MustParse("4.17.0")) {
-		g.Get(gvk.ValidatingAdmissionPolicy, types.NamespacedName{Name: "kueue-validating-admission-policy"}).Eventually().Should(
-			jq.Match(`.metadata.ownerReferences == "%s"`, componentApi.KueueInstanceName),
+	v, err := tc.GetClusterVersion()
+	g.Expect(err).ToNot(HaveOccurred())
+	if v.GTE(semver.MustParse("4.17.0")) {
+		g.Get(gvk.ValidatingAdmissionPolicy, types.NamespacedName{Name: "kueue-validating-admission-policy"}).Eventually().WithTimeout(1 * time.Second).
+			Should(
+				jq.Match(`.metadata.ownerReferences[0].name == "%s"`, componentApi.KueueInstanceName),
+			)
+		g.Get(gvk.ValidatingAdmissionPolicyBinding, types.NamespacedName{Name: "kueue-validating-admission-policy-binding"}).Eventually().WithTimeout(1 * time.Second).Should(
+			jq.Match(`.metadata.ownerReferences | length == 0`),
 		)
-		vapb, err := g.Get(gvk.ValidatingAdmissionPolicyBinding, types.NamespacedName{Name: "kueue-validating-admission-policy-binding"}).Get()
-		g.Expect(err).ToNot(HaveOccurred())
-		g.Expect(vapb.GetOwnerReferences()).Should(BeEmpty())
 		return
 	}
-	scheme := runtime.NewScheme()
-	vap := &unstructured.Unstructured{}
-	vap.SetKind(gvk.ValidatingAdmissionPolicy.Kind)
-	err := resources.EnsureGroupVersionKind(scheme, vap)
-	g.Expect(err).To(HaveOccurred())
-	g.Expect(err.Error()).To(ContainSubstring("failed to get GVK"))
-
-	vapb := &unstructured.Unstructured{}
-	vapb.SetKind(gvk.ValidatingAdmissionPolicyBinding.Kind)
-	err = resources.EnsureGroupVersionKind(scheme, vapb)
-	g.Expect(err).To(HaveOccurred())
-	g.Expect(err.Error()).To(ContainSubstring("failed to get GVK"))
+	_, err = g.Get(gvk.ValidatingAdmissionPolicy, types.NamespacedName{Name: "kueue-validating-admission-policy"}).Get()
+	g.Expect(err).Should(MatchError(&meta.NoKindMatchError{}))
+	_, err = g.Get(gvk.ValidatingAdmissionPolicyBinding, types.NamespacedName{Name: "kueue-validating-admission-policy-binding"}).Get()
+	g.Expect(err).Should(MatchError(&meta.NoKindMatchError{}))
 }