Skip to content

Commit

Permalink
findings
Browse files Browse the repository at this point in the history
  • Loading branch information
lburgazzoli committed Feb 7, 2025
1 parent 1b735b8 commit 1089d36
Show file tree
Hide file tree
Showing 21 changed files with 103 additions and 111 deletions.
4 changes: 2 additions & 2 deletions controllers/components/codeflare/codeflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.CodeFlare.CodeFlareCommonStatus = c.Status.CodeFlareCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -98,7 +98,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/dashboard/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentNameUpstream] = true
dsc.Status.Components.Dashboard.DashboardCommonStatus = c.Status.DashboardCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -100,7 +100,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.DataSciencePipelines.DataSciencePipelinesCommonStatus = c.Status.DataSciencePipelinesCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -113,7 +113,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/feastoperator/feastoperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[ComponentName] = true
dsc.Status.Components.FeastOperator.FeastOperatorCommonStatus = c.Status.FeastOperatorCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -98,7 +98,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/kserve/kserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.Kserve.KserveCommonStatus = c.Status.KserveCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -126,7 +126,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/kueue/kueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.Kueue.KueueCommonStatus = c.Status.KueueCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -98,7 +98,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
1 change: 0 additions & 1 deletion controllers/components/kueue/kueue_controller_actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const (
)

func checkPreConditions(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {

rConfig, err := cluster.HasCRD(ctx, rr.Client, gvk.MultiKueueConfigV1Alpha1)
if err != nil {
return odherrors.NewStopError("failed to check %s CRDs version: %w", gvk.MultiKueueConfigV1Alpha1, err)
Expand Down
4 changes: 2 additions & 2 deletions controllers/components/modelcontroller/modelcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl

switch s.GetManagementState(dsc) {
case operatorv1.Managed:
if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -112,7 +112,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(&dsc.Status, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/modelmeshserving/modelmeshserving.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.ModelMeshServing.ModelMeshServingCommonStatus = c.Status.ModelMeshServingCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -100,7 +100,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/modelregistry/modelregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.ModelRegistry.ModelRegistryCommonStatus = c.Status.ModelRegistryCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -100,7 +100,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/ray/ray.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.Ray.RayCommonStatus = c.Status.RayCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -98,7 +98,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/trainingoperator/trainingoperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.TrainingOperator.TrainingOperatorCommonStatus = c.Status.TrainingOperatorCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -97,7 +97,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/trustyai/trustyai.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.TrustyAI.TrustyAICommonStatus = c.Status.TrustyAICommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -100,7 +100,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
4 changes: 2 additions & 2 deletions controllers/components/workbenches/workbenches.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.Components.Workbenches.WorkbenchesCommonStatus = c.Status.WorkbenchesCommonStatus.DeepCopy()

if rc := conditions.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil {
nc.Status = rc.Status
nc.Reason = rc.Reason
nc.Message = rc.Message
Expand All @@ -108,7 +108,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc))
}

conditions.SetStatusCondition(&dsc.Status.Conditions, nc)
conditions.SetStatusCondition(dsc, nc)

return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func updateStatus(ctx context.Context, rr *odhtype.ReconciliationRequest) error
instance.Status.InstalledComponents = make(map[string]bool)
}

err := computeComponentsStatus(ctx, rr.Client, instance, cr.DefaultRegistry())
err := computeComponentsStatus(ctx, rr.Client, rr, cr.DefaultRegistry())
if err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package datasciencecluster

import (
"context"
"errors"
"fmt"
"strings"

Expand All @@ -15,6 +16,7 @@ import (
cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry"
odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
)

// computeComponentsStatus checks the status of all registered components in a DataScienceCluster instance
Expand All @@ -31,9 +33,14 @@ import (
func computeComponentsStatus(
ctx context.Context,
cli *odhClient.Client,
instance *dscv1.DataScienceCluster,
rr *types.ReconciliationRequest,
reg *cr.Registry,
) error {
instance, ok := rr.Instance.(*dscv1.DataScienceCluster)
if !ok {
return errors.New("failed to convert to DataScienceCluster")
}

notReadyComponents := make([]string, 0)
managedComponent := 0

Expand All @@ -56,7 +63,7 @@ func computeComponentsStatus(

managedComponent++

if !conditions.IsStatusConditionTrue(ci.GetStatus().Conditions, status.ConditionTypeReady) {
if !conditions.IsStatusConditionTrue(ci, status.ConditionTypeReady) {
notReadyComponents = append(notReadyComponents, component.GetName())
}

Expand All @@ -65,25 +72,22 @@ func computeComponentsStatus(

switch {
case len(notReadyComponents) > 0:
conditions.SetStatusCondition(&instance.Status.Conditions, common.Condition{
rr.Conditions.SetCondition(common.Condition{
Type: status.ConditionTypeComponentsReady,
Status: metav1.ConditionFalse,
Reason: status.NotReadyReason,
Message: fmt.Sprintf("Some components are not ready: %s", strings.Join(notReadyComponents, ",")),
})
case managedComponent == 0:
conditions.SetStatusCondition(&instance.Status.Conditions, common.Condition{
rr.Conditions.SetCondition(common.Condition{
Type: status.ConditionTypeComponentsReady,
Status: metav1.ConditionTrue,
Severity: common.ConditionSeverityInfo,
Reason: status.NoManagedComponentsReason,
Message: status.NoManagedComponentsReason,
})
default:
conditions.SetStatusCondition(&instance.Status.Conditions, common.Condition{
Type: status.ConditionTypeComponentsReady,
Status: metav1.ConditionTrue,
})
rr.Conditions.MarkTrue(status.ConditionTypeComponentsReady)
}

if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion controllers/services/monitoring/monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,6 @@ func isComponentReady(ctx context.Context, cli *odhcli.Client, obj common.Platfo
case err != nil:
return false, fmt.Errorf("failed to get component instance: %w", err)
default:
return conditions.IsStatusConditionTrue(obj.GetStatus().Conditions, status.ConditionTypeReady), nil
return conditions.IsStatusConditionTrue(obj.GetStatus(), status.ConditionTypeReady), nil
}
}
2 changes: 1 addition & 1 deletion pkg/cluster/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
k8serr "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down
14 changes: 4 additions & 10 deletions pkg/controller/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,15 @@ func (r *Manager) IsHappy() bool {
return false
}

return IsStatusConditionTrue(r.accessor.GetConditions(), r.happy)
return IsStatusConditionTrue(r.accessor, r.happy)
}

func (r *Manager) GetTopLevelCondition() *common.Condition {
return r.GetCondition(r.happy)
}

func (r *Manager) GetCondition(t string) *common.Condition {
return FindStatusCondition(r.accessor.GetConditions(), t)
return FindStatusCondition(r.accessor, t)
}

// SetCondition sets the given condition on the manager. It updates the list of conditions and
Expand All @@ -143,13 +143,10 @@ func (r *Manager) SetCondition(cond common.Condition) {
return
}

conditions := r.accessor.GetConditions()

if !SetStatusCondition(&conditions, cond) {
if !SetStatusCondition(r.accessor, cond) {
return
}

r.accessor.SetConditions(conditions)
r.RecomputeHappiness(cond.Type)
}

Expand All @@ -166,13 +163,10 @@ func (r *Manager) ClearCondition(t string) error {
return nil
}

conditions := r.accessor.GetConditions()

if !RemoveStatusCondition(&conditions, t) {
if !RemoveStatusCondition(r.accessor, t) {
return nil
}

r.accessor.SetConditions(conditions)
r.RecomputeHappiness(t)

return nil
Expand Down
Loading

0 comments on commit 1089d36

Please sign in to comment.