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

Recreate stale OauthClient resource #716

Merged
merged 2 commits into from
Nov 14, 2023
Merged

Recreate stale OauthClient resource #716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d13388
Patch stale OauthClient resource
VaishnaviHire Nov 9, 2023
010042f
Fix DSCI Patch
VaishnaviHire Nov 12, 2023
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
2 changes: 1 addition & 1 deletion components/codeflare/codeflare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (c *CodeFlare) GetComponentName() string {
// Verifies that CodeFlare implements ComponentInterface.
var _ components.ComponentInterface = (*CodeFlare)(nil)

func (c *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec) error {
func (c *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
var imageParamMap = map[string]string{
"odh-codeflare-operator-controller-image": "RELATED_IMAGE_ODH_CODEFLARE_OPERATOR_IMAGE", // no need mcad, embedded in cfo
"namespace": dscispec.ApplicationsNamespace,
Expand Down
2 changes: 1 addition & 1 deletion components/component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ type ManifestsConfig struct {
}

type ComponentInterface interface {
ReconcileComponent(cli client.Client, owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec) error
ReconcileComponent(cli client.Client, owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, currentComponentStatus bool) error
Cleanup(cli client.Client, DSCISpec *dsciv1.DSCInitializationSpec) error
GetComponentName() string
GetManagementState() operatorv1.ManagementState
Expand Down
33 changes: 32 additions & 1 deletion components/dashboard/dashboard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (

operatorv1 "github.com/openshift/api/operator/v1"
routev1 "github.com/openshift/api/route/v1"
v1 "k8s.io/api/core/v1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand Down Expand Up @@ -73,7 +75,8 @@ func (d *Dashboard) GetComponentName() string {
// Verifies that Dashboard implements ComponentInterface.
var _ components.ComponentInterface = (*Dashboard)(nil)

func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec) error {
//nolint:gocyclo
Copy link
Contributor

Choose a reason for hiding this comment

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

@VaishnaviHire @zdtsw as I am merging 605 with incubation I came across this change and I am wondering what was the rationale behind disabling gocyclo linter in this particular case.

These tools play a crucial role in ensuring our code remains straightforward and accessible, especially for those who might be revisiting it after some time or are new to the codebase.

My concern is that bypassing these checks could lead to potential challenges in maintaining the project in the long run. I'm quite curious to understand the reasoning behind explicitly disabling this check. Was there a specific obstacle that prevented simplifying the code to comply with the linter? Maybe we can figure out together how to make this code simpler and benefit from linter complaints instead of keeping them silent? :)

func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, currentComponentStatus bool) error {
var imageParamMap = map[string]string{
"odh-dashboard-image": "RELATED_IMAGE_ODH_DASHBOARD_IMAGE",
}
Expand All @@ -87,6 +90,9 @@ func (d *Dashboard) ReconcileComponent(cli client.Client, owner metav1.Object, d

// Update Default rolebinding
if enabled {
if err := d.cleanOauthClientSecrets(cli, dscispec, currentComponentStatus); err != nil {
return err
}
// Download manifests and update paths
if err := d.OverrideManifests(string(platform)); err != nil {
return err
Expand Down Expand Up @@ -259,3 +265,28 @@ func (d *Dashboard) deployConsoleLink(cli client.Client, owner metav1.Object, na

return nil
}

func (d *Dashboard) cleanOauthClientSecrets(cli client.Client, dscispec *dsciv1.DSCInitializationSpec, currentComponentStatus bool) error {
// Remove previous oauth-client secrets
// Check if component is going from state of `Not Installed --> Installed`
// Assumption: Component is currently set to enabled
if !currentComponentStatus {
// Delete client secrets from previous installation
oauthClientSecret := &v1.Secret{}
err := cli.Get(context.TODO(), client.ObjectKey{
Namespace: dscispec.ApplicationsNamespace,
Name: "dashboard-oauth-client",
}, oauthClientSecret)
if err != nil {
if !apierrs.IsNotFound(err) {
return err
}
} else {
err := cli.Delete(context.TODO(), oauthClientSecret)
if err != nil {
return fmt.Errorf("error deleting oauth client secret: %v", err)
}
}
}
return nil
}
2 changes: 1 addition & 1 deletion components/datasciencepipelines/datasciencepipelines.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (d *DataSciencePipelines) GetComponentName() string {
// Verifies that Dashboard implements ComponentInterface.
var _ components.ComponentInterface = (*DataSciencePipelines)(nil)

func (d *DataSciencePipelines) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec) error {
func (d *DataSciencePipelines) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
var imageParamMap = map[string]string{
"IMAGES_APISERVER": "RELATED_IMAGE_ODH_ML_PIPELINES_API_SERVER_IMAGE",
"IMAGES_ARTIFACT": "RELATED_IMAGE_ODH_ML_PIPELINES_ARTIFACT_MANAGER_IMAGE",
Expand Down
2 changes: 1 addition & 1 deletion components/kserve/kserve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (k *Kserve) GetComponentName() string {
// Verifies that Kserve implements ComponentInterface.
var _ components.ComponentInterface = (*Kserve)(nil)

func (k *Kserve) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec) error {
func (k *Kserve) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
// paramMap for Kserve to use.
var imageParamMap = map[string]string{}

Expand Down
12 changes: 1 addition & 11 deletions components/modelmeshserving/modelmeshserving.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
package modelmeshserving

import (
"context"
"path/filepath"
"strings"

Expand Down Expand Up @@ -67,7 +66,7 @@ func (m *ModelMeshServing) GetComponentName() string {
// Verifies that Dashboard implements ComponentInterface.
var _ components.ComponentInterface = (*ModelMeshServing)(nil)

func (m *ModelMeshServing) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec) error {
func (m *ModelMeshServing) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
var imageParamMap = map[string]string{
"odh-mm-rest-proxy": "RELATED_IMAGE_ODH_MM_REST_PROXY_IMAGE",
"odh-modelmesh-runtime-adapter": "RELATED_IMAGE_ODH_MODELMESH_RUNTIME_ADAPTER_IMAGE",
Expand Down Expand Up @@ -136,15 +135,6 @@ func (m *ModelMeshServing) ReconcileComponent(cli client.Client, owner metav1.Ob
}
}

// Get monitoring namespace
dscInit := &dsciv1.DSCInitialization{}
err = cli.Get(context.TODO(), client.ObjectKey{
Name: "default",
}, dscInit)
if err != nil {
return err
}

// CloudService Monitoring handling
if platform == deploy.ManagedRhods {
// first model-mesh rules
Expand Down
2 changes: 1 addition & 1 deletion components/ray/ray.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (r *Ray) GetComponentName() string {
// Verifies that Ray implements ComponentInterface.
var _ components.ComponentInterface = (*Ray)(nil)

func (r *Ray) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec) error {
func (r *Ray) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
var imageParamMap = map[string]string{
"odh-kuberay-operator-controller-image": "RELATED_IMAGE_ODH_KUBERAY_OPERATOR_CONTROLLER_IMAGE",
"namespace": dscispec.ApplicationsNamespace,
Expand Down
2 changes: 1 addition & 1 deletion components/trustyai/trustyai.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (t *TrustyAI) GetComponentName() string {
// Verifies that TrustyAI implements ComponentInterface.
var _ components.ComponentInterface = (*TrustyAI)(nil)

func (t *TrustyAI) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec) error {
func (t *TrustyAI) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
var imageParamMap = map[string]string{
"trustyaiServiceImage": "RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_IMAGE",
"trustyaiOperatorImage": "RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_OPERATOR_IMAGE",
Expand Down
2 changes: 1 addition & 1 deletion components/workbenches/workbenches.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (w *Workbenches) GetComponentName() string {
// Verifies that Workbench implements ComponentInterface.
var _ components.ComponentInterface = (*Workbenches)(nil)

func (w *Workbenches) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsci.DSCInitializationSpec) error {
func (w *Workbenches) ReconcileComponent(cli client.Client, owner metav1.Object, dscispec *dsci.DSCInitializationSpec, _ bool) error {
var imageParamMap = map[string]string{
"odh-notebook-controller-image": "RELATED_IMAGE_ODH_NOTEBOOK_CONTROLLER_IMAGE",
"odh-kf-notebook-controller-image": "RELATED_IMAGE_ODH_KF_NOTEBOOK_CONTROLLER_IMAGE",
Expand Down
2 changes: 1 addition & 1 deletion controllers/datasciencecluster/datasciencecluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context
}

// Reconcile component
err = component.ReconcileComponent(r.Client, instance, r.DataScienceCluster.DSCISpec)
err = component.ReconcileComponent(r.Client, instance, r.DataScienceCluster.DSCISpec, instance.Status.InstalledComponents[componentName])

if err != nil {
// reconciliation failed: log errors, raise event and update status accordingly
Expand Down
13 changes: 11 additions & 2 deletions controllers/secretgenerator/secretgenerator_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package secretgenerator

import (
"context"
"encoding/json"
"fmt"
"time"

Expand Down Expand Up @@ -227,8 +228,16 @@ func (r *SecretGeneratorReconciler) createOAuthClient(ctx context.Context, name
err := r.Client.Create(ctx, oauthClient)
if err != nil {
if apierrs.IsAlreadyExists(err) {
secGenLog.Info("OAuth client resource already exists", "name", oauthClient.Name)

secGenLog.Info("OAuth client resource already exists, patch it", "name", oauthClient.Name)
data, err := json.Marshal(oauthClient)
if err != nil {
return fmt.Errorf("failed to get DataScienceCluster custom resource data: %v", err)
}
err = r.Client.Patch(context.TODO(), oauthClient, client.RawPatch(types.ApplyPatchType, data),
client.ForceOwnership, client.FieldOwner("opendatahub-operator"))
if err != nil {
return err
}
return nil
}
}
Expand Down
2 changes: 0 additions & 2 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,12 @@ func main() { //nolint:funlen
if !disableDSCConfig {
if err = upgrade.CreateDefaultDSCI(setupClient, platform, dscApplicationsNamespace, dscMonitoringNamespace); err != nil {
setupLog.Error(err, "unable to create initial setup for the operator")
os.Exit(1)
}
}

// Apply update from legacy operator
if err = upgrade.UpdateFromLegacyVersion(setupClient, platform); err != nil {
setupLog.Error(err, "unable to update from legacy operator version")
os.Exit(1)
}

if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
Expand Down
16 changes: 13 additions & 3 deletions pkg/upgrade/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,21 +220,31 @@ func CreateDefaultDSCI(cli client.Client, platform deploy.Platform, appNamespace
Spec: *defaultDsciSpec,
}

patchedDSCI := &dsci.DSCInitialization{
TypeMeta: metav1.TypeMeta{
Kind: "DSCInitialization",
APIVersion: "dscinitialization.opendatahub.io/v1",
},
Spec: *defaultDsciSpec,
}

instances := &dsci.DSCInitializationList{}
if err := cli.List(context.TODO(), instances); err != nil {
return err
}

switch {
case len(instances.Items) > 1:
return fmt.Errorf("only one instance of DSCInitialization object is allowed. Please delete other instances ")
fmt.Printf("only one instance of DSCInitialization object is allowed. Please delete other instances ")
return nil
case len(instances.Items) == 1:
if platform == deploy.ManagedRhods || platform == deploy.SelfManagedRhods {
data, err := json.Marshal(defaultDsciSpec)
data, err := json.Marshal(patchedDSCI)
if err != nil {
return err
}
err = cli.Patch(context.TODO(), defaultDsci, client.RawPatch(types.ApplyPatchType, data),
existingDSCI := &instances.Items[0]
err = cli.Patch(context.TODO(), existingDSCI, client.RawPatch(types.ApplyPatchType, data),
client.ForceOwnership, client.FieldOwner("opendatahub-operator"))
if err != nil {
return err
Expand Down
Loading