[sync] add support for kueue to work with VAP on OCP 4.16+ (#1480) (#…

…1515)

* feat: add support for kueue to work with VAP on OCP 4.16+ (#1480)

* feat: add support for kueue to work with VAP on OCP 4.16+

- only add VAP specific manifests into list if latest OCP version in history is over 16 in min version
  update: explicit check 4.17.0 not just minor version
- now each component can get ocp version without calculate
- add test in kueue
- make VAPB object user configable (as no ownerreference set)
- user cannot update VAP object ( it get reconciled back to default value)
- 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>

* test: move check VAP/VAPB after test of "kueue CR ready"

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

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
(cherry picked from commit e6235d4)

* test: reset back kserve to use manifests from 2.16

    - there is a bug in the latest 2.17, without fix, our e2e wont work
    - this is just a workaround

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

---------

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

bundle/manifests/rhods-operator.clusterserviceversion.yaml

@@ -372,6 +372,8 @@ spec:
           - admissionregistration.k8s.io
           resources:
           - mutatingwebhookconfigurations
+          - validatingadmissionpolicies
+          - validatingadmissionpolicybindings
           - validatingwebhookconfigurations
           verbs:
           - create

config/rbac/role.yaml

@@ -28,6 +28,8 @@ rules:
   - admissionregistration.k8s.io
   resources:
   - mutatingwebhookconfigurations
+  - validatingadmissionpolicies
+  - validatingadmissionpolicybindings
   - validatingwebhookconfigurations
   verbs:
   - create

controllers/components/kueue/kueue_controller.go

@@ -19,6 +19,7 @@ package kueue
 import (
 	"context"
 
+	"github.com/blang/semver/v4"
 	promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
@@ -29,6 +30,8 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	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/controller/actions/deploy"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
@@ -41,9 +44,15 @@ import (
 )
 
 func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
-	_, err := 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{}).
@@ -72,15 +81,15 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 			kustomize.WithLabel(labels.ODH.Component(LegacyComponentName), labels.True),
 			kustomize.WithLabel(labels.K8SCommon.PartOf, LegacyComponentName),
 		)).
+		WithAction(customizeResources).
 		WithAction(deploy.NewAction(
 			deploy.WithCache(),
 		)).
 		WithAction(updatestatus.NewAction()).
 		// must be the final action
-		WithAction(gc.NewAction()).
-		Build(ctx)
+		WithAction(gc.NewAction())
 
-	if err != nil {
+	if _, err := b.Build(ctx); err != nil {
 		return err // no need customize error, it is done in the caller main
 	}
 

controllers/components/kueue/kueue_controller_actions.go

@@ -5,13 +5,21 @@ import (
 	"fmt"
 
 	componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
 	odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
 	odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
 )
 
 func initialize(_ context.Context, rr *odhtypes.ReconciliationRequest) error {
 	rr.Manifests = append(rr.Manifests, manifestsPath())
+	return nil
+}
 
+func extraInitialize(_ context.Context, rr *odhtypes.ReconciliationRequest) error {
+	// Add specific manifests if OCP is greater or equal 4.17.
+	rr.Manifests = append(rr.Manifests, extramanifestsPath())
 	return nil
 }
 
@@ -42,3 +50,14 @@ func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
 
 	return nil
 }
+
+func customizeResources(_ context.Context, rr *odhtypes.ReconciliationRequest) error {
+	for i := range rr.Resources {
+		if rr.Resources[i].GroupVersionKind() == gvk.ValidatingAdmissionPolicyBinding {
+			// admin can update this resource
+			resources.SetAnnotation(&rr.Resources[i], annotations.ManagedByODHOperator, "false")
+			break // fast exist function
+		}
+	}
+	return nil
+}

controllers/components/kueue/kueue_support.go

@@ -33,3 +33,11 @@ func manifestsPath() odhtypes.ManifestInfo {
 		SourcePath: "rhoai",
 	}
 }
+
+func extramanifestsPath() odhtypes.ManifestInfo {
+	return odhtypes.ManifestInfo{
+		Path:       odhdeploy.DefaultManifestPath,
+		ContextDir: ComponentName,
+		SourcePath: "rhoai/ocp-4.17-addons",
+	}
+}

controllers/datasciencecluster/kubebuilder_rbac.go

@@ -205,6 +205,8 @@ package datasciencecluster
 // +kubebuilder:rbac:groups=components.platform.opendatahub.io,resources=kueues/finalizers,verbs=update
 // +kubebuilder:rbac:groups="monitoring.coreos.com",resources=prometheusrules,verbs=get;create;patch;delete;deletecollection;list;watch
 // +kubebuilder:rbac:groups="monitoring.coreos.com",resources=podmonitors,verbs=get;create;delete;update;watch;list;patch
+// +kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=validatingadmissionpolicybindings,verbs=get;create;delete;update;watch;list;patch
+// +kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=validatingadmissionpolicies,verbs=get;create;delete;update;watch;list;patch
 
 // CFO
 //+kubebuilder:rbac:groups=components.platform.opendatahub.io,resources=codeflares,verbs=get;list;watch;create;update;patch;delete
@@ -240,7 +242,7 @@ package datasciencecluster
 // +kubebuilder:rbac:groups="operator.knative.dev",resources=knativeservings,verbs=*
 // +kubebuilder:rbac:groups="config.openshift.io",resources=ingresses,verbs=get
 
-// TODO: WB
+// WB
 // +kubebuilder:rbac:groups=components.platform.opendatahub.io,resources=workbenches,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=components.platform.opendatahub.io,resources=workbenches/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=components.platform.opendatahub.io,resources=workbenches/finalizers,verbs=update

get_all_manifests.sh

@@ -11,7 +11,7 @@ declare -A COMPONENT_MANIFESTS=(
     ["workbenches/odh-notebook-controller"]="red-hat-data-services:kubeflow:rhoai-2.17:components/odh-notebook-controller/config"
     ["workbenches/notebooks"]="red-hat-data-services:notebooks:rhoai-2.17:manifests"
     ["modelmeshserving"]="red-hat-data-services:modelmesh-serving:rhoai-2.17:config"
-    ["kserve"]="red-hat-data-services:kserve:rhoai-2.17:config"
+    ["kserve"]="red-hat-data-services:kserve:rhoai-2.16:config"
     ["kueue"]="red-hat-data-services:kueue:rhoai-2.17:config"
     ["codeflare"]="red-hat-data-services:codeflare-operator:rhoai-2.17:config"
     ["ray"]="red-hat-data-services:kuberay:rhoai-2.17:ray-operator/config"

pkg/cluster/cluster_config.go

@@ -32,9 +32,15 @@ type Release struct {
 	Version version.OperatorVersion `json:"version,omitempty"`
 }
 
+type ClusterInfo struct {
+	Type    string                  `json:"type,omitempty"` // openshift , TODO: can be other value if we later support other type
+	Version version.OperatorVersion `json:"version,omitempty"`
+}
+
 var clusterConfig struct {
-	Namespace string
-	Release   Release
+	Namespace   string
+	Release     Release
+	ClusterInfo ClusterInfo
 }
 
 // Init initializes cluster configuration variables on startup
@@ -54,15 +60,21 @@ func Init(ctx context.Context, cli client.Client) error {
 		return err
 	}
 
+	clusterConfig.ClusterInfo, err = getClusterInfo(ctx, cli)
+	if err != nil {
+		return err
+	}
+
 	printClusterConfig(log)
 
 	return nil
 }
 
 func printClusterConfig(log logr.Logger) {
 	log.Info("Cluster config",
-		"Namespace", clusterConfig.Namespace,
-		"Release", clusterConfig.Release)
+		"Operator Namespace", clusterConfig.Namespace,
+		"Release", clusterConfig.Release,
+		"Cluster", clusterConfig.ClusterInfo)
 }
 
 func GetOperatorNamespace() (string, error) {
@@ -76,6 +88,10 @@ func GetRelease() Release {
 	return clusterConfig.Release
 }
 
+func GetClusterInfo() ClusterInfo {
+	return clusterConfig.ClusterInfo
+}
+
 func GetDomain(ctx context.Context, c client.Client) (string, error) {
 	ingress := &unstructured.Unstructured{}
 	ingress.SetGroupVersionKind(gvk.OpenshiftIngress)
@@ -95,6 +111,21 @@ func GetDomain(ctx context.Context, c client.Client) (string, error) {
 	return domain, err
 }
 
+// This is an openshift speicifc implementation.
+func getOCPVersion(ctx context.Context, c client.Client) (version.OperatorVersion, error) {
+	clusterVersion := &configv1.ClusterVersion{}
+	if err := c.Get(ctx, client.ObjectKey{
+		Name: OpenShiftVersionObj,
+	}, clusterVersion); err != nil {
+		return version.OperatorVersion{}, errors.New("unable to get OCP version")
+	}
+	v, err := semver.ParseTolerant(clusterVersion.Status.History[0].Version)
+	if err != nil {
+		return version.OperatorVersion{}, errors.New("unable to parse OCP version")
+	}
+	return version.OperatorVersion{Version: v}, nil
+}
+
 func getOperatorNamespace() (string, error) {
 	operatorNS, exist := os.LookupEnv("OPERATOR_NAMESPACE")
 	if exist && operatorNS != "" {
@@ -199,6 +230,7 @@ func getRelease(ctx context.Context, cli client.Client) (Release, error) {
 			Version: semver.Version{},
 		},
 	}
+
 	// Set platform
 	platform, err := getPlatform(ctx, cli)
 	if err != nil {
@@ -230,6 +262,23 @@ func getRelease(ctx context.Context, cli client.Client) (Release, error) {
 	return initRelease, nil
 }
 
+func getClusterInfo(ctx context.Context, cli client.Client) (ClusterInfo, error) {
+	c := ClusterInfo{
+		Version: version.OperatorVersion{
+			Version: semver.Version{},
+		},
+		Type: "OpenShift",
+	}
+	// Set OCP
+	ocpVersion, err := getOCPVersion(ctx, cli)
+	if err != nil {
+		return c, err
+	}
+	c.Version = ocpVersion
+
+	return c, nil
+}
+
 // IsDefaultAuthMethod returns true if the default authentication method is IntegratedOAuth or empty.
 // This will give indication that Operator should create userGroups or not in the cluster.
 func IsDefaultAuthMethod(ctx context.Context, cli client.Client) (bool, error) {

pkg/cluster/const.go

@@ -15,4 +15,7 @@ const (
 
 	// Default cluster-scope Authentication CR name.
 	ClusterAuthenticationObj = "cluster"
+
+	// Default OpenShift version CR name.
+	OpenShiftVersionObj = "version"
 )

pkg/cluster/gvk/gvk.go

@@ -219,4 +219,16 @@ var (
 		Version: "v1alpha1",
 		Kind:    "Auth",
 	}
+
+	ValidatingAdmissionPolicy = schema.GroupVersionKind{
+		Group:   "admissionregistration.k8s.io",
+		Version: "v1",
+		Kind:    "ValidatingAdmissionPolicy",
+	}
+
+	ValidatingAdmissionPolicyBinding = schema.GroupVersionKind{
+		Group:   "admissionregistration.k8s.io",
+		Version: "v1",
+		Kind:    "ValidatingAdmissionPolicyBinding",
+	}
 )

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,10 +2,18 @@ package e2e_test
 
 import (
 	"testing"
+	"time"
 
+	"github.com/blang/semver/v4"
 	"github.com/stretchr/testify/require"
+	"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/gvk"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq"
+
+	. "github.com/onsi/gomega"
 )
 
 func kueueTestSuite(t *testing.T) {
@@ -21,9 +29,30 @@ func kueueTestSuite(t *testing.T) {
 	t.Run("Validate component enabled", componentCtx.ValidateComponentEnabled)
 	t.Run("Validate operands have OwnerReferences", componentCtx.ValidateOperandsOwnerReferences)
 	t.Run("Validate update operand resources", componentCtx.ValidateUpdateDeploymentsResources)
+	t.Run("Validate Kueue Dynamically create VAP and VAPB", componentCtx.validateKueueVAPReady)
 	t.Run("Validate component disabled", componentCtx.ValidateComponentDisabled)
 }
 
 type KueueTestCtx struct {
 	*ComponentTestCtx
 }
+
+func (tc *KueueTestCtx) validateKueueVAPReady(t *testing.T) {
+	g := tc.NewWithT(t)
+	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`),
+		)
+		return
+	}
+	_, 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{}))
+}