From 1089d36d08bb7d0b036af4a660cb442ca8e3a30b Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Fri, 7 Feb 2025 18:09:42 +0100 Subject: [PATCH] findings --- controllers/components/codeflare/codeflare.go | 4 +- controllers/components/dashboard/dashboard.go | 4 +- .../datasciencepipelines.go | 4 +- .../components/feastoperator/feastoperator.go | 4 +- controllers/components/kserve/kserve.go | 4 +- controllers/components/kueue/kueue.go | 4 +- .../kueue/kueue_controller_actions.go | 1 - .../modelcontroller/modelcontroller.go | 4 +- .../modelmeshserving/modelmeshserving.go | 4 +- .../components/modelregistry/modelregistry.go | 4 +- controllers/components/ray/ray.go | 4 +- .../trainingoperator/trainingoperator.go | 4 +- controllers/components/trustyai/trustyai.go | 4 +- .../components/workbenches/workbenches.go | 4 +- .../datasciencecluster_controller_actions.go | 2 +- .../datasciencecluster_controller_support.go | 20 +-- controllers/services/monitoring/monitoring.go | 2 +- pkg/cluster/resources.go | 2 +- pkg/controller/conditions/conditions.go | 14 +-- .../conditions/conditions_support.go | 119 +++++++++--------- pkg/utils/test/matchers/matchers.go | 2 +- 21 files changed, 103 insertions(+), 111 deletions(-) diff --git a/controllers/components/codeflare/codeflare.go b/controllers/components/codeflare/codeflare.go index 68db3d9ce90..cc95e4a19bd 100644 --- a/controllers/components/codeflare/codeflare.go +++ b/controllers/components/codeflare/codeflare.go @@ -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 @@ -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 } diff --git a/controllers/components/dashboard/dashboard.go b/controllers/components/dashboard/dashboard.go index ebedbc6e729..33d83ca2eea 100644 --- a/controllers/components/dashboard/dashboard.go +++ b/controllers/components/dashboard/dashboard.go @@ -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 @@ -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 } diff --git a/controllers/components/datasciencepipelines/datasciencepipelines.go b/controllers/components/datasciencepipelines/datasciencepipelines.go index 27cb2ee25c8..72556e9c0dd 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines.go @@ -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 @@ -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 } diff --git a/controllers/components/feastoperator/feastoperator.go b/controllers/components/feastoperator/feastoperator.go index 65767004201..19beca1d47d 100644 --- a/controllers/components/feastoperator/feastoperator.go +++ b/controllers/components/feastoperator/feastoperator.go @@ -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 @@ -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 } diff --git a/controllers/components/kserve/kserve.go b/controllers/components/kserve/kserve.go index 8f2ac791d6a..0ecb8967601 100644 --- a/controllers/components/kserve/kserve.go +++ b/controllers/components/kserve/kserve.go @@ -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 @@ -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 } diff --git a/controllers/components/kueue/kueue.go b/controllers/components/kueue/kueue.go index f4b6df00f86..85a9f687180 100644 --- a/controllers/components/kueue/kueue.go +++ b/controllers/components/kueue/kueue.go @@ -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 @@ -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 } diff --git a/controllers/components/kueue/kueue_controller_actions.go b/controllers/components/kueue/kueue_controller_actions.go index 8dc3d040f0c..76a3f2041ea 100644 --- a/controllers/components/kueue/kueue_controller_actions.go +++ b/controllers/components/kueue/kueue_controller_actions.go @@ -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) diff --git a/controllers/components/modelcontroller/modelcontroller.go b/controllers/components/modelcontroller/modelcontroller.go index d80076e4fef..67efcf948bc 100644 --- a/controllers/components/modelcontroller/modelcontroller.go +++ b/controllers/components/modelcontroller/modelcontroller.go @@ -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 @@ -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 } diff --git a/controllers/components/modelmeshserving/modelmeshserving.go b/controllers/components/modelmeshserving/modelmeshserving.go index 0c19572a33a..0bdf03680fe 100644 --- a/controllers/components/modelmeshserving/modelmeshserving.go +++ b/controllers/components/modelmeshserving/modelmeshserving.go @@ -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 @@ -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 } diff --git a/controllers/components/modelregistry/modelregistry.go b/controllers/components/modelregistry/modelregistry.go index bb6358dda2b..dba8df228d4 100644 --- a/controllers/components/modelregistry/modelregistry.go +++ b/controllers/components/modelregistry/modelregistry.go @@ -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 @@ -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 } diff --git a/controllers/components/ray/ray.go b/controllers/components/ray/ray.go index f930d4311da..b96b76afbe5 100644 --- a/controllers/components/ray/ray.go +++ b/controllers/components/ray/ray.go @@ -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 @@ -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 } diff --git a/controllers/components/trainingoperator/trainingoperator.go b/controllers/components/trainingoperator/trainingoperator.go index 65ce4dd3b22..47dbdbe2dce 100644 --- a/controllers/components/trainingoperator/trainingoperator.go +++ b/controllers/components/trainingoperator/trainingoperator.go @@ -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 @@ -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 } diff --git a/controllers/components/trustyai/trustyai.go b/controllers/components/trustyai/trustyai.go index c68343714d0..1f2362192bc 100644 --- a/controllers/components/trustyai/trustyai.go +++ b/controllers/components/trustyai/trustyai.go @@ -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 @@ -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 } diff --git a/controllers/components/workbenches/workbenches.go b/controllers/components/workbenches/workbenches.go index e4cab837fc9..41ab6f7f989 100644 --- a/controllers/components/workbenches/workbenches.go +++ b/controllers/components/workbenches/workbenches.go @@ -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 @@ -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 } diff --git a/controllers/datasciencecluster/datasciencecluster_controller_actions.go b/controllers/datasciencecluster/datasciencecluster_controller_actions.go index 5f8500ee17b..b37913d96af 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller_actions.go +++ b/controllers/datasciencecluster/datasciencecluster_controller_actions.go @@ -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 } diff --git a/controllers/datasciencecluster/datasciencecluster_controller_support.go b/controllers/datasciencecluster/datasciencecluster_controller_support.go index a74d3902839..ffbac248fe8 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller_support.go +++ b/controllers/datasciencecluster/datasciencecluster_controller_support.go @@ -2,6 +2,7 @@ package datasciencecluster import ( "context" + "errors" "fmt" "strings" @@ -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 @@ -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 @@ -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()) } @@ -65,14 +72,14 @@ 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, @@ -80,10 +87,7 @@ func computeComponentsStatus( 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 { diff --git a/controllers/services/monitoring/monitoring.go b/controllers/services/monitoring/monitoring.go index 349589480f5..98c8f8fae49 100644 --- a/controllers/services/monitoring/monitoring.go +++ b/controllers/services/monitoring/monitoring.go @@ -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 } } diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 0934e73510f..f3d072b43d7 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -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" diff --git a/pkg/controller/conditions/conditions.go b/pkg/controller/conditions/conditions.go index 092a983436d..666daaab18c 100644 --- a/pkg/controller/conditions/conditions.go +++ b/pkg/controller/conditions/conditions.go @@ -121,7 +121,7 @@ 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 { @@ -129,7 +129,7 @@ func (r *Manager) GetTopLevelCondition() *common.Condition { } 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 @@ -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) } @@ -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 diff --git a/pkg/controller/conditions/conditions_support.go b/pkg/controller/conditions/conditions_support.go index 6cda9fc1f1d..fc6f3f2ef27 100644 --- a/pkg/controller/conditions/conditions_support.go +++ b/pkg/controller/conditions/conditions_support.go @@ -1,6 +1,7 @@ package conditions import ( + "slices" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -8,104 +9,90 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" ) -func SetStatusCondition(conditions *[]common.Condition, newCondition common.Condition) bool { - if conditions == nil { - return false - } - - changed := false +func SetStatusCondition(a common.ConditionsAccessor, newCondition common.Condition) bool { + conditions := a.GetConditions() // reset LastHeartbeatTime to ensure is not set in any condition that is // eventually carrying it from an old implementation newCondition.LastHeartbeatTime = nil - existingCondition := FindStatusCondition(*conditions, newCondition.Type) - if existingCondition == nil { + idx := slices.IndexFunc(conditions, func(condition common.Condition) bool { + return condition.Type == newCondition.Type + }) + + if idx == -1 { if newCondition.LastTransitionTime.IsZero() { newCondition.LastTransitionTime = metav1.NewTime(time.Now()) } - *conditions = append(*conditions, newCondition) + conditions = append(conditions, newCondition) + a.SetConditions(conditions) return true } - if existingCondition.Status != newCondition.Status { - existingCondition.Status = newCondition.Status - if !newCondition.LastTransitionTime.IsZero() { - existingCondition.LastTransitionTime = newCondition.LastTransitionTime - } else { - existingCondition.LastTransitionTime = metav1.NewTime(time.Now()) - } - changed = true + if equals(conditions[idx], newCondition) { + return false } - if existingCondition.Reason != newCondition.Reason { - existingCondition.Reason = newCondition.Reason - changed = true - } - if existingCondition.Message != newCondition.Message { - existingCondition.Message = newCondition.Message - changed = true - } - if existingCondition.ObservedGeneration != newCondition.ObservedGeneration { - existingCondition.ObservedGeneration = newCondition.ObservedGeneration - changed = true - } - if existingCondition.Severity != newCondition.Severity { - existingCondition.Severity = newCondition.Severity - changed = true + updateTransitionTime := conditions[idx].Status != newCondition.Status + + conditions[idx] = newCondition + conditions[idx].LastHeartbeatTime = nil + + if updateTransitionTime { + conditions[idx].LastTransitionTime = newCondition.LastTransitionTime + + if conditions[idx].LastTransitionTime.IsZero() { + conditions[idx].LastTransitionTime = metav1.NewTime(time.Now()) + } } - // reset LastHeartbeatTime to ensure is not set in any condition that is - // eventually carrying it from an old implementation - existingCondition.LastHeartbeatTime = nil + a.SetConditions(conditions) - return changed + return true } -func RemoveStatusCondition(conditions *[]common.Condition, conditionType string) bool { - if conditions == nil || len(*conditions) == 0 { +func RemoveStatusCondition(a common.ConditionsAccessor, conditionType string) bool { + conditions := a.GetConditions() + l := len(conditions) + + if l == 0 { return false } - newConditions := make([]common.Condition, 0, len(*conditions)-1) - for _, condition := range *conditions { - if condition.Type != conditionType { - newConditions = append(newConditions, condition) - } - } + conditions = slices.DeleteFunc(conditions, func(condition common.Condition) bool { + return condition.Type == conditionType + }) - removed := len(*conditions) != len(newConditions) - *conditions = newConditions + removed := l != len(conditions) + if removed { + a.SetConditions(conditions) + } return removed } -func FindStatusCondition(conditions []common.Condition, conditionType string) *common.Condition { - for i := range conditions { - if conditions[i].Type == conditionType { - return &conditions[i] +func FindStatusCondition(a common.ConditionsAccessor, conditionType string) *common.Condition { + for _, c := range a.GetConditions() { + if c.Type == conditionType { + return c.DeepCopy() } } return nil } -func IsStatusConditionTrue(conditions []common.Condition, conditionType string) bool { - return IsStatusConditionPresentAndEqual(conditions, conditionType, metav1.ConditionTrue) +func IsStatusConditionTrue(a common.ConditionsAccessor, conditionType string) bool { + return IsStatusConditionPresentAndEqual(a, conditionType, metav1.ConditionTrue) } -func IsStatusConditionFalse(conditions []common.Condition, conditionType string) bool { - return IsStatusConditionPresentAndEqual(conditions, conditionType, metav1.ConditionFalse) +func IsStatusConditionFalse(a common.ConditionsAccessor, conditionType string) bool { + return IsStatusConditionPresentAndEqual(a, conditionType, metav1.ConditionFalse) } -func IsStatusConditionPresentAndEqual(conditions []common.Condition, conditionType string, status metav1.ConditionStatus) bool { - for _, condition := range conditions { - if condition.Type == conditionType { - return condition.Status == status - } - } - - return false +func IsStatusConditionPresentAndEqual(a common.ConditionsAccessor, conditionType string, status metav1.ConditionStatus) bool { + return slices.ContainsFunc(a.GetConditions(), func(condition common.Condition) bool { + return condition.Type == conditionType && condition.Status == status + }) } func applyOpts(c *common.Condition, opts ...Option) { @@ -113,3 +100,11 @@ func applyOpts(c *common.Condition, opts ...Option) { o(c) } } + +func equals(c1 common.Condition, c2 common.Condition) bool { + return c1.Status == c2.Status && + c1.Reason == c2.Reason && + c1.Message == c2.Message && + c1.ObservedGeneration == c2.ObservedGeneration && + c1.Severity == c2.Severity +} diff --git a/pkg/utils/test/matchers/matchers.go b/pkg/utils/test/matchers/matchers.go index da4f1631c22..a890c3bf936 100644 --- a/pkg/utils/test/matchers/matchers.go +++ b/pkg/utils/test/matchers/matchers.go @@ -8,7 +8,7 @@ import ( func ExtractStatusCondition(conditionType string) func(in types.ResourceObject) common.Condition { return func(in types.ResourceObject) common.Condition { - c := conditions.FindStatusCondition(in.GetStatus().Conditions, conditionType) + c := conditions.FindStatusCondition(in.GetStatus(), conditionType) if c == nil { return common.Condition{} }