diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index e249fbe3ad5..fa61b6093e7 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -293,6 +293,8 @@ spec: - admissionregistration.k8s.io resources: - mutatingwebhookconfigurations + - validatingadmissionpolicies + - validatingadmissionpolicybindings - validatingwebhookconfigurations verbs: - create diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index de7385f6a9d..06ab6910f9e 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -28,6 +28,8 @@ rules: - admissionregistration.k8s.io resources: - mutatingwebhookconfigurations + - validatingadmissionpolicies + - validatingadmissionpolicybindings - validatingwebhookconfigurations verbs: - create diff --git a/controllers/components/kueue/kueue_controller.go b/controllers/components/kueue/kueue_controller.go index f8cbe32a105..37da9104d0f 100644 --- a/controllers/components/kueue/kueue_controller.go +++ b/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,7 +44,7 @@ import ( ) func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error { - _, err := reconciler.ReconcilerFor(mgr, &componentApi.Kueue{}). + b := reconciler.ReconcilerFor(mgr, &componentApi.Kueue{}). // customized Owns() for Component with new predicates Owns(&corev1.ConfigMap{}). Owns(&corev1.Secret{}). @@ -72,15 +75,21 @@ 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 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 } diff --git a/controllers/components/kueue/kueue_controller_actions.go b/controllers/components/kueue/kueue_controller_actions.go index 35ed0e1562b..d8324f5ed6c 100644 --- a/controllers/components/kueue/kueue_controller_actions.go +++ b/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 +} diff --git a/controllers/components/kueue/kueue_support.go b/controllers/components/kueue/kueue_support.go index 40169e4bd25..4632b0ca8ad 100644 --- a/controllers/components/kueue/kueue_support.go +++ b/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", + } +} diff --git a/controllers/datasciencecluster/kubebuilder_rbac.go b/controllers/datasciencecluster/kubebuilder_rbac.go index dcf03dbf194..86f4956a1d2 100644 --- a/controllers/datasciencecluster/kubebuilder_rbac.go +++ b/controllers/datasciencecluster/kubebuilder_rbac.go @@ -156,6 +156,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 @@ -191,7 +193,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 diff --git a/pkg/cluster/cluster_config.go b/pkg/cluster/cluster_config.go index 08a1c445bfe..2afbbdf1a63 100644 --- a/pkg/cluster/cluster_config.go +++ b/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,6 +60,11 @@ 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 @@ -61,8 +72,9 @@ func Init(ctx context.Context, cli client.Client) error { 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) { diff --git a/pkg/cluster/const.go b/pkg/cluster/const.go index 6a6562bff07..af8242ded2d 100644 --- a/pkg/cluster/const.go +++ b/pkg/cluster/const.go @@ -15,4 +15,7 @@ const ( // Default cluster-scope Authentication CR name. ClusterAuthenticationObj = "cluster" + + // Default OpenShift version CR name. + OpenShiftVersionObj = "version" ) diff --git a/pkg/cluster/gvk/gvk.go b/pkg/cluster/gvk/gvk.go index 06a7f199699..0c3fd589682 100644 --- a/pkg/cluster/gvk/gvk.go +++ b/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", + } ) diff --git a/tests/e2e/kueue_test.go b/tests/e2e/kueue_test.go index 4f3299f184c..2b7a19328cc 100644 --- a/tests/e2e/kueue_test.go +++ b/tests/e2e/kueue_test.go @@ -3,9 +3,19 @@ package e2e_test import ( "testing" + "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/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" ) func kueueTestSuite(t *testing.T) { @@ -20,6 +30,7 @@ func kueueTestSuite(t *testing.T) { t.Run("Validate component enabled", componentCtx.ValidateComponentEnabled) t.Run("Validate operands have OwnerReferences", componentCtx.ValidateOperandsOwnerReferences) + t.Run("Validate Kueue Dynamically create VAP", componentCtx.validateKueueVAPReady) t.Run("Validate update operand resources", componentCtx.ValidateUpdateDeploymentsResources) t.Run("Validate component disabled", componentCtx.ValidateComponentDisabled) } @@ -27,3 +38,28 @@ func kueueTestSuite(t *testing.T) { type KueueTestCtx struct { *ComponentTestCtx } + +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), + ) + 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")) +}