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.

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

chore: remove duplicated GetPlatform call in each component #1055

Merged
merged 2 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 11 additions & 8 deletions components/codeflare/codeflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,13 @@ func (c *CodeFlare) GetComponentName() string {
return ComponentName
}

func (c *CodeFlare) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
func (c *CodeFlare) ReconcileComponent(ctx context.Context,
cli client.Client,
logger logr.Logger,
owner metav1.Object,
dscispec *dsciv1.DSCInitializationSpec,
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
Expand All @@ -66,14 +72,11 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context, cli client.Client, l

enabled := c.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
platform, err := cluster.GetPlatform(cli)
if err != nil {
return err
}

if enabled {
if c.DevFlags != nil {
// Download manifests and update paths
if err = c.OverrideManifests(string(platform)); err != nil {
if err := c.OverrideManifests(string(platform)); err != nil {
return err
}
}
Expand Down Expand Up @@ -115,10 +118,10 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context, cli client.Client, l
}

// inject prometheus codeflare*.rules in to /opt/manifests/monitoring/prometheus/prometheus-configs.yaml
if err = c.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentName); err != nil {
if err := c.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentName); err != nil {
return err
}
if err = deploy.DeployManifestsFromPath(cli, owner,
if err := deploy.DeployManifestsFromPath(cli, owner,
filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
dscispec.Monitoring.Namespace,
"prometheus", true); err != nil {
Expand Down
3 changes: 2 additions & 1 deletion components/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
ctrlogger "github.com/opendatahub-io/opendatahub-operator/v2/pkg/logger"
)

Expand Down Expand Up @@ -79,7 +80,7 @@ type ManifestsConfig struct {

type ComponentInterface interface {
ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, currentComponentStatus bool) error
owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, platform cluster.Platform, currentComponentStatus bool) error
Cleanup(cli client.Client, DSCISpec *dsciv1.DSCInitializationSpec) error
GetComponentName() string
GetManagementState() operatorv1.ManagementState
Expand Down
12 changes: 5 additions & 7 deletions components/dashboard/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,11 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
logger logr.Logger,
owner metav1.Object,
dscispec *dsciv1.DSCInitializationSpec,
platform cluster.Platform,
currentComponentExist bool,
) error {
var l logr.Logger
platform, err := cluster.GetPlatform(cli)
if err != nil {
return err
}

if platform == cluster.SelfManagedRhods || platform == cluster.ManagedRhods {
l = d.ConfigComponentLogger(logger, ComponentNameSupported, dscispec)
} else {
Expand Down Expand Up @@ -183,7 +181,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
if err := d.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentNameSupported); err != nil {
return err
}
if err = deploy.DeployManifestsFromPath(cli, owner,
if err := deploy.DeployManifestsFromPath(cli, owner,
filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
dscispec.Monitoring.Namespace,
"prometheus", true); err != nil {
Expand All @@ -194,11 +192,11 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
return nil
default:
// base
if err = deploy.DeployManifestsFromPath(cli, owner, Path, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
if err := deploy.DeployManifestsFromPath(cli, owner, Path, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
return err
}
// ISV
if err = deploy.DeployManifestsFromPath(cli, owner, PathISV, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
if err := deploy.DeployManifestsFromPath(cli, owner, PathISV, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
return err
}
// consolelink
Expand Down
11 changes: 4 additions & 7 deletions components/datasciencepipelines/datasciencepipelines.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
logger logr.Logger,
owner metav1.Object,
dscispec *dsciv1.DSCInitializationSpec,
platform cluster.Platform,
_ bool,
) error {
l := d.ConfigComponentLogger(logger, ComponentName, dscispec)
Expand All @@ -90,14 +91,10 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
enabled := d.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

platform, err := cluster.GetPlatform(cli)
if err != nil {
return err
}
if enabled {
if d.DevFlags != nil {
// Download manifests and update paths
if err = d.OverrideManifests(string(platform)); err != nil {
if err := d.OverrideManifests(string(platform)); err != nil {
return err
}
}
Expand All @@ -119,7 +116,7 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
if platform == cluster.OpenDataHub || platform == "" {
manifestsPath = filepath.Join(OverlayPath, "odh")
}
if err = deploy.DeployManifestsFromPath(cli, owner, manifestsPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
if err := deploy.DeployManifestsFromPath(cli, owner, manifestsPath, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
return err
}
l.Info("apply manifests done")
Expand All @@ -138,7 +135,7 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
if err := d.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentName); err != nil {
return err
}
if err = deploy.DeployManifestsFromPath(cli, owner,
if err := deploy.DeployManifestsFromPath(cli, owner,
filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
dscispec.Monitoring.Namespace,
"prometheus", true); err != nil {
Expand Down
10 changes: 3 additions & 7 deletions components/kserve/kserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (k *Kserve) GetComponentName() string {
}

func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
logger logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := k.ConfigComponentLogger(logger, ComponentName, dscispec)
// paramMap for Kserve to use.
var imageParamMap = map[string]string{}
Expand All @@ -107,10 +107,6 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,

enabled := k.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
platform, err := cluster.GetPlatform(cli)
if err != nil {
return err
}

if !enabled {
if err := k.removeServerlessFeatures(dscispec); err != nil {
Expand All @@ -123,7 +119,7 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
}
if k.DevFlags != nil {
// Download manifests and update paths
if err = k.OverrideManifests(string(platform)); err != nil {
if err := k.OverrideManifests(string(platform)); err != nil {
return err
}
}
Expand All @@ -136,7 +132,7 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
}
}

if err = k.configureServiceMesh(cli, dscispec); err != nil {
if err := k.configureServiceMesh(cli, dscispec); err != nil {
return fmt.Errorf("failed configuring service mesh while reconciling kserve component. cause: %w", err)
}

Expand Down
11 changes: 3 additions & 8 deletions components/kueue/kueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,18 @@ func (k *Kueue) GetComponentName() string {
}

func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
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
platform, err := cluster.GetPlatform(cli)
if err != nil {
return err
}

if enabled {
if k.DevFlags != nil {
// Download manifests and update paths
if err = k.OverrideManifests(string(platform)); err != nil {
if err := k.OverrideManifests(string(platform)); err != nil {
return err
}
}
Expand All @@ -97,7 +92,7 @@ func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, logge
if err := k.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentName); err != nil {
return err
}
if err = deploy.DeployManifestsFromPath(cli, owner,
if err := deploy.DeployManifestsFromPath(cli, owner,
filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
dscispec.Monitoring.Namespace,
"prometheus", true); err != nil {
Expand Down
11 changes: 4 additions & 7 deletions components/modelmeshserving/modelmeshserving.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
logger logr.Logger,
owner metav1.Object,
dscispec *dsciv1.DSCInitializationSpec,
platform cluster.Platform,
_ bool,
) error {
l := m.ConfigComponentLogger(logger, ComponentName, dscispec)
Expand All @@ -94,16 +95,12 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,

enabled := m.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
platform, err := cluster.GetPlatform(cli)
if err != nil {
return err
}

// Update Default rolebinding
if enabled {
if m.DevFlags != nil {
// Download manifests and update paths
if err = m.OverrideManifests(string(platform)); err != nil {
if err := m.OverrideManifests(string(platform)); err != nil {
return err
}
}
Expand All @@ -123,7 +120,7 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
}
}

if err = deploy.DeployManifestsFromPath(cli, owner, Path, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
if err := deploy.DeployManifestsFromPath(cli, owner, Path, dscispec.ApplicationsNamespace, ComponentName, enabled); err != nil {
return fmt.Errorf("failed to apply manifests from %s : %w", Path, err)
}
l.WithValues("Path", Path).Info("apply manifests done for modelmesh")
Expand Down Expand Up @@ -165,7 +162,7 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
if err := m.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, DependentComponentName); err != nil {
return err
}
if err = deploy.DeployManifestsFromPath(cli, owner,
if err := deploy.DeployManifestsFromPath(cli, owner,
filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
dscispec.Monitoring.Namespace,
"prometheus", true); err != nil {
Expand Down
15 changes: 4 additions & 11 deletions components/modelregistry/modelregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (m *ModelRegistry) GetComponentName() string {
}

func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := m.ConfigComponentLogger(logger, ComponentName, dscispec)
var imageParamMap = map[string]string{
"IMAGES_MODELREGISTRY_OPERATOR": "RELATED_IMAGE_ODH_MODEL_REGISTRY_OPERATOR_IMAGE",
Expand All @@ -68,15 +68,10 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien
}
enabled := m.GetManagementState() == operatorv1.Managed

platform, err := cluster.GetPlatform(cli)
if err != nil {
return err
}

if enabled {
if m.DevFlags != nil {
// Download manifests and update paths
if err = m.OverrideManifests(string(platform)); err != nil {
if err := m.OverrideManifests(string(platform)); err != nil {
return err
}
}
Expand All @@ -96,15 +91,13 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien
}
}
// Deploy ModelRegistry Operator
err = deploy.DeployManifestsFromPath(cli, owner, Path, dscispec.ApplicationsNamespace, m.GetComponentName(), enabled)
if err != nil {
if err := deploy.DeployManifestsFromPath(cli, owner, Path, dscispec.ApplicationsNamespace, m.GetComponentName(), enabled); err != nil {
return err
}
l.Info("apply manifests done")

// Create additional model registry resources, componentEnabled=true because these extras are never deleted!
err = deploy.DeployManifestsFromPath(cli, owner, Path+"/extras", dscispec.ApplicationsNamespace, m.GetComponentName(), true)
if err != nil {
if err := deploy.DeployManifestsFromPath(cli, owner, Path+"/extras", dscispec.ApplicationsNamespace, m.GetComponentName(), true); err != nil {
return err
}
l.Info("apply extra manifests done")
Expand Down
10 changes: 3 additions & 7 deletions components/ray/ray.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (r *Ray) GetComponentName() string {
}

func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := r.ConfigComponentLogger(logger, ComponentName, dscispec)

var imageParamMap = map[string]string{
Expand All @@ -66,15 +66,11 @@ func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, logger

enabled := r.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
platform, err := cluster.GetPlatform(cli)
if err != nil {
return err
}

if enabled {
if r.DevFlags != nil {
// Download manifests and update paths
if err = r.OverrideManifests(string(platform)); err != nil {
if err := r.OverrideManifests(string(platform)); err != nil {
return err
}
}
Expand All @@ -101,7 +97,7 @@ func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, logger
if err := r.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentName); err != nil {
return err
}
if err = deploy.DeployManifestsFromPath(cli, owner,
if err := deploy.DeployManifestsFromPath(cli, owner,
filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
dscispec.Monitoring.Namespace,
"prometheus", true); err != nil {
Expand Down
10 changes: 3 additions & 7 deletions components/trainingoperator/trainingoperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (r *TrainingOperator) GetComponentName() string {
}

func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := r.ConfigComponentLogger(logger, ComponentName, dscispec)

var imageParamMap = map[string]string{
Expand All @@ -66,15 +66,11 @@ func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Cl

enabled := r.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
platform, err := cluster.GetPlatform(cli)
if err != nil {
return err
}

if enabled {
if r.DevFlags != nil {
// Download manifests and update paths
if err = r.OverrideManifests(string(platform)); err != nil {
if err := r.OverrideManifests(string(platform)); err != nil {
return err
}
}
Expand Down Expand Up @@ -102,7 +98,7 @@ func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Cl
if err := r.UpdatePrometheusConfig(cli, enabled && monitoringEnabled, ComponentName); err != nil {
return err
}
if err = deploy.DeployManifestsFromPath(cli, owner,
if err := deploy.DeployManifestsFromPath(cli, owner,
filepath.Join(deploy.DefaultManifestPath, "monitoring", "prometheus", "apps"),
dscispec.Monitoring.Namespace,
"prometheus", true); err != nil {
Expand Down
Loading
Loading