Skip to content

Commit

Permalink
components: move params.env image updating to Init stage (#1191)
Browse files Browse the repository at this point in the history 
* components, main: add Component Init method

Add Init() method to the Component interface and call it from main on
startup.

Will be used to move startup-time code from ReconcileComponent
(like adjusting params.env).

Signed-off-by: Yauheni Kaliuta <[email protected]>

* components: move params.env image updating to Init stage

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]>

---------

Signed-off-by: Yauheni Kaliuta <[email protected]>
  • Loading branch information
@ykaliuta
ykaliuta authored Oct 3, 2024
1 parent 8bebb1a commit 73823da
Show file tree
Hide file tree
Showing 13 changed files with 258 additions and 154 deletions.
27 changes: 18 additions & 9 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 @@ -64,10 +79,6 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context,
dscispec *dsciv1.DSCInitializationSpec,
platform cluster.Platform,
_ bool) error {
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 @@ -89,11 +100,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
5 changes: 5 additions & 0 deletions components/component.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ type Component struct {
DevFlags *DevFlags `json:"devFlags,omitempty"`
}

func (c *Component) Init(_ context.Context, _ cluster.Platform) error {
return nil
}

func (c *Component) GetManagementState() operatorv1.ManagementState {
return c.ManagementState
}
Expand Down Expand Up @@ -77,6 +81,7 @@ type ManifestsConfig struct {
}

type ComponentInterface interface {
Init(ctx context.Context, platform cluster.Platform) error
ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, platform cluster.Platform, currentComponentStatus bool) error
Cleanup(ctx context.Context, cli client.Client, owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec) error
Expand Down
35 changes: 24 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,26 @@ type Dashboard struct {
components.Component `json:""`
}

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

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 All @@ -68,16 +90,9 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
platform cluster.Platform,
currentComponentExist bool,
) error {
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 @@ -92,8 +107,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 @@ -114,7 +127,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 @@ -71,25 +101,6 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
platform cluster.Platform,
_ bool,
) error {
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 @@ -101,12 +112,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 @@ -96,11 +113,6 @@ func (k *Kserve) GetComponentName() string {

func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
l logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
// 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 @@ -140,12 +152,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
24 changes: 15 additions & 9 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 @@ -55,10 +70,6 @@ func (k *Kueue) GetComponentName() string {

func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
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
if enabled {
Expand All @@ -68,11 +79,6 @@ func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, l log
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 73823da

Please sign in to comment.