Skip to content

Commit

Permalink
update: move ocp version into main
Browse files Browse the repository at this point in the history
- now each component can get ocp version without calculate
- add test in kueue

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
  • Loading branch information
zdtsw committed Jan 9, 2025
1 parent c4a33c2 commit 4be326d
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,8 @@ spec:
properties:
name:
type: string
ocpversion:
type: string
version:
type: string
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ spec:
properties:
name:
type: string
ocpversion:
type: string
version:
type: string
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,8 @@ spec:
properties:
name:
type: string
ocpversion:
type: string
version:
type: string
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ spec:
properties:
name:
type: string
ocpversion:
type: string
version:
type: string
type: object
Expand Down
4 changes: 2 additions & 2 deletions controllers/components/kueue/kueue_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
// Owns(&admissionregistrationv1.ValidatingAdmissionPolicyBinding{}).
WatchesGVK(
gvk.ValidatingAdmissionPolicy,
reconciler.Dynamic(),
reconciler.Dynamic(vapPredicate),
).
WatchesGVK(
gvk.ValidatingAdmissionPolicyBinding,
reconciler.Dynamic(),
reconciler.Dynamic(vapPredicate),
).

Check warning on line 71 in controllers/components/kueue/kueue_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/kueue/kueue_controller.go#L60-L71

Added lines #L60 - L71 were not covered by tests
Owns(&appsv1.Deployment{}, reconciler.WithPredicates(resources.NewDeploymentPredicate())).
Watches(
Expand Down
14 changes: 2 additions & 12 deletions controllers/components/kueue/kueue_controller_actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,17 @@ import (
"context"
"fmt"

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"
odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
)

func initialize(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {

Check warning on line 12 in controllers/components/kueue/kueue_controller_actions.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/kueue/kueue_controller_actions.go#L12

Added line #L12 was not covered by tests
rr.Manifests = append(rr.Manifests, manifestsPath())
// Add OCP 4.17+ specific manifests if Minor > 16 {
ov, err := cluster.GetOCPVersion(ctx, rr.Client)
if err != nil {
return fmt.Errorf("failed to get OCP version: %w", err)
}
ctrl.Log.Info("OCP: ", "version", ov.String())
if ov.Minor > 16 { // it is safe to have to check only on minor, v3 has EOL last version is 11
ctrl.Log.Info("OCP version is over 4.16")
// Add OCP 4.17+ specific manifests if Minor > 16
if rr.Release.OCPVersion.Minor > 16 {
rr.Manifests = append(rr.Manifests, extramanifestsPath())
}

Check warning on line 17 in controllers/components/kueue/kueue_controller_actions.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/kueue/kueue_controller_actions.go#L14-L17

Added lines #L14 - L17 were not covered by tests

return nil
}

Expand Down
8 changes: 8 additions & 0 deletions controllers/components/kueue/kueue_support.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package kueue

import (
"context"

conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"

componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
)
Expand Down Expand Up @@ -41,3 +44,8 @@ func extramanifestsPath() odhtypes.ManifestInfo {
SourcePath: "rhoai/ocp-4.17-addons",
}

Check warning on line 45 in controllers/components/kueue/kueue_support.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/kueue/kueue_support.go#L40-L45

Added lines #L40 - L45 were not covered by tests
}

// Add OCP 4.17+ specific manifests if Minor > 16 .
func vapPredicate(context.Context, *odhtypes.ReconciliationRequest) bool {
return cluster.GetRelease().OCPVersion.Minor > 16

Check warning on line 50 in controllers/components/kueue/kueue_support.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/kueue/kueue_support.go#L49-L50

Added lines #L49 - L50 were not covered by tests
}
24 changes: 18 additions & 6 deletions pkg/cluster/cluster_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ type Platform string
// Release includes information on operator version and platform
// +kubebuilder:object:generate=true
type Release struct {
Name Platform `json:"name,omitempty"`
Version version.OperatorVersion `json:"version,omitempty"`
Name Platform `json:"name,omitempty"`
Version version.OperatorVersion `json:"version,omitempty"`
OCPVersion version.OperatorVersion `json:"ocpversion,omitempty"` // would like to just use semversion but it does not has deepcopy...
}

var clusterConfig struct {
Expand Down Expand Up @@ -95,14 +96,18 @@ func GetDomain(ctx context.Context, c client.Client) (string, error) {
return domain, err
}

func GetOCPVersion(ctx context.Context, c client.Client) (semver.Version, error) {
func getOCPVersion(ctx context.Context, c client.Client) (version.OperatorVersion, error) {
clusterVersion := &configv1.ClusterVersion{}
if err := c.Get(ctx, client.ObjectKey{
Name: "version",
Name: OpenShiftVersionObj,
}, clusterVersion); err != nil {
return semver.Version{}, errors.New("unable to get OCP version")
return version.OperatorVersion{}, errors.New("unable to get OCP version")
}
return semver.Make(clusterVersion.Status.History[0].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

Check warning on line 110 in pkg/cluster/cluster_config.go

View check run for this annotation

Codecov / codecov/patch

pkg/cluster/cluster_config.go#L99-L110

Added lines #L99 - L110 were not covered by tests
}

func getOperatorNamespace() (string, error) {
Expand Down Expand Up @@ -209,6 +214,13 @@ func getRelease(ctx context.Context, cli client.Client) (Release, error) {
Version: semver.Version{},
},
}
// Set OCP
ocpVersion, err := getOCPVersion(ctx, cli)
if err != nil {
return initRelease, err
}
initRelease.OCPVersion = ocpVersion

Check warning on line 223 in pkg/cluster/cluster_config.go

View check run for this annotation

Codecov / codecov/patch

pkg/cluster/cluster_config.go#L217-L223

Added lines #L217 - L223 were not covered by tests
// Set platform
platform, err := getPlatform(ctx, cli)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/cluster/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@ const (

// Default cluster-scope Authentication CR name.
ClusterAuthenticationObj = "cluster"

// Default OpenShift version CR name.
OpenShiftVersionObj = "version"
)
1 change: 1 addition & 0 deletions pkg/cluster/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 29 additions & 0 deletions tests/e2e/kueue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
corev1 "k8s.io/api/core/v1"
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/util/retry"
Expand Down Expand Up @@ -59,6 +60,11 @@ func kueueTestSuite(t *testing.T) {
require.NoError(t, err, "error getting all Kueue's Ownerrefrences")
})

t.Run("Validate Kueue Dynamically create VAP", func(t *testing.T) {
err = kueueCtx.validateVAPReady()
require.NoError(t, err, "Kueue instance is not Ready")
})

t.Run("Validate Kueue Ready", func(t *testing.T) {
err = kueueCtx.validateKueueReady()
require.NoError(t, err, "Kueue instance is not Ready")
Expand Down Expand Up @@ -164,6 +170,29 @@ func (tc *KueueTestCtx) testOwnerReferences() error {
return nil
}

func (tc *KueueTestCtx) validateVAPReady() error {
// get OCP version from DSC
keydsc := types.NamespacedName{Name: "default-dsc"}
dsc := &dscv1.DataScienceCluster{}
err := tc.testCtx.customClient.Get(tc.testCtx.ctx, keydsc, dsc)
if err != nil {
return fmt.Errorf("expect one DSC CR to be found but got error: %w", err)
}

// if ocp is 4.17+ then VAP should be created.
if dsc.Status.Release.OCPVersion.Minor > 16 {
keyvap := types.NamespacedName{Name: "kueue-validating-admission-policy"}
vap := &unstructured.Unstructured{}
vap.SetGroupVersionKind(gvk.ValidatingAdmissionPolicy)

err := tc.testCtx.customClient.Get(tc.testCtx.ctx, keyvap, vap)
if err != nil {
return fmt.Errorf("expect validatingadminssionpolicy to be found but got error: %w", err)
}
}
return nil
}

// Verify Kueue instance is in Ready phase when kueue deployments are up and running.
func (tc *KueueTestCtx) validateKueueReady() error {
err := wait.PollUntilContextTimeout(tc.testCtx.ctx, generalRetryInterval, componentReadyTimeout, true, func(ctx context.Context) (bool, error) {
Expand Down

0 comments on commit 4be326d

Please sign in to comment.