Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

components: move params.env image updating to Init stage #1191

Open
wants to merge 3 commits into
base: incubation
Choose a base branch
from
Open

components: move params.env image updating to Init stage #1191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8946f1d
components, main: add Component Init method
ykaliuta Sep 12, 2024
ee756db
main, init: wrap setupLog into Context to init components
ykaliuta Sep 12, 2024
15f9b9c
components: move params.env image updating to Init stage
ykaliuta Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions components/codeflare/codeflare.go
View file
Open in desktop
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also update docs on https://github.com/opendatahub-io/opendatahub-operator/blob/incubation/pkg/deploy/envParams.go#L67
just to be clear that it only update if it does not match otherwise it wont do anything

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean 450e19f ("deploy: ApplyParams: do not write params.env if there are no updates (#1186)") functionality?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, by reading the comments, i do not feel it reflects what the fuction is doing with recent chagnes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's anyway unrelated to this patchset, can come as fixup for the aforementioned patch.

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
5 changes: 5 additions & 0 deletions components/component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ type Component struct {
DevFlags *DevFlags `json:"devFlags,omitempty"`
}

func (c *Component) Init(_ context.Context, _ cluster.Platform) error {
return nil
}
Comment on lines +42 to +44
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why need this? you foresee some component might not have implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly put just in case. Can remove.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought was left from the first commit, when only one component implemented with Init(..) at that time.


func (c *Component) GetManagementState() operatorv1.ManagementState {
return c.ManagementState
}
Expand Down Expand Up @@ -78,6 +82,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
47 changes: 36 additions & 11 deletions components/dashboard/dashboard.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading