Skip to content

Commit

Permalink
components: move params.env image updating to Init stage
Browse files Browse the repository at this point in the history 
Jira: https://issues.redhat.com/browse/RHOAIENG-11592

Image names in environment are not supposed to be changed during
runtime of the operator, so it makes sense to update them only on
startup.

If manifests are overriden by DevFlags, the DevFlags' version will
be used.

The change is straight forward for most of the components where only
images are updated and params.env is located in the kustomize root
directory, but some components (dashboard, ray, codeflare,
modelregistry) also update some extra parameters. For them image
part only is moved to Init since other updates require runtime DSCI
information.

The patch also changes logic for ray, codeflare, and modelregistry
in this regard to update non-image parameters regardless of DevFlags
like it was changed in dashboard recently.

The DevFlags functionality brings some concerns:

- For most components the code is written such a way that as soon as
DevFlags supplied the global path variables are changed and never
reverted back to the defaults. For some (dashboard, trustyai) there
is (still global) OverridePath/entryPath pair and manifests reverted
to the default, BUT there is no logic of transition.

- codeflare: when manifests are overridden namespace param is
updated in the hardcoded (stock) path;

This logic is preserved.

Signed-off-by: Yauheni Kaliuta <[email protected]>
  • Loading branch information
@ykaliuta
ykaliuta committed Sep 17, 2024
1 parent 12b61a1 commit 199dc92
Show file tree
Hide file tree
Showing 11 changed files with 244 additions and 151 deletions.
26 changes: 18 additions & 8 deletions components/codeflare/codeflare.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/components"
Expand All @@ -35,6 +36,20 @@ type CodeFlare struct {
components.Component `json:""`
}

func (c *CodeFlare) Init(ctx context.Context, _ cluster.Platform) error {
log := logf.FromContext(ctx).WithName(ComponentName)

var imageParamMap = map[string]string{
"codeflare-operator-controller-image": "RELATED_IMAGE_ODH_CODEFLARE_OPERATOR_IMAGE", // no need mcad, embedded in cfo
}

if err := deploy.ApplyParams(ParamsPath, imageParamMap); err != nil {
log.Error(err, "failed to update image", "path", CodeflarePath+"/bases")
}

return nil
}

func (c *CodeFlare) OverrideManifests(ctx context.Context, _ cluster.Platform) error {
// If devflags are set, update default manifests path
if len(c.DevFlags.Manifests) != 0 {
Expand Down Expand Up @@ -65,9 +80,6 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context,
platform cluster.Platform,
_ bool) error {
l := c.ConfigComponentLogger(logger, ComponentName, dscispec)
var imageParamMap = map[string]string{
"codeflare-operator-controller-image": "RELATED_IMAGE_ODH_CODEFLARE_OPERATOR_IMAGE", // no need mcad, embedded in cfo
}

enabled := c.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
Expand All @@ -90,11 +102,9 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context,
dependentOperator, ComponentName)
}

// Update image parameters only when we do not have customized manifests set
if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (c.DevFlags == nil || len(c.DevFlags.Manifests) == 0) {
if err := deploy.ApplyParams(ParamsPath, imageParamMap, map[string]string{"namespace": dscispec.ApplicationsNamespace}); err != nil {
return fmt.Errorf("failed update image from %s : %w", CodeflarePath+"/bases", err)
}
// It updates stock manifests, overridden manifests should contain proper namespace
if err := deploy.ApplyParams(ParamsPath, nil, map[string]string{"namespace": dscispec.ApplicationsNamespace}); err != nil {
return fmt.Errorf("failed update image from %s : %w", CodeflarePath+"/bases", err)
}
}

Expand Down
47 changes: 36 additions & 11 deletions components/dashboard/dashboard.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/components"
Expand All @@ -31,6 +32,7 @@ var (
PathSelfDownstream = PathDownstream + "/onprem"
PathManagedDownstream = PathDownstream + "/addon"
OverridePath = ""
DefaultPath = ""
)

// Verifies that Dashboard implements ComponentInterface.
Expand All @@ -42,6 +44,38 @@ type Dashboard struct {
components.Component `json:""`
}

func componentName(platform cluster.Platform) string {
var name string
switch platform {
case cluster.SelfManagedRhods, cluster.ManagedRhods:
name = ComponentNameDownstream
default:
name = ComponentNameUpstream
}

return name
}

func (d *Dashboard) Init(ctx context.Context, platform cluster.Platform) error {
log := logf.FromContext(ctx).WithName(componentName(platform))

imageParamMap := map[string]string{
"odh-dashboard-image": "RELATED_IMAGE_ODH_DASHBOARD_IMAGE",
}
DefaultPath := map[cluster.Platform]string{
cluster.SelfManagedRhods: PathDownstream + "/onprem",
cluster.ManagedRhods: PathDownstream + "/addon",
cluster.OpenDataHub: PathUpstream,
cluster.Unknown: PathUpstream,
}[platform]

if err := deploy.ApplyParams(DefaultPath, imageParamMap); err != nil {
log.Error(err, "failed to update image", "path", DefaultPath)
}

return nil
}

func (d *Dashboard) OverrideManifests(ctx context.Context, platform cluster.Platform) error {
// If devflags are set, update default manifests path
if len(d.DevFlags.Manifests) != 0 {
Expand Down Expand Up @@ -76,16 +110,9 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
l = d.ConfigComponentLogger(logger, ComponentNameUpstream, dscispec)
}

entryPath := map[cluster.Platform]string{
cluster.SelfManagedRhods: PathDownstream + "/onprem",
cluster.ManagedRhods: PathDownstream + "/addon",
cluster.OpenDataHub: PathUpstream,
cluster.Unknown: PathUpstream,
}[platform]

entryPath := DefaultPath
enabled := d.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
imageParamMap := make(map[string]string)

if enabled {
// 1. cleanup OAuth client related secret and CR if dashboard is in 'installed false' status
Expand All @@ -100,8 +127,6 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
if OverridePath != "" {
entryPath = OverridePath
}
} else { // Update image parameters if devFlags is not provided
imageParamMap["odh-dashboard-image"] = "RELATED_IMAGE_ODH_DASHBOARD_IMAGE"
}

// 2. platform specific RBAC
Expand All @@ -122,7 +147,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
}

// 4. update params.env regardless devFlags is provided of not
if err := deploy.ApplyParams(entryPath, imageParamMap, extraParamsMap); err != nil {
if err := deploy.ApplyParams(entryPath, nil, extraParamsMap); err != nil {
return fmt.Errorf("failed to update params.env from %s : %w", entryPath, err)
}
}
Expand Down
55 changes: 30 additions & 25 deletions components/datasciencepipelines/datasciencepipelines.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/components"
Expand All @@ -41,6 +42,35 @@ type DataSciencePipelines struct {
components.Component `json:""`
}

func (d *DataSciencePipelines) Init(ctx context.Context, _ cluster.Platform) error {
log := logf.FromContext(ctx).WithName(ComponentName)

var imageParamMap = map[string]string{
// v1
"IMAGES_APISERVER": "RELATED_IMAGE_ODH_ML_PIPELINES_API_SERVER_IMAGE",
"IMAGES_ARTIFACT": "RELATED_IMAGE_ODH_ML_PIPELINES_ARTIFACT_MANAGER_IMAGE",
"IMAGES_PERSISTENTAGENT": "RELATED_IMAGE_ODH_ML_PIPELINES_PERSISTENCEAGENT_IMAGE",
"IMAGES_SCHEDULEDWORKFLOW": "RELATED_IMAGE_ODH_ML_PIPELINES_SCHEDULEDWORKFLOW_IMAGE",
"IMAGES_CACHE": "RELATED_IMAGE_ODH_ML_PIPELINES_CACHE_IMAGE",
"IMAGES_DSPO": "RELATED_IMAGE_ODH_DATA_SCIENCE_PIPELINES_OPERATOR_CONTROLLER_IMAGE",
// v2
"IMAGESV2_ARGO_APISERVER": "RELATED_IMAGE_ODH_ML_PIPELINES_API_SERVER_V2_IMAGE",
"IMAGESV2_ARGO_PERSISTENCEAGENT": "RELATED_IMAGE_ODH_ML_PIPELINES_PERSISTENCEAGENT_V2_IMAGE",
"IMAGESV2_ARGO_SCHEDULEDWORKFLOW": "RELATED_IMAGE_ODH_ML_PIPELINES_SCHEDULEDWORKFLOW_V2_IMAGE",
"IMAGESV2_ARGO_ARGOEXEC": "RELATED_IMAGE_ODH_DATA_SCIENCE_PIPELINES_ARGO_ARGOEXEC_IMAGE",
"IMAGESV2_ARGO_WORKFLOWCONTROLLER": "RELATED_IMAGE_ODH_DATA_SCIENCE_PIPELINES_ARGO_WORKFLOWCONTROLLER_IMAGE",
"V2_DRIVER_IMAGE": "RELATED_IMAGE_ODH_ML_PIPELINES_DRIVER_IMAGE",
"V2_LAUNCHER_IMAGE": "RELATED_IMAGE_ODH_ML_PIPELINES_LAUNCHER_IMAGE",
"IMAGESV2_ARGO_MLMDGRPC": "RELATED_IMAGE_ODH_MLMD_GRPC_SERVER_IMAGE",
}

if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
log.Error(err, "failed to update image", "path", Path)
}

return nil
}

func (d *DataSciencePipelines) OverrideManifests(ctx context.Context, _ cluster.Platform) error {
// If devflags are set, update default manifests path
if len(d.DevFlags.Manifests) != 0 {
Expand Down Expand Up @@ -72,25 +102,6 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
_ bool,
) error {
l := d.ConfigComponentLogger(logger, ComponentName, dscispec)
var imageParamMap = map[string]string{
// v1
"IMAGES_APISERVER": "RELATED_IMAGE_ODH_ML_PIPELINES_API_SERVER_IMAGE",
"IMAGES_ARTIFACT": "RELATED_IMAGE_ODH_ML_PIPELINES_ARTIFACT_MANAGER_IMAGE",
"IMAGES_PERSISTENTAGENT": "RELATED_IMAGE_ODH_ML_PIPELINES_PERSISTENCEAGENT_IMAGE",
"IMAGES_SCHEDULEDWORKFLOW": "RELATED_IMAGE_ODH_ML_PIPELINES_SCHEDULEDWORKFLOW_IMAGE",
"IMAGES_CACHE": "RELATED_IMAGE_ODH_ML_PIPELINES_CACHE_IMAGE",
"IMAGES_DSPO": "RELATED_IMAGE_ODH_DATA_SCIENCE_PIPELINES_OPERATOR_CONTROLLER_IMAGE",
// v2
"IMAGESV2_ARGO_APISERVER": "RELATED_IMAGE_ODH_ML_PIPELINES_API_SERVER_V2_IMAGE",
"IMAGESV2_ARGO_PERSISTENCEAGENT": "RELATED_IMAGE_ODH_ML_PIPELINES_PERSISTENCEAGENT_V2_IMAGE",
"IMAGESV2_ARGO_SCHEDULEDWORKFLOW": "RELATED_IMAGE_ODH_ML_PIPELINES_SCHEDULEDWORKFLOW_V2_IMAGE",
"IMAGESV2_ARGO_ARGOEXEC": "RELATED_IMAGE_ODH_DATA_SCIENCE_PIPELINES_ARGO_ARGOEXEC_IMAGE",
"IMAGESV2_ARGO_WORKFLOWCONTROLLER": "RELATED_IMAGE_ODH_DATA_SCIENCE_PIPELINES_ARGO_WORKFLOWCONTROLLER_IMAGE",
"V2_DRIVER_IMAGE": "RELATED_IMAGE_ODH_ML_PIPELINES_DRIVER_IMAGE",
"V2_LAUNCHER_IMAGE": "RELATED_IMAGE_ODH_ML_PIPELINES_LAUNCHER_IMAGE",
"IMAGESV2_ARGO_MLMDGRPC": "RELATED_IMAGE_ODH_MLMD_GRPC_SERVER_IMAGE",
}

enabled := d.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand All @@ -102,12 +113,6 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
}
}
// skip check if the dependent operator has beeninstalled, this is done in dashboard
// Update image parameters only when we do not have customized manifests set
if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (d.DevFlags == nil || len(d.DevFlags.Manifests) == 0) {
if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
return fmt.Errorf("failed to update image from %s : %w", Path, err)
}
}
// Check for existing Argo Workflows
if err := UnmanagedArgoWorkFlowExists(ctx, cli); err != nil {
return err
Expand Down
28 changes: 17 additions & 11 deletions components/kserve/kserve.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1"
Expand Down Expand Up @@ -56,6 +57,22 @@ type Kserve struct {
DefaultDeploymentMode DefaultDeploymentMode `json:"defaultDeploymentMode,omitempty"`
}

func (k *Kserve) Init(ctx context.Context, _ cluster.Platform) error {
log := logf.FromContext(ctx).WithName(ComponentName)

// dependentParamMap for odh-model-controller to use.
var dependentParamMap = map[string]string{
"odh-model-controller": "RELATED_IMAGE_ODH_MODEL_CONTROLLER_IMAGE",
}

// Update image parameters for odh-model-controller
if err := deploy.ApplyParams(DependentPath, dependentParamMap); err != nil {
log.Error(err, "failed to update image", "path", DependentPath)
}

return nil
}

func (k *Kserve) OverrideManifests(ctx context.Context, _ cluster.Platform) error {
// Download manifests if defined by devflags
// Go through each manifest and set the overlays if defined
Expand Down Expand Up @@ -98,11 +115,6 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := k.ConfigComponentLogger(logger, ComponentName, dscispec)

// dependentParamMap for odh-model-controller to use.
var dependentParamMap = map[string]string{
"odh-model-controller": "RELATED_IMAGE_ODH_MODEL_CONTROLLER_IMAGE",
}

enabled := k.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand Down Expand Up @@ -142,12 +154,6 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
if err := cluster.UpdatePodSecurityRolebinding(ctx, cli, dscispec.ApplicationsNamespace, "odh-model-controller"); err != nil {
return err
}
// Update image parameters for odh-model-controller
if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (k.DevFlags == nil || len(k.DevFlags.Manifests) == 0) {
if err := deploy.ApplyParams(DependentPath, dependentParamMap); err != nil {
return fmt.Errorf("failed to update image %s: %w", DependentPath, err)
}
}
}

if err := deploy.DeployManifestsFromPath(ctx, cli, owner, DependentPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
Expand Down
23 changes: 15 additions & 8 deletions components/kueue/kueue.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/components"
Expand All @@ -31,6 +32,20 @@ type Kueue struct {
components.Component `json:""`
}

func (k *Kueue) Init(ctx context.Context, _ cluster.Platform) error {
log := logf.FromContext(ctx).WithName(ComponentName)

var imageParamMap = map[string]string{
"odh-kueue-controller-image": "RELATED_IMAGE_ODH_KUEUE_CONTROLLER_IMAGE", // new kueue image
}

if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
log.Error(err, "failed to update image", "path", Path)
}

return nil
}

func (k *Kueue) OverrideManifests(ctx context.Context, _ cluster.Platform) error {
// If devflags are set, update default manifests path
if len(k.DevFlags.Manifests) != 0 {
Expand All @@ -56,9 +71,6 @@ func (k *Kueue) GetComponentName() string {
func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := k.ConfigComponentLogger(logger, ComponentName, dscispec)
var imageParamMap = map[string]string{
"odh-kueue-controller-image": "RELATED_IMAGE_ODH_KUEUE_CONTROLLER_IMAGE", // new kueue image
}

enabled := k.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
Expand All @@ -69,11 +81,6 @@ func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, logge
return err
}
}
if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (k.DevFlags == nil || len(k.DevFlags.Manifests) == 0) {
if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
return fmt.Errorf("failed to update image from %s : %w", Path, err)
}
}
}
// Deploy Kueue Operator
if err := deploy.DeployManifestsFromPath(ctx, cli, owner, Path, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
Expand Down
Loading

0 comments on commit 199dc92

Please sign in to comment.