From 92a3c25f56be92b2ad4173c44a154ee979c078f0 Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Fri, 17 Jan 2025 13:54:43 +0100 Subject: [PATCH 1/2] available condition --- .../v1/datasciencecluster_types.go | 34 +- .../v1/zz_generated.deepcopy.go | 9 +- ...er.opendatahub.io_datascienceclusters.yaml | 57 ++- ...er.opendatahub.io_datascienceclusters.yaml | 57 ++- controllers/components/codeflare/codeflare.go | 51 +-- .../codeflare/codeflare_controller.go | 10 +- .../components/codeflare/codeflare_support.go | 8 +- controllers/components/dashboard/dashboard.go | 51 +-- .../dashboard/dashboard_controller.go | 13 +- .../components/dashboard/dashboard_support.go | 7 +- .../datasciencepipelines.go | 51 +-- .../datasciencepipelines_controller.go | 8 +- ...datasciencepipelines_controller_actions.go | 53 ++- .../datasciencepipelines_support.go | 15 +- .../components/feastoperator/feastoperator.go | 51 +-- .../feastoperator/feastoperator_controller.go | 4 +- .../feastoperator/feastoperator_support.go | 4 +- controllers/components/kserve/kserve.go | 69 ++-- .../components/kserve/kserve_controller.go | 9 +- .../kserve/kserve_controller_actions.go | 83 +++-- .../components/kserve/kserve_support.go | 7 +- controllers/components/kueue/kueue.go | 51 +-- .../components/kueue/kueue_controller.go | 25 +- .../kueue/kueue_controller_actions.go | 37 +- controllers/components/kueue/kueue_support.go | 8 +- .../modelcontroller/modelcontroller.go | 51 +-- .../modelcontroller_controller.go | 10 +- .../modelcontroller_support.go | 8 +- .../modelmeshserving/modelmeshserving.go | 51 +-- .../modelmeshserving_controller.go | 8 +- .../modelmeshserving_support.go | 8 +- .../components/modelregistry/modelregistry.go | 51 +-- .../modelregistry/modelregistry_controller.go | 10 +- .../modelregistry_controller_actions.go | 50 +-- .../modelregistry/modelregistry_support.go | 18 +- controllers/components/ray/ray.go | 51 +-- controllers/components/ray/ray_controller.go | 8 +- controllers/components/ray/ray_support.go | 8 +- .../trainingoperator/trainingoperator.go | 51 +-- .../trainingoperator_controller.go | 8 +- .../trainingoperator_support.go | 8 +- controllers/components/trustyai/trustyai.go | 51 +-- .../trustyai/trustyai_controller.go | 10 +- .../trustyai/trustyai_controller_actions.go | 24 +- .../components/trustyai/trustyai_support.go | 8 +- .../components/workbenches/workbenches.go | 51 +-- .../workbenches/workbenches_controller.go | 7 +- .../workbenches/workbenches_support.go | 10 +- .../datasciencecluster_controller.go | 306 +++------------- .../datasciencecluster_controller_actions.go | 125 +++++++ .../datasciencecluster_controller_support.go | 86 +++++ controllers/services/auth/auth_controller.go | 1 - .../services/auth/auth_controller_actions.go | 11 - controllers/services/monitoring/monitoring.go | 2 +- .../monitoring/monitoring_controller.go | 15 +- .../monitoring_controller_actions.go | 58 --- docs/api-overview.md | 6 +- main.go | 30 +- pkg/componentsregistry/componentsregistry.go | 5 +- pkg/controller/actions/actions.go | 3 + .../actions/deploy/action_deploy.go | 5 +- .../actions/deploy/action_deploy_support.go | 14 + .../action_deployments_available.go} | 65 ++-- .../action_deployments_available_test.go} | 73 +++- pkg/controller/conditions/conditions.go | 345 ++++++++++++++++++ .../conditions/conditions_support.go | 22 +- pkg/controller/conditions/conditions_test.go | 144 ++++++++ .../predicates/dependent/dependent.go | 8 + pkg/controller/reconciler/reconciler.go | 153 ++++++-- .../reconciler/reconciler_support.go | 35 +- pkg/controller/reconciler/reconciler_test.go | 201 ++++++++++ pkg/controller/types/types.go | 12 +- tests/e2e/components_test.go | 1 + tests/e2e/datasciencepipelines_test.go | 18 + tests/e2e/kserve_test.go | 14 + tests/e2e/modelregistry_test.go | 14 + tests/e2e/workbenches_test.go | 3 +- 77 files changed, 2093 insertions(+), 1014 deletions(-) create mode 100644 controllers/datasciencecluster/datasciencecluster_controller_actions.go create mode 100644 controllers/datasciencecluster/datasciencecluster_controller_support.go rename pkg/controller/actions/{updatestatus/action_update_status.go => status/deployments/action_deployments_available.go} (64%) rename pkg/controller/actions/{updatestatus/action_update_status_test.go => status/deployments/action_deployments_available_test.go} (77%) create mode 100644 pkg/controller/conditions/conditions.go create mode 100644 pkg/controller/conditions/conditions_test.go create mode 100644 pkg/controller/reconciler/reconciler_test.go diff --git a/apis/datasciencecluster/v1/datasciencecluster_types.go b/apis/datasciencecluster/v1/datasciencecluster_types.go index 3115868b4a8..b728542fb2d 100644 --- a/apis/datasciencecluster/v1/datasciencecluster_types.go +++ b/apis/datasciencecluster/v1/datasciencecluster_types.go @@ -18,7 +18,6 @@ package v1 import ( "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -115,16 +114,7 @@ type ComponentsStatus struct { // DataScienceClusterStatus defines the observed state of DataScienceCluster. type DataScienceClusterStatus struct { - // Phase describes the Phase of DataScienceCluster reconciliation state - // This is used by OLM UI to provide status information to the user - Phase string `json:"phase,omitempty"` - - // Conditions describes the state of the DataScienceCluster resource. - // +optional - Conditions []conditionsv1.Condition `json:"conditions,omitempty"` - - // The generation observed by the deployment controller. - ObservedGeneration int64 `json:"observedGeneration,omitempty"` + common.Status `json:",inline"` // RelatedObjects is a list of objects created and maintained by this operator. // Object references will be added to this list after they have been created AND found in the cluster. @@ -143,10 +133,20 @@ type DataScienceClusterStatus struct { Release common.Release `json:"release,omitempty"` } +func (s *DataScienceClusterStatus) GetConditions() []common.Condition { + return s.Conditions +} + +func (s *DataScienceClusterStatus) SetConditions(conditions []common.Condition) { + s.Conditions = append(conditions[:0:0], conditions...) +} + // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:resource:scope=Cluster,shortName=dsc // +kubebuilder:storageversion +// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`,description="Ready" +// +kubebuilder:printcolumn:name="Reason",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].reason`,description="Reason" // DataScienceCluster is the Schema for the datascienceclusters API. type DataScienceCluster struct { @@ -166,6 +166,18 @@ type DataScienceClusterList struct { Items []DataScienceCluster `json:"items"` } +func (c *DataScienceCluster) GetConditions() []common.Condition { + return c.Status.GetConditions() +} + +func (c *DataScienceCluster) GetStatus() *common.Status { + return &c.Status.Status +} + +func (c *DataScienceCluster) SetConditions(conditions []common.Condition) { + c.Status.SetConditions(conditions) +} + func init() { SchemeBuilder.Register(&DataScienceCluster{}, &DataScienceClusterList{}) } diff --git a/apis/datasciencecluster/v1/zz_generated.deepcopy.go b/apis/datasciencecluster/v1/zz_generated.deepcopy.go index a7c9cfe5f33..1e0b174f030 100644 --- a/apis/datasciencecluster/v1/zz_generated.deepcopy.go +++ b/apis/datasciencecluster/v1/zz_generated.deepcopy.go @@ -21,7 +21,6 @@ limitations under the License. package v1 import ( - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" corev1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -158,13 +157,7 @@ func (in *DataScienceClusterSpec) DeepCopy() *DataScienceClusterSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DataScienceClusterStatus) DeepCopyInto(out *DataScienceClusterStatus) { *out = *in - if in.Conditions != nil { - in, out := &in.Conditions, &out.Conditions - *out = make([]conditionsv1.Condition, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } + in.Status.DeepCopyInto(&out.Status) if in.RelatedObjects != nil { in, out := &in.RelatedObjects, &out.RelatedObjects *out = make([]corev1.ObjectReference, len(*in)) diff --git a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml index b9cfd46c7a1..e6e2f7c1b77 100644 --- a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -16,7 +16,16 @@ spec: singular: datasciencecluster scope: Cluster versions: - - name: v1 + - additionalPrinterColumns: + - description: Ready + jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - description: Reason + jsonPath: .status.conditions[?(@.type=="Ready")].reason + name: Reason + type: string + name: v1 schema: openAPIV3Schema: description: DataScienceCluster is the Schema for the datascienceclusters @@ -1133,34 +1142,61 @@ spec: type: object type: object conditions: - description: Conditions describes the state of the DataScienceCluster - resource. items: - description: |- - Condition represents the state of the operator's - reconciliation functionality. properties: lastHeartbeatTime: + description: |- + The last time we got an update on a given condition, this should not be set and is + present only for backward compatibility reasons format: date-time type: string lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. + If that is not known, then using the time when the API field changed is acceptable. format: date-time type: string message: + description: message is a human-readable message indicating + details about the transition. type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + The value should be a CamelCase string. + type: string + severity: + description: |- + Severity with which to treat failures of this type of condition. + When this is not specified, it defaults to Error. type: string status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown type: string type: - description: ConditionType is the state of the operator's reconciliation - functionality. + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string required: - status - type type: object type: array + x-kubernetes-list-type: atomic errorMessage: type: string installedComponents: @@ -1169,13 +1205,10 @@ spec: description: List of components with status if installed or not type: object observedGeneration: - description: The generation observed by the deployment controller. + description: The generation observed by the resource controller. format: int64 type: integer phase: - description: |- - Phase describes the Phase of DataScienceCluster reconciliation state - This is used by OLM UI to provide status information to the user type: string relatedObjects: description: |- diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index d283af931d0..907afc6d2a3 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -16,7 +16,16 @@ spec: singular: datasciencecluster scope: Cluster versions: - - name: v1 + - additionalPrinterColumns: + - description: Ready + jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - description: Reason + jsonPath: .status.conditions[?(@.type=="Ready")].reason + name: Reason + type: string + name: v1 schema: openAPIV3Schema: description: DataScienceCluster is the Schema for the datascienceclusters @@ -1133,34 +1142,61 @@ spec: type: object type: object conditions: - description: Conditions describes the state of the DataScienceCluster - resource. items: - description: |- - Condition represents the state of the operator's - reconciliation functionality. properties: lastHeartbeatTime: + description: |- + The last time we got an update on a given condition, this should not be set and is + present only for backward compatibility reasons format: date-time type: string lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. + If that is not known, then using the time when the API field changed is acceptable. format: date-time type: string message: + description: message is a human-readable message indicating + details about the transition. type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + The value should be a CamelCase string. + type: string + severity: + description: |- + Severity with which to treat failures of this type of condition. + When this is not specified, it defaults to Error. type: string status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown type: string type: - description: ConditionType is the state of the operator's reconciliation - functionality. + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string required: - status - type type: object type: array + x-kubernetes-list-type: atomic errorMessage: type: string installedComponents: @@ -1169,13 +1205,10 @@ spec: description: List of components with status if installed or not type: object observedGeneration: - description: The generation observed by the deployment controller. + description: The generation observed by the resource controller. format: int64 type: integer phase: - description: |- - Phase describes the Phase of DataScienceCluster reconciliation state - This is used by OLM UI to provide status information to the user type: string relatedObjects: description: |- diff --git a/controllers/components/codeflare/codeflare.go b/controllers/components/codeflare/codeflare.go index 7c74ff48bba..e860d0262da 100644 --- a/controllers/components/codeflare/codeflare.go +++ b/controllers/components/codeflare/codeflare.go @@ -1,12 +1,12 @@ package codeflare import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -63,44 +64,50 @@ func (s *componentHandler) Init(_ common.Platform) error { return nil } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.CodeFlare) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.CodeFlare{} + c.Name = componentApi.CodeFlareInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, err + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to CodeFlare") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.CodeFlare.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.CodeFlare.CodeFlareCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.CodeFlare.CodeFlareCommonStatus = c.Status.CodeFlareCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/codeflare/codeflare_controller.go b/controllers/components/codeflare/codeflare_controller.go index 6969db01d3a..96016c876dd 100644 --- a/controllers/components/codeflare/codeflare_controller.go +++ b/controllers/components/codeflare/codeflare_controller.go @@ -30,8 +30,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -39,8 +39,6 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" ) -// CodeFlareReconciler reconciles a CodeFlare object. - func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error { _, err := reconciler.ReconcilerFor( mgr, @@ -65,7 +63,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. reconciler.WithPredicates( component.ForLabel(labels.ODH.Component(LegacyComponentName), labels.True)), ). - // Add CodeFlare-specific actions + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction()). @@ -77,9 +75,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). // must be final action WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) if err != nil { diff --git a/controllers/components/codeflare/codeflare_support.go b/controllers/components/codeflare/codeflare_support.go index 213e0d02fb7..45b1f5c8ec4 100644 --- a/controllers/components/codeflare/codeflare_support.go +++ b/controllers/components/codeflare/codeflare_support.go @@ -3,8 +3,6 @@ package codeflare import ( "path" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" @@ -14,7 +12,7 @@ import ( const ( ComponentName = componentApi.CodeFlareComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.CodeFlareKind + status.ReadySuffix) + ReadyConditionType = componentApi.CodeFlareKind + status.ReadySuffix // LegacyComponentName is the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing @@ -28,6 +26,10 @@ var ( imageParamMap = map[string]string{ "codeflare-operator-controller-image": "RELATED_IMAGE_ODH_CODEFLARE_OPERATOR_IMAGE", } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + } ) func manifestsPath() odhtypes.ManifestInfo { diff --git a/controllers/components/dashboard/dashboard.go b/controllers/components/dashboard/dashboard.go index 732efe32674..50eddeec701 100644 --- a/controllers/components/dashboard/dashboard.go +++ b/controllers/components/dashboard/dashboard.go @@ -1,12 +1,12 @@ package dashboard import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -65,44 +66,50 @@ func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) common.Pla } } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.Dashboard) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.Dashboard{} + c.Name = componentApi.DashboardInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to Dashboard") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentNameUpstream] = false dsc.Status.Components.Dashboard.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.Dashboard.DashboardCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentNameUpstream] = true dsc.Status.Components.Dashboard.DashboardCommonStatus = c.Status.DashboardCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/dashboard/dashboard_controller.go b/controllers/components/dashboard/dashboard_controller.go index ef0c9406097..24e09059a22 100644 --- a/controllers/components/dashboard/dashboard_controller.go +++ b/controllers/components/dashboard/dashboard_controller.go @@ -33,7 +33,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -86,7 +86,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. reconciler.Dynamic(), reconciler.WithPredicates(resources.Deleted()), ). - // actions + // Detecting component status should be the first step as the + // subsequent action could fail for transient errors, but we + // should report the current component status if possible + WithAction(deployments.NewAction()). + WithAction(updateStatus). WithAction(initialize). WithAction(devFlags). WithAction(setKustomizedParams). @@ -108,12 +112,13 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). - WithAction(updateStatus). // must be the final action WithAction(gc.NewAction( gc.WithUnremovables(gvk.OdhDashboardConfig), )). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) if err != nil { diff --git a/controllers/components/dashboard/dashboard_support.go b/controllers/components/dashboard/dashboard_support.go index 8f14f34dbde..b66322bff4f 100644 --- a/controllers/components/dashboard/dashboard_support.go +++ b/controllers/components/dashboard/dashboard_support.go @@ -4,7 +4,6 @@ import ( "context" "fmt" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" @@ -19,7 +18,7 @@ import ( const ( ComponentName = componentApi.DashboardComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.DashboardKind + status.ReadySuffix) + ReadyConditionType = componentApi.DashboardKind + status.ReadySuffix // Legacy component names are the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing @@ -61,6 +60,10 @@ var ( imagesMap = map[string]string{ "odh-dashboard-image": "RELATED_IMAGE_ODH_DASHBOARD_IMAGE", } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + } ) func defaultManifestInfo(p common.Platform) odhtypes.ManifestInfo { diff --git a/controllers/components/datasciencepipelines/datasciencepipelines.go b/controllers/components/datasciencepipelines/datasciencepipelines.go index b9f4f0ff293..62da58c7d81 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines.go @@ -1,12 +1,12 @@ package datasciencepipelines import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -17,6 +17,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -68,44 +69,50 @@ func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) common.Pla } } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.DataSciencePipelines) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.DataSciencePipelines{} + c.Name = componentApi.DataSciencePipelinesInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to DataSciencePipelines") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.DataSciencePipelines.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.DataSciencePipelines.DataSciencePipelinesCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.DataSciencePipelines.DataSciencePipelinesCommonStatus = c.Status.DataSciencePipelinesCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/datasciencepipelines/datasciencepipelines_controller.go b/controllers/components/datasciencepipelines/datasciencepipelines_controller.go index e3a426ba3ad..f03fbe8b9a2 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines_controller.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines_controller.go @@ -31,8 +31,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -61,8 +61,8 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. reconciler.WithPredicates( component.ForLabel(labels.ODH.Component(LegacyComponentName), labels.True)), ). - // Add datasciencepipelines-specific actions WithAction(checkPreConditions). + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction()). @@ -74,9 +74,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). // must be the final action WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) if err != nil { diff --git a/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go b/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go index d0dc7c65a1b..b85832983c5 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go @@ -20,15 +20,13 @@ import ( "context" "fmt" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" - odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + odherr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" @@ -36,35 +34,36 @@ import ( ) func checkPreConditions(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - dsp, ok := rr.Instance.(*componentApi.DataSciencePipelines) - if !ok { - return fmt.Errorf("resource instance %v is not a componentApi.DataSciencePipelines", rr.Instance) - } + rr.Conditions.MarkTrue(status.ConditionArgoWorkflowAvailable) - workflowCRD := &apiextensionsv1.CustomResourceDefinition{} - if err := rr.Client.Get(ctx, client.ObjectKey{Name: ArgoWorkflowCRD}, workflowCRD); err != nil { - if k8serr.IsNotFound(err) { - return nil - } - return odherrors.NewStopError("failed to get existing Workflow CRD : %v", err) + crd, err := cluster.GetCRD(ctx, rr.Client, ArgoWorkflowCRD) + switch { + case k8serr.IsNotFound(err): + return nil + case err != nil: + err = odherr.NewStopError("failed to check for existing %s CRD: %w", ArgoWorkflowCRD, err) + + rr.Conditions.MarkFalse( + status.ConditionArgoWorkflowAvailable, + conditions.WithObservedGeneration(rr.Instance.GetGeneration()), + conditions.WithError(err), + ) + + return err } // Verify if existing workflow is deployed by ODH with label // if not then set Argo capability status condition to false - odhLabelValue, odhLabelExists := workflowCRD.Labels[labels.ODH.Component(LegacyComponentName)] + odhLabelValue, odhLabelExists := crd.Labels[labels.ODH.Component(LegacyComponentName)] if !odhLabelExists || odhLabelValue != "true" { - s := dsp.GetStatus() - s.Phase = "NotReady" - - conditions.SetStatusCondition(dsp, common.Condition{ - Type: status.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: status.DataSciencePipelinesDoesntOwnArgoCRDReason, - Message: status.DataSciencePipelinesDoesntOwnArgoCRDMessage, - ObservedGeneration: s.ObservedGeneration, - }) - - return odherrors.NewStopError(status.DataSciencePipelinesDoesntOwnArgoCRDMessage) + rr.Conditions.MarkFalse( + status.ConditionArgoWorkflowAvailable, + conditions.WithObservedGeneration(rr.Instance.GetGeneration()), + conditions.WithReason(status.DataSciencePipelinesDoesntOwnArgoCRDReason), + conditions.WithMessage(status.DataSciencePipelinesDoesntOwnArgoCRDMessage), + ) + + return ErrArgoWorkflowAPINotOwned } return nil diff --git a/controllers/components/datasciencepipelines/datasciencepipelines_support.go b/controllers/components/datasciencepipelines/datasciencepipelines_support.go index f01c3c6d90c..ce0260b549a 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines_support.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines_support.go @@ -3,12 +3,11 @@ package datasciencepipelines import ( "path" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" ) @@ -17,7 +16,7 @@ const ( ArgoWorkflowCRD = "workflows.argoproj.io" ComponentName = componentApi.DataSciencePipelinesComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.DataSciencePipelinesKind + status.ReadySuffix) + ReadyConditionType = componentApi.DataSciencePipelinesKind + status.ReadySuffix // LegacyComponentName is the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing @@ -26,6 +25,10 @@ const ( platformVersionParamsKey = "PLATFORMVERSION" ) +var ( + ErrArgoWorkflowAPINotOwned = odherrors.NewStopError(status.DataSciencePipelinesDoesntOwnArgoCRDMessage) +) + var ( imageParamMap = map[string]string{ "IMAGES_DSPO": "RELATED_IMAGE_ODH_DATA_SCIENCE_PIPELINES_OPERATOR_CONTROLLER_IMAGE", @@ -46,6 +49,12 @@ var ( cluster.OpenDataHub: "overlays/odh", cluster.Unknown: "overlays/odh", } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + status.ConditionArgoWorkflowAvailable, + } + paramsPath = path.Join(odhdeploy.DefaultManifestPath, ComponentName, "base") ) diff --git a/controllers/components/feastoperator/feastoperator.go b/controllers/components/feastoperator/feastoperator.go index fef0eda4f6e..c3f2518af7e 100644 --- a/controllers/components/feastoperator/feastoperator.go +++ b/controllers/components/feastoperator/feastoperator.go @@ -1,12 +1,12 @@ package feastoperator import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -63,44 +64,50 @@ func (s *componentHandler) Init(_ common.Platform) error { return nil } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.FeastOperator) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.FeastOperator{} + c.Name = componentApi.FeastOperatorInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to FeastOperator") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[ComponentName] = false dsc.Status.Components.FeastOperator.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.FeastOperator.FeastOperatorCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[ComponentName] = true dsc.Status.Components.FeastOperator.FeastOperatorCommonStatus = c.Status.FeastOperatorCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/feastoperator/feastoperator_controller.go b/controllers/components/feastoperator/feastoperator_controller.go index 12e13277faf..52f6222b554 100644 --- a/controllers/components/feastoperator/feastoperator_controller.go +++ b/controllers/components/feastoperator/feastoperator_controller.go @@ -13,8 +13,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -41,6 +41,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. component.ForLabel(labels.ODH.Component(ComponentName), labels.True)), ). // Add FeastOperator-specific actions + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction()). @@ -52,7 +53,6 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). // must be the final action WithAction(gc.NewAction()). Build(ctx) diff --git a/controllers/components/feastoperator/feastoperator_support.go b/controllers/components/feastoperator/feastoperator_support.go index 5432c1ad475..f22da67b921 100644 --- a/controllers/components/feastoperator/feastoperator_support.go +++ b/controllers/components/feastoperator/feastoperator_support.go @@ -1,8 +1,6 @@ package feastoperator import ( - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" @@ -12,7 +10,7 @@ import ( const ( ComponentName = componentApi.FeastOperatorComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.FeastOperatorKind + status.ReadySuffix) + ReadyConditionType = componentApi.FeastOperatorKind + status.ReadySuffix ManifestsSourcePath = "overlays/odh" ) diff --git a/controllers/components/kserve/kserve.go b/controllers/components/kserve/kserve.go index 60d5f3830cf..a10dc11b729 100644 --- a/controllers/components/kserve/kserve.go +++ b/controllers/components/kserve/kserve.go @@ -1,12 +1,12 @@ package kserve import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -15,7 +15,9 @@ import ( dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" + odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -31,7 +33,22 @@ const ( // deployment to the new component name, so keep it around till we figure out a solution. LegacyComponentName = "kserve" - ReadyConditionType = conditionsv1.ConditionType(componentApi.KserveKind + status.ReadySuffix) + ReadyConditionType = componentApi.KserveKind + status.ReadySuffix +) + +var ( + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + status.ConditionServerlessAvailable, + status.ConditionServiceMeshAvailable, + } +) + +var ( + ErrServiceMeshNotConfigured = odherrors.NewStopError(status.ServiceMeshNotConfiguredMessage) + ErrServiceMeshMemberAPINotFound = odherrors.NewStopError(status.ServiceMeshOperatorNotInstalledMessage) + ErrServiceMeshOperatorNotInstalled = odherrors.NewStopError(status.ServiceMeshOperatorNotInstalledMessage) + ErrServerlessOperatorNotInstalled = odherrors.NewStopError(status.ServerlessOperatorNotInstalledMessage) ) type componentHandler struct{} @@ -75,44 +92,50 @@ func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) common.Pla } } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.Kserve) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.Kserve{} + c.Name = componentApi.KserveInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to Kserve") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.Kserve.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.Kserve.KserveCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.Kserve.KserveCommonStatus = c.Status.KserveCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/kserve/kserve_controller.go b/controllers/components/kserve/kserve_controller.go index 4202b4599cb..37db5d79bf3 100644 --- a/controllers/components/kserve/kserve_controller.go +++ b/controllers/components/kserve/kserve_controller.go @@ -38,8 +38,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/clusterrole" @@ -150,6 +150,8 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. // actions WithAction(checkPreConditions). + WithAction(deployments.NewAction()). + WithAction(setStatusFields). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction()). @@ -173,10 +175,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(setStatusFields). - WithAction(updatestatus.NewAction()). // must be the final action WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) return err diff --git a/controllers/components/kserve/kserve_controller_actions.go b/controllers/components/kserve/kserve_controller_actions.go index 55c755baaa8..6b31467f88f 100644 --- a/controllers/components/kserve/kserve_controller_actions.go +++ b/controllers/components/kserve/kserve_controller_actions.go @@ -10,7 +10,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" 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" @@ -36,60 +35,66 @@ func checkPreConditions(ctx context.Context, rr *odhtypes.ReconciliationRequest) } if k.Spec.Serving.ManagementState != operatorv1.Managed { + rr.Conditions.MarkFalse( + status.ConditionServerlessAvailable, + conditions.WithSeverity(common.ConditionSeverityInfo), + conditions.WithReason(string(k.Spec.Serving.ManagementState)), + conditions.WithMessage("Serving management state is set to: %s", k.Spec.Serving.ManagementState)) + rr.Conditions.MarkFalse( + status.ConditionServiceMeshAvailable, + conditions.WithSeverity(common.ConditionSeverityInfo), + conditions.WithReason(string(k.Spec.Serving.ManagementState)), + conditions.WithMessage("Serving management state is set to: %s", k.Spec.Serving.ManagementState)) + return nil } + rr.Conditions.MarkUnknown(status.ConditionServerlessAvailable) + rr.Conditions.MarkUnknown(status.ConditionServiceMeshAvailable) + if rr.DSCI.Spec.ServiceMesh == nil || rr.DSCI.Spec.ServiceMesh.ManagementState != operatorv1.Managed { - s := k.GetStatus() - s.Phase = status.PhaseNotReady - - conditions.SetStatusCondition(k, common.Condition{ - Type: status.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: status.ServiceMeshNotConfiguredReason, - Message: status.ServiceMeshNotConfiguredMessage, - ObservedGeneration: s.ObservedGeneration, - }) - - return odherrors.NewStopError(status.ServiceMeshNotConfiguredMessage) + rr.Conditions.MarkFalse( + status.ConditionServerlessAvailable, + conditions.WithObservedGeneration(rr.Instance.GetGeneration()), + conditions.WithReason(status.ServiceMeshNotConfiguredReason), + conditions.WithMessage(status.ServiceMeshNotConfiguredMessage), + ) + + return ErrServiceMeshNotConfigured } if found, err := cluster.OperatorExists(ctx, rr.Client, serviceMeshOperator); err != nil || !found { - s := k.GetStatus() - s.Phase = status.PhaseNotReady - - conditions.SetStatusCondition(k, common.Condition{ - Type: status.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: status.ServiceMeshOperatorNotInstalledReason, - Message: status.ServiceMeshOperatorNotInstalledMessage, - ObservedGeneration: s.ObservedGeneration, - }) - + e := ErrServiceMeshOperatorNotInstalled if err != nil { - return odherrors.NewStopErrorW(err) + e = odherrors.NewStopErrorW(err) } - return odherrors.NewStopError(status.ServiceMeshOperatorNotInstalledMessage) + rr.Conditions.MarkFalse( + status.ConditionServiceMeshAvailable, + conditions.WithObservedGeneration(rr.Instance.GetGeneration()), + conditions.WithError(e), + ) + + return e + } else { + rr.Conditions.MarkTrue(status.ConditionServiceMeshAvailable) } if found, err := cluster.OperatorExists(ctx, rr.Client, serverlessOperator); err != nil || !found { - s := k.GetStatus() - s.Phase = status.PhaseNotReady - - conditions.SetStatusCondition(k, common.Condition{ - Type: status.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: status.ServerlessOperatorNotInstalledReason, - Message: status.ServerlessOperatorNotInstalledMessage, - ObservedGeneration: s.ObservedGeneration, - }) - + e := ErrServerlessOperatorNotInstalled if err != nil { - return odherrors.NewStopErrorW(err) + e = odherrors.NewStopErrorW(err) } - return odherrors.NewStopError(status.ServerlessOperatorNotInstalledMessage) + rr.Conditions.MarkFalse( + status.ConditionServerlessAvailable, + conditions.WithObservedGeneration(rr.Instance.GetGeneration()), + conditions.WithError(e), + ) + + return e + } else { + rr.Conditions.MarkTrue(status.ConditionServerlessAvailable) } return nil diff --git a/controllers/components/kserve/kserve_support.go b/controllers/components/kserve/kserve_support.go index 3a374eaaeea..0c5f4cb1fd8 100644 --- a/controllers/components/kserve/kserve_support.go +++ b/controllers/components/kserve/kserve_support.go @@ -8,6 +8,7 @@ import ( "path" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -153,7 +154,11 @@ func removeServerlessFeatures(ctx context.Context, cli client.Client, k *compone func getDefaultDeploymentMode(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec) (string, error) { kserveConfigMap := corev1.ConfigMap{} - if err := cli.Get(ctx, client.ObjectKey{Name: kserveConfigMapName, Namespace: dscispec.ApplicationsNamespace}, &kserveConfigMap); err != nil { + err := cli.Get(ctx, client.ObjectKey{Name: kserveConfigMapName, Namespace: dscispec.ApplicationsNamespace}, &kserveConfigMap) + if errors.IsNotFound(err) { + return "", nil + } + if err != nil { return "", err } diff --git a/controllers/components/kueue/kueue.go b/controllers/components/kueue/kueue.go index 4d120b2caca..79d734e823e 100644 --- a/controllers/components/kueue/kueue.go +++ b/controllers/components/kueue/kueue.go @@ -1,12 +1,12 @@ package kueue import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -63,44 +64,50 @@ func (s *componentHandler) Init(platform common.Platform) error { return nil } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.Kueue) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.Kueue{} + c.Name = componentApi.KueueInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to Kueue") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.Kueue.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.Kueue.KueueCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.Kueue.KueueCommonStatus = c.Status.KueueCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/kueue/kueue_controller.go b/controllers/components/kueue/kueue_controller.go index 55822134713..839404497bf 100644 --- a/controllers/components/kueue/kueue_controller.go +++ b/controllers/components/kueue/kueue_controller.go @@ -35,8 +35,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -48,10 +48,17 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. b := reconciler.ReconcilerFor(mgr, &componentApi.Kueue{}) if cluster.GetClusterInfo().Version.GTE(semver.MustParse("4.17.0")) { - b = b.OwnsGVK(gvk.ValidatingAdmissionPolicy). // "own" VAP, because we want it has owner so when kueue is removed it gets cleaned. - WatchesGVK(gvk.ValidatingAdmissionPolicyBinding). // "watch" VAPB, because we want it to be configable by user and it can be left behind when kueue is remov - WithAction(extraInitialize) + // "own" VAP, because we want it has owner so when kueue is removed it gets cleaned. + b = b.OwnsGVK(gvk.ValidatingAdmissionPolicy) + + // "watch" VAPB, because we want it to be configurable by user, and it can be left behind + // when kueue is removed + b = b.WatchesGVK(gvk.ValidatingAdmissionPolicyBinding) + + // add OCP 4.17.0 specific menifests + b = b.WithAction(extraInitialize) } + // customized Owns() for Component with new predicates b.Owns(&corev1.ConfigMap{}). Owns(&corev1.Secret{}). @@ -74,8 +81,8 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. reconciler.WithPredicates( component.ForLabel(labels.ODH.Component(LegacyComponentName), labels.True)), ). - // Add Kueue-specific actions - WithAction(checkPreConditions). // check if CRD multikueueconfigs/multikueueclusters with v1alpha1 exist in cluster and not in termination + WithAction(checkPreConditions). + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction()). @@ -88,9 +95,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). // must be the final action - WithAction(gc.NewAction()) + WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...) if _, err := b.Build(ctx); err != nil { return err // no need customize error, it is done in the caller main diff --git a/controllers/components/kueue/kueue_controller_actions.go b/controllers/components/kueue/kueue_controller_actions.go index 2393de1589e..4ad3e0c5d79 100644 --- a/controllers/components/kueue/kueue_controller_actions.go +++ b/controllers/components/kueue/kueue_controller_actions.go @@ -4,44 +4,35 @@ import ( "context" "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" - "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" ) +const ( + MultiKueueCRDMessage = "MultiKueue CRDs MultiKueueConfig v1alpha1 and/or MultiKueueCluster v1Alpha1 exist, please remove them to proceed" +) + func checkPreConditions(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - k, ok := rr.Instance.(*componentApi.Kueue) - if !ok { - return fmt.Errorf("resource instance %v is not a componentApi.Kueue)", rr.Instance) + rConfig, err := cluster.HasCRDWithVersion(ctx, rr.Client, gvk.MultiKueueConfigV1Alpha1.GroupKind(), gvk.MultiKueueConfigV1Alpha1.Version) + if err != nil { + return odherrors.NewStopError("failed to check %s CRDs version: %w", gvk.MultiKueueConfigV1Alpha1, err) } - rConfig, eConfig := cluster.HasCRDWithVersion(ctx, rr.Client, gvk.MultiKueueConfigV1Alpha1.GroupKind(), gvk.MultiKueueConfigV1Alpha1.Version) - rCluster, eCluster := cluster.HasCRDWithVersion(ctx, rr.Client, gvk.MultikueueClusterV1Alpha1.GroupKind(), gvk.MultikueueClusterV1Alpha1.Version) - if eConfig != nil || eCluster != nil { - return odherrors.NewStopError("failed to check CRDs version: %v, %v", eConfig, eCluster) + rCluster, err := cluster.HasCRDWithVersion(ctx, rr.Client, gvk.MultikueueClusterV1Alpha1.GroupKind(), gvk.MultikueueClusterV1Alpha1.Version) + if err != nil { + return odherrors.NewStopError("failed to check %s CRDs version: %w", gvk.MultiKueueConfigV1Alpha1, err) } - if rConfig || rCluster { - s := k.GetStatus() - s.Phase = status.PhaseNotReady - conditions.SetStatusCondition(k, common.Condition{ - Type: status.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: status.MultiKueueCRDReason, - Message: status.MultiKueueCRDMessage, - ObservedGeneration: s.ObservedGeneration, - }) - return odherrors.NewStopError(status.MultiKueueCRDMessage) + + if !rConfig || !rCluster { + return odherrors.NewStopError(MultiKueueCRDMessage) } + return nil } diff --git a/controllers/components/kueue/kueue_support.go b/controllers/components/kueue/kueue_support.go index 4632b0ca8ad..e71a16aad2d 100644 --- a/controllers/components/kueue/kueue_support.go +++ b/controllers/components/kueue/kueue_support.go @@ -1,8 +1,6 @@ package kueue import ( - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" @@ -12,7 +10,7 @@ import ( const ( ComponentName = componentApi.KueueComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.KueueKind + status.ReadySuffix) + ReadyConditionType = componentApi.KueueKind + status.ReadySuffix // LegacyComponentName is the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing @@ -24,6 +22,10 @@ var ( imageParamMap = map[string]string{ "odh-kueue-controller-image": "RELATED_IMAGE_ODH_KUEUE_CONTROLLER_IMAGE", } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + } ) func manifestsPath() odhtypes.ManifestInfo { diff --git a/controllers/components/modelcontroller/modelcontroller.go b/controllers/components/modelcontroller/modelcontroller.go index 035caf0c7c0..e23efe50de2 100644 --- a/controllers/components/modelcontroller/modelcontroller.go +++ b/controllers/components/modelcontroller/modelcontroller.go @@ -1,12 +1,12 @@ package modelcontroller import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -84,37 +85,43 @@ func (s *componentHandler) Init(_ common.Platform) error { return nil } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.ModelController) - if !ok { - return errors.New("failed to convert to ModelController") +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.ModelController{} + c.Name = componentApi.ModelControllerInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil } - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) + if !ok { + return cs, errors.New("failed to convert to DataScienceCluster") } + rr.Conditions.MarkFalse(ReadyConditionType) + switch s.GetManagementState(dsc) { case operatorv1.Managed: - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/modelcontroller/modelcontroller_controller.go b/controllers/components/modelcontroller/modelcontroller_controller.go index 6417c5e65ee..b8c2238be26 100644 --- a/controllers/components/modelcontroller/modelcontroller_controller.go +++ b/controllers/components/modelcontroller/modelcontroller_controller.go @@ -33,7 +33,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -66,9 +66,9 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. reconciler.WithPredicates( component.ForLabel(labels.ODH.Component(LegacyComponentName), labels.True)), ). - // Add ModelController specific actions + WithAction(deployments.NewAction()). WithAction(initialize). - WithAction(devFlags). // devFlags triggerd by changes in DSC kserve and ModelMeshServing, also update .status.devflagurl + WithAction(devFlags). WithAction(kustomize.NewAction( kustomize.WithCache(), kustomize.WithLabel(labels.ODH.Component(LegacyComponentName), labels.True), @@ -77,8 +77,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) // include GenerationChangedPredicate no need set in each Owns() above if err != nil { diff --git a/controllers/components/modelcontroller/modelcontroller_support.go b/controllers/components/modelcontroller/modelcontroller_support.go index 49fe24a8189..6967e1b502b 100644 --- a/controllers/components/modelcontroller/modelcontroller_support.go +++ b/controllers/components/modelcontroller/modelcontroller_support.go @@ -1,8 +1,6 @@ package modelcontroller import ( - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" @@ -12,7 +10,7 @@ import ( const ( ComponentName = componentApi.ModelControllerComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.ModelControllerKind + status.ReadySuffix) + ReadyConditionType = componentApi.ModelControllerKind + status.ReadySuffix // LegacyComponentName is the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing @@ -24,6 +22,10 @@ var ( imageParamMap = map[string]string{ "odh-model-controller": "RELATED_IMAGE_ODH_MODEL_CONTROLLER_IMAGE", } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + } ) func manifestsPath() types.ManifestInfo { diff --git a/controllers/components/modelmeshserving/modelmeshserving.go b/controllers/components/modelmeshserving/modelmeshserving.go index 26b6034f001..44ae332614e 100644 --- a/controllers/components/modelmeshserving/modelmeshserving.go +++ b/controllers/components/modelmeshserving/modelmeshserving.go @@ -1,12 +1,12 @@ package modelmeshserving import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -65,44 +66,50 @@ func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) common.Pla } } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.ModelMeshServing) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.ModelMeshServing{} + c.Name = componentApi.ModelMeshServingInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to ModelMeshServing") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.ModelMeshServing.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.ModelMeshServing.ModelMeshServingCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.ModelMeshServing.ModelMeshServingCommonStatus = c.Status.ModelMeshServingCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/modelmeshserving/modelmeshserving_controller.go b/controllers/components/modelmeshserving/modelmeshserving_controller.go index 516f7466319..7fa0a01084a 100644 --- a/controllers/components/modelmeshserving/modelmeshserving_controller.go +++ b/controllers/components/modelmeshserving/modelmeshserving_controller.go @@ -34,8 +34,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/clusterrole" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" @@ -83,7 +83,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. }, )), ). - // Add ModelMeshServing specific actions + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction()). @@ -95,8 +95,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) // include GenerationChangedPredicate no need set in each Owns() above if err != nil { diff --git a/controllers/components/modelmeshserving/modelmeshserving_support.go b/controllers/components/modelmeshserving/modelmeshserving_support.go index f90066198d4..991580bf56b 100644 --- a/controllers/components/modelmeshserving/modelmeshserving_support.go +++ b/controllers/components/modelmeshserving/modelmeshserving_support.go @@ -1,8 +1,6 @@ package modelmeshserving import ( - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" @@ -12,7 +10,7 @@ import ( const ( ComponentName = componentApi.ModelMeshServingComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.ModelMeshServingKind + status.ReadySuffix) + ReadyConditionType = componentApi.ModelMeshServingKind + status.ReadySuffix // LegacyComponentName is the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing @@ -27,6 +25,10 @@ var ( "odh-modelmesh": "RELATED_IMAGE_ODH_MODELMESH_IMAGE", "odh-modelmesh-controller": "RELATED_IMAGE_ODH_MODELMESH_CONTROLLER_IMAGE", } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + } ) func manifestsPath() odhtypes.ManifestInfo { diff --git a/controllers/components/modelregistry/modelregistry.go b/controllers/components/modelregistry/modelregistry.go index 33f407dab13..a1380cf0b4b 100644 --- a/controllers/components/modelregistry/modelregistry.go +++ b/controllers/components/modelregistry/modelregistry.go @@ -1,12 +1,12 @@ package modelregistry import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -65,44 +66,50 @@ func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) common.Pla } } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.ModelRegistry) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.ModelRegistry{} + c.Name = componentApi.ModelRegistryInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to ModelRegistry") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.ModelRegistry.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.ModelRegistry.ModelRegistryCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.ModelRegistry.ModelRegistryCommonStatus = c.Status.ModelRegistryCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/modelregistry/modelregistry_controller.go b/controllers/components/modelregistry/modelregistry_controller.go index 9ae46c52ee8..c6c88d77e91 100644 --- a/controllers/components/modelregistry/modelregistry_controller.go +++ b/controllers/components/modelregistry/modelregistry_controller.go @@ -34,8 +34,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/template" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/generation" @@ -80,8 +80,9 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. // namespaces that may not be known when the controller is started, hence // it should be watched dynamically WatchesGVK(gvk.ServiceMeshMember, reconciler.Dynamic()). - // actions WithAction(checkPreConditions). + WithAction(deployments.NewAction()). + WithAction(updateStatus). WithAction(initialize). WithAction(releases.NewAction()). WithAction(configureDependencies). @@ -97,12 +98,13 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). - WithAction(updateStatus). // must be the final action WithAction(gc.NewAction( gc.WithUnremovables(gvk.ServiceMeshMember), )). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) if err != nil { diff --git a/controllers/components/modelregistry/modelregistry_controller_actions.go b/controllers/components/modelregistry/modelregistry_controller_actions.go index 1a8eff0c9b7..a6e7283e295 100644 --- a/controllers/components/modelregistry/modelregistry_controller_actions.go +++ b/controllers/components/modelregistry/modelregistry_controller_actions.go @@ -7,14 +7,13 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" - odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" @@ -24,28 +23,36 @@ import ( _ "embed" ) -func checkPreConditions(_ context.Context, rr *odhtypes.ReconciliationRequest) error { - mr, ok := rr.Instance.(*componentApi.ModelRegistry) - if !ok { - return fmt.Errorf("resource instance %v is not a componentApi.ModelRegistry", rr.Instance) - } +func checkPreConditions(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { + rr.Conditions.MarkTrue(status.ConditionServerlessAvailable) - if rr.DSCI.Spec.ServiceMesh != nil && rr.DSCI.Spec.ServiceMesh.ManagementState == operatorv1.Managed { - return nil - } + if rr.DSCI.Spec.ServiceMesh == nil || rr.DSCI.Spec.ServiceMesh.ManagementState != operatorv1.Managed { + rr.Conditions.MarkFalse( + status.ConditionServerlessAvailable, + conditions.WithObservedGeneration(rr.Instance.GetGeneration()), + conditions.WithReason(status.ServiceMeshNotConfiguredReason), + conditions.WithMessage(status.ServiceMeshNotConfiguredMessage), + ) - s := mr.GetStatus() - s.Phase = "NotReady" + return ErrServiceMeshNotConfigured + } - conditions.SetStatusCondition(mr, common.Condition{ - Type: status.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: status.ServiceMeshNotConfiguredReason, - Message: status.ServiceMeshNotConfiguredMessage, - ObservedGeneration: s.ObservedGeneration, - }) + _, err := cluster.GetCRD(ctx, rr.Client, ServiceMeshMemberCRD) + switch { + case k8serr.IsNotFound(err): + rr.Conditions.MarkFalse( + status.ConditionServerlessAvailable, + conditions.WithObservedGeneration(rr.Instance.GetGeneration()), + conditions.WithReason(status.ServiceMeshNotConfiguredReason), + conditions.WithMessage(ServiceMeshMemberAPINotFound), + ) + + return ErrServiceMeshMemberAPINotFound + case err != nil: + return err + } - return odherrors.NewStopError(status.ServiceMeshNotConfiguredMessage) + return nil } func initialize(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { @@ -97,7 +104,6 @@ func configureDependencies(ctx context.Context, rr *odhtypes.ReconciliationReque } // Namespace - if err := rr.AddResources( &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -105,7 +111,7 @@ func configureDependencies(ctx context.Context, rr *odhtypes.ReconciliationReque }, }, ); err != nil { - return fmt.Errorf("failed to add namespace %s to manifests", mr.Spec.RegistriesNamespace) + return fmt.Errorf("failed to add namespace %s to manifests: %w", mr.Spec.RegistriesNamespace, err) } // Secret diff --git a/controllers/components/modelregistry/modelregistry_support.go b/controllers/components/modelregistry/modelregistry_support.go index 4cfd0842821..fc3fa311272 100644 --- a/controllers/components/modelregistry/modelregistry_support.go +++ b/controllers/components/modelregistry/modelregistry_support.go @@ -4,10 +4,9 @@ import ( "embed" "path" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" + odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" ) @@ -15,19 +14,25 @@ import ( const ( ComponentName = componentApi.ModelRegistryComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.ModelRegistryKind + status.ReadySuffix) + ReadyConditionType = componentApi.ModelRegistryKind + status.ReadySuffix DefaultModelRegistriesNamespace = "odh-model-registries" DefaultModelRegistryCert = "default-modelregistry-cert" BaseManifestsSourcePath = "overlays/odh" ServiceMeshMemberTemplate = "resources/servicemesh-member.tmpl.yaml" - + ServiceMeshMemberCRD = "servicemeshmembers.maistra.io" + ServiceMeshMemberAPINotFound = "ServiceMeshMember API not found" // LegacyComponentName is the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing // deployment to the new component name, so keep it around till we figure out a solution. LegacyComponentName = "model-registry-operator" ) +var ( + ErrServiceMeshNotConfigured = odherrors.NewStopError(status.ServiceMeshNotConfiguredMessage) + ErrServiceMeshMemberAPINotFound = odherrors.NewStopError(ServiceMeshMemberAPINotFound) +) + var ( imagesMap = map[string]string{ "IMAGES_MODELREGISTRY_OPERATOR": "RELATED_IMAGE_ODH_MODEL_REGISTRY_OPERATOR_IMAGE", @@ -38,6 +43,11 @@ var ( extraParamsMap = map[string]string{ "DEFAULT_CERT": DefaultModelRegistryCert, } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + status.ConditionServerlessAvailable, + } ) //go:embed resources diff --git a/controllers/components/ray/ray.go b/controllers/components/ray/ray.go index f47fc6fa6f2..b868de035de 100644 --- a/controllers/components/ray/ray.go +++ b/controllers/components/ray/ray.go @@ -1,12 +1,12 @@ package ray import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -63,44 +64,50 @@ func (s *componentHandler) Init(_ common.Platform) error { return nil } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.Ray) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.Ray{} + c.Name = componentApi.RayInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to Ray") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.Ray.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.Ray.RayCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.Ray.RayCommonStatus = c.Status.RayCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/ray/ray_controller.go b/controllers/components/ray/ray_controller.go index 48eca16ad83..f7c39b96e4c 100644 --- a/controllers/components/ray/ray_controller.go +++ b/controllers/components/ray/ray_controller.go @@ -30,8 +30,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -58,7 +58,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. reconciler.WithPredicates( component.ForLabel(labels.ODH.Component(LegacyComponentName), labels.True)), ). - // Add Ray-specific actions + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction()). @@ -70,9 +70,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). // must be the final action WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) if err != nil { diff --git a/controllers/components/ray/ray_support.go b/controllers/components/ray/ray_support.go index 34a9f33136f..e71f744f0b0 100644 --- a/controllers/components/ray/ray_support.go +++ b/controllers/components/ray/ray_support.go @@ -1,8 +1,6 @@ package ray import ( - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" @@ -12,7 +10,7 @@ import ( const ( ComponentName = componentApi.RayComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.RayKind + status.ReadySuffix) + ReadyConditionType = componentApi.RayKind + status.ReadySuffix // LegacyComponentName is the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing @@ -24,6 +22,10 @@ var ( imageParamMap = map[string]string{ "odh-kuberay-operator-controller-image": "RELATED_IMAGE_ODH_KUBERAY_OPERATOR_CONTROLLER_IMAGE", } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + } ) func manifestPath() types.ManifestInfo { diff --git a/controllers/components/trainingoperator/trainingoperator.go b/controllers/components/trainingoperator/trainingoperator.go index 3418972f704..f879a6e4c5a 100644 --- a/controllers/components/trainingoperator/trainingoperator.go +++ b/controllers/components/trainingoperator/trainingoperator.go @@ -1,12 +1,12 @@ package trainingoperator import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -62,44 +63,50 @@ func (s *componentHandler) Init(platform common.Platform) error { return nil } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.TrainingOperator) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.TrainingOperator{} + c.Name = componentApi.TrainingOperatorInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to TrainingOperator") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.TrainingOperator.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.TrainingOperator.TrainingOperatorCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.TrainingOperator.TrainingOperatorCommonStatus = c.Status.TrainingOperatorCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/trainingoperator/trainingoperator_controller.go b/controllers/components/trainingoperator/trainingoperator_controller.go index 6e5ceecfd10..a9b456b967f 100644 --- a/controllers/components/trainingoperator/trainingoperator_controller.go +++ b/controllers/components/trainingoperator/trainingoperator_controller.go @@ -30,8 +30,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -55,7 +55,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. reconciler.WithPredicates( component.ForLabel(labels.ODH.Component(LegacyComponentName), labels.True)), ). - // Add TrainingOperator-specific actions + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction()). @@ -67,9 +67,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). // must be the final action WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) if err != nil { diff --git a/controllers/components/trainingoperator/trainingoperator_support.go b/controllers/components/trainingoperator/trainingoperator_support.go index c99b7e9ff22..f3aef6ac1b5 100644 --- a/controllers/components/trainingoperator/trainingoperator_support.go +++ b/controllers/components/trainingoperator/trainingoperator_support.go @@ -1,8 +1,6 @@ package trainingoperator import ( - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" @@ -12,7 +10,7 @@ import ( const ( ComponentName = componentApi.TrainingOperatorComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.TrainingOperatorKind + status.ReadySuffix) + ReadyConditionType = componentApi.TrainingOperatorKind + status.ReadySuffix // LegacyComponentName is the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing @@ -24,6 +22,10 @@ var ( imageParamMap = map[string]string{ "odh-training-operator-controller-image": "RELATED_IMAGE_ODH_TRAINING_OPERATOR_IMAGE", } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + } ) func manifestPath() types.ManifestInfo { diff --git a/controllers/components/trustyai/trustyai.go b/controllers/components/trustyai/trustyai.go index a9c51076dad..e9c882aaf70 100644 --- a/controllers/components/trustyai/trustyai.go +++ b/controllers/components/trustyai/trustyai.go @@ -1,12 +1,12 @@ package trustyai import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -65,44 +66,50 @@ func (s *componentHandler) Init(platform common.Platform) error { return nil } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.TrustyAI) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.TrustyAI{} + c.Name = componentApi.TrustyAIInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to TrustyAI") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.TrustyAI.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.TrustyAI.TrustyAICommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.TrustyAI.TrustyAICommonStatus = c.Status.TrustyAICommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/trustyai/trustyai_controller.go b/controllers/components/trustyai/trustyai_controller.go index c0cc39c8588..464415f8675 100644 --- a/controllers/components/trustyai/trustyai_controller.go +++ b/controllers/components/trustyai/trustyai_controller.go @@ -31,8 +31,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -65,8 +65,8 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. }, )), ). - // Add TrustyAI-specific actions - WithAction(checkPreConditions). // check if CRD isvc is there + WithAction(checkPreConditions). + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction()). @@ -78,9 +78,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). // must be the final action WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) if err != nil { diff --git a/controllers/components/trustyai/trustyai_controller_actions.go b/controllers/components/trustyai/trustyai_controller_actions.go index 1cb5d73a644..f33aa16e69b 100644 --- a/controllers/components/trustyai/trustyai_controller_actions.go +++ b/controllers/components/trustyai/trustyai_controller_actions.go @@ -20,37 +20,25 @@ import ( "context" "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" ) func checkPreConditions(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - t, ok := rr.Instance.(*componentApi.TrustyAI) - if !ok { - return fmt.Errorf("resource instance %v is not a componentApi.TrustyAI)", rr.Instance) + isvc, err := cluster.HasCRD(ctx, rr.Client, gvk.InferenceServices) + if err != nil { + return odherrors.NewStopError("failed to check %s CRDs version: %w", gvk.InferenceServices, err) } - if err := cluster.CustomResourceDefinitionExists(ctx, rr.Client, gvk.InferenceServices.GroupKind()); err != nil { - s := t.GetStatus() - s.Phase = status.PhaseNotReady - conditions.SetStatusCondition(t, common.Condition{ - Type: status.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: status.ISVCMissingCRDReason, - Message: status.ISVCMissingCRDMessage, - ObservedGeneration: s.ObservedGeneration, - }) - return odherrors.NewStopError("failed to find InferenceService CRD: %v", err) + if !isvc { + return odherrors.NewStopError(status.ISVCMissingCRDMessage) } + return nil } diff --git a/controllers/components/trustyai/trustyai_support.go b/controllers/components/trustyai/trustyai_support.go index 6f85425f13e..9af195d7836 100644 --- a/controllers/components/trustyai/trustyai_support.go +++ b/controllers/components/trustyai/trustyai_support.go @@ -1,8 +1,6 @@ package trustyai import ( - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" @@ -14,7 +12,7 @@ import ( const ( ComponentName = componentApi.TrustyAIComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.TrustyAIKind + status.ReadySuffix) + ReadyConditionType = componentApi.TrustyAIKind + status.ReadySuffix // LegacyComponentName is the name of the component that is assigned to deployments // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing @@ -34,6 +32,10 @@ var ( cluster.OpenDataHub: "/overlays/odh", cluster.Unknown: "/overlays/odh", } + + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + } ) func manifestsPath(p common.Platform) types.ManifestInfo { diff --git a/controllers/components/workbenches/workbenches.go b/controllers/components/workbenches/workbenches.go index 68e551dc031..9b050803671 100644 --- a/controllers/components/workbenches/workbenches.go +++ b/controllers/components/workbenches/workbenches.go @@ -1,12 +1,12 @@ package workbenches import ( + "context" "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -16,6 +16,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) @@ -73,44 +74,50 @@ func (s *componentHandler) Init(platform common.Platform) error { return nil } -func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { - c, ok := obj.(*componentApi.Workbenches) +func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { + cs := metav1.ConditionUnknown + + c := componentApi.Workbenches{} + c.Name = componentApi.WorkbenchesInstanceName + + if err := rr.Client.Get(ctx, client.ObjectKeyFromObject(&c), &c); err != nil && !k8serr.IsNotFound(err) { + return cs, nil + } + + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) if !ok { - return errors.New("failed to convert to Workbenches") + return cs, errors.New("failed to convert to DataScienceCluster") } dsc.Status.InstalledComponents[LegacyComponentName] = false dsc.Status.Components.Workbenches.ManagementSpec.ManagementState = s.GetManagementState(dsc) dsc.Status.Components.Workbenches.WorkbenchesCommonStatus = nil - nc := conditionsv1.Condition{ - Type: ReadyConditionType, - Status: corev1.ConditionFalse, - Reason: "Unknown", - Message: "Not Available", - } + rr.Conditions.MarkFalse(ReadyConditionType) switch s.GetManagementState(dsc) { case operatorv1.Managed: dsc.Status.InstalledComponents[LegacyComponentName] = true dsc.Status.Components.Workbenches.WorkbenchesCommonStatus = c.Status.WorkbenchesCommonStatus.DeepCopy() - if rc := conditions.FindStatusCondition(c, status.ConditionTypeReady); rc != nil { - nc.Status = corev1.ConditionStatus(rc.Status) - nc.Reason = rc.Reason - nc.Message = rc.Message + if rc := conditions.FindStatusCondition(c.GetStatus(), status.ConditionTypeReady); rc != nil { + rr.Conditions.MarkFrom(ReadyConditionType, *rc) + cs = rc.Status + } else { + cs = metav1.ConditionFalse } case operatorv1.Removed: - nc.Status = corev1.ConditionFalse - nc.Reason = string(operatorv1.Removed) - nc.Message = "Component ManagementState is set to " + string(operatorv1.Removed) + rr.Conditions.MarkFalse( + ReadyConditionType, + conditions.WithReason(string(operatorv1.Removed)), + conditions.WithMessage("Component ManagementState is set to %s", operatorv1.Removed), + conditions.WithSeverity(common.ConditionSeverityInfo), + ) default: - return fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) + return cs, fmt.Errorf("unknown state %s ", s.GetManagementState(dsc)) } - conditionsv1.SetStatusCondition(&dsc.Status.Conditions, nc) - - return nil + return cs, nil } diff --git a/controllers/components/workbenches/workbenches_controller.go b/controllers/components/workbenches/workbenches_controller.go index f63238893bc..e273cb877c1 100644 --- a/controllers/components/workbenches/workbenches_controller.go +++ b/controllers/components/workbenches/workbenches_controller.go @@ -31,8 +31,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" @@ -61,6 +61,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. reconciler.WithPredicates( component.ForLabel(labels.ODH.Component(LegacyComponentName), labels.True)), ). + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(devFlags). WithAction(releases.NewAction( @@ -75,9 +76,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updatestatus.NewAction()). // must be the final action WithAction(gc.NewAction()). + // declares the list of additional, controller specific conditions that are + // contributing to the controller readiness status + WithConditions(conditionTypes...). Build(ctx) if err != nil { diff --git a/controllers/components/workbenches/workbenches_support.go b/controllers/components/workbenches/workbenches_support.go index 436f7f2bebb..c6187be2c67 100644 --- a/controllers/components/workbenches/workbenches_support.go +++ b/controllers/components/workbenches/workbenches_support.go @@ -3,8 +3,6 @@ package workbenches import ( "path" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" @@ -14,7 +12,7 @@ import ( const ( ComponentName = componentApi.WorkbenchesComponentName - ReadyConditionType = conditionsv1.ConditionType(componentApi.WorkbenchesKind + status.ReadySuffix) + ReadyConditionType = componentApi.WorkbenchesKind + status.ReadySuffix notebooksPath = "notebooks" notebookImagesManifestSourcePath = "overlays/additional" @@ -37,6 +35,12 @@ var ( notebookContextDir = path.Join(ComponentName, notebooksPath) ) +var ( + conditionTypes = []string{ + status.ConditionDeploymentsAvailable, + } +) + // manifests for nbc in ODH and RHOAI + downstream use it for imageparams. func notebookControllerManifestInfo(sourcePath string) odhtypes.ManifestInfo { return odhtypes.ManifestInfo{ diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 6ba3c26c34f..c65b70adfd1 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -19,289 +19,65 @@ package datasciencecluster import ( "context" - "fmt" - "slices" - "strings" - operatorv1 "github.com/openshift/api/operator/v1" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" - corev1 "k8s.io/api/core/v1" - k8serr "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/record" + "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" - 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/handlers" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/dependent" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/reconciler" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" ) -// DataScienceClusterReconciler reconciles a DataScienceCluster object. -type DataScienceClusterReconciler struct { - *odhClient.Client - Scheme *runtime.Scheme - // Recorder to generate events - Recorder record.EventRecorder -} - -const ( - finalizerName = "datasciencecluster.opendatahub.io/finalizer" - fieldOwner = "datasciencecluster.opendatahub.io" -) - -// TODO: all the logic about the deletion configmap should be moved to another controller -// https://issues.redhat.com/browse/RHOAIENG-16674 - -// Reconcile is part of the main kubernetes reconciliation loop which aims to -// move the current state of the cluster closer to the desired state. -func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - log := logf.FromContext(ctx).WithName("DataScienceCluster") - log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name) - instance := &dscv1.DataScienceCluster{} - err := r.Client.Get(ctx, req.NamespacedName, instance) - - switch { - case k8serr.IsNotFound(err): - return ctrl.Result{}, nil - case err != nil: - return ctrl.Result{}, err - } - - if controllerutil.RemoveFinalizer(instance, finalizerName) { - if err := r.Client.Update(ctx, instance); err != nil { - return ctrl.Result{}, err - } - } - - if !instance.ObjectMeta.DeletionTimestamp.IsZero() { - log.Info("Finalization DataScienceCluster start deleting instance", "name", instance.Name) - return ctrl.Result{}, nil - } - - // validate pre-requisites - if err := r.validate(ctx, instance); err != nil { - log.Info(err.Error()) - status.SetCondition(&instance.Status.Conditions, "Degraded", status.ReconcileFailed, err.Error(), corev1.ConditionTrue) - } - - // deploy components - if err := r.reconcileComponents(ctx, instance, cr.DefaultRegistry()); err != nil { - log.Info(err.Error()) - status.SetCondition(&instance.Status.Conditions, "Degraded", status.ReconcileFailed, err.Error(), corev1.ConditionTrue) - } - - // keep conditions sorted - slices.SortFunc(instance.Status.Conditions, func(a, b conditionsv1.Condition) int { - return strings.Compare(string(a.Type), string(b.Type)) - }) - - err = r.Client.ApplyStatus(ctx, instance, client.FieldOwner(fieldOwner), client.ForceOwnership) - switch { - case err == nil: - return ctrl.Result{}, nil - case k8serr.IsNotFound(err): - return ctrl.Result{}, nil - default: - r.reportError(ctx, err, instance, "failed to update DataScienceCluster status") - return ctrl.Result{}, err - } -} - -func (r *DataScienceClusterReconciler) validate(ctx context.Context, _ *dscv1.DataScienceCluster) error { - // This case should not happen, since there is a webhook that blocks the creation - // of more than one instance of the DataScienceCluster, however one can create a - // DataScienceCluster instance while the operator is stopped, hence this extra check - - if _, err := cluster.GetDSCI(ctx, r.Client); err != nil { - return fmt.Errorf("failed to get a valid DataScienceCluster instance, %w", err) - } - - if _, err := cluster.GetDSC(ctx, r.Client); err != nil { - return fmt.Errorf("failed to get a valid DSCInitialization instance, %w", err) - } - - return nil -} - -func (r *DataScienceClusterReconciler) reconcileComponents( - ctx context.Context, - instance *dscv1.DataScienceCluster, - reg *cr.Registry) error { - log := logf.FromContext(ctx).WithName("DataScienceCluster") - - notReadyComponents := make([]string, 0) - - // all DSC defined components - componentErrors := reg.ForEach(func(component cr.ComponentHandler) error { - ci, err := r.reconcileComponent(ctx, instance, component) - if err != nil { - return err - } - - if !cr.IsManaged(component, instance) { - return nil - } - - if !conditions.IsStatusConditionTrue(ci, status.ConditionTypeReady) { - notReadyComponents = append(notReadyComponents, component.GetName()) - } - - return nil - }) - - // Process errors for components - if componentErrors != nil { - log.Info("DataScienceCluster Deployment Incomplete.") - - status.SetCompleteCondition( - &instance.Status.Conditions, - status.ReconcileCompletedWithComponentErrors, - fmt.Sprintf("DataScienceCluster resource reconciled with component errors: %v", componentErrors), - ) - - r.Recorder.Eventf(instance, corev1.EventTypeNormal, - "DataScienceClusterComponentFailures", - "DataScienceCluster instance %s created, but have some failures in component %v", instance.Name, componentErrors) - } else { - log.Info("DataScienceCluster Deployment Completed.") - - // finalize reconciliation - status.SetCompleteCondition( - &instance.Status.Conditions, - status.ReconcileCompleted, - "DataScienceCluster resource reconciled successfully", - ) - } - - if len(notReadyComponents) != 0 { - instance.Status.Phase = status.PhaseNotReady - - conditionsv1.SetStatusCondition(&instance.Status.Conditions, conditionsv1.Condition{ - Type: conditionsv1.ConditionType(status.ConditionTypeReady), - Status: corev1.ConditionFalse, - Reason: "NotReady", - Message: fmt.Sprintf("Some components are not ready: %s", strings.Join(notReadyComponents, ",")), - }) - } else { - instance.Status.Phase = status.PhaseReady - - conditionsv1.SetStatusCondition(&instance.Status.Conditions, conditionsv1.Condition{ - Type: conditionsv1.ConditionType(status.ConditionTypeReady), - Status: corev1.ConditionTrue, - Reason: "Ready", - Message: "Ready", - }) - } - - instance.Status.Release = cluster.GetRelease() - instance.Status.ObservedGeneration = instance.Generation - - if componentErrors != nil { - return componentErrors - } - - return nil -} - -func (r *DataScienceClusterReconciler) reconcileComponent( - ctx context.Context, - instance *dscv1.DataScienceCluster, - component cr.ComponentHandler, -) (common.PlatformObject, error) { - ms := component.GetManagementState(instance) - componentCR := component.NewCRObject(instance) - - switch ms { - case operatorv1.Managed: - err := ctrl.SetControllerReference(instance, componentCR, r.Scheme) - if err != nil { - return nil, err - } - err = r.Client.Apply(ctx, componentCR, client.FieldOwner(fieldOwner), client.ForceOwnership) - if err != nil { - return nil, err - } - case operatorv1.Removed: - err := r.Client.Delete(ctx, componentCR, client.PropagationPolicy(metav1.DeletePropagationForeground)) - if err != nil && !k8serr.IsNotFound(err) { - return nil, err - } - default: - return nil, fmt.Errorf("unsupported management state: %s", ms) - } - - if instance.Status.InstalledComponents == nil { - instance.Status.InstalledComponents = make(map[string]bool) - } - - err := component.UpdateDSCStatus(instance, componentCR) - if err != nil { - return nil, fmt.Errorf("failed to update status of DataScienceCluster component %s: %w", component.GetName(), err) - } - - return componentCR, nil -} - -func (r *DataScienceClusterReconciler) reportError(ctx context.Context, err error, instance *dscv1.DataScienceCluster, message string) { - logf.FromContext(ctx).Error(err, message, "instance.Name", instance.Name) - r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DataScienceClusterReconcileError", - "%s for instance %s", message, instance.Name) -} - -// SetupWithManager sets up the controller with the Manager. -func (r *DataScienceClusterReconciler) SetupWithManager(_ context.Context, mgr ctrl.Manager) error { +func NewDataScienceClusterReconciler(ctx context.Context, mgr ctrl.Manager) error { componentsPredicate := dependent.New(dependent.WithWatchStatus(true)) - return ctrl.NewControllerManagedBy(mgr). - For(&dscv1.DataScienceCluster{}, builder.WithPredicates(predicates.DefaultPredicate)). - // components - Owns(&componentApi.Dashboard{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.Workbenches{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.Ray{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.ModelRegistry{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.TrustyAI{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.Kueue{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.CodeFlare{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.TrainingOperator{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.DataSciencePipelines{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.Kserve{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.ModelMeshServing{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.ModelController{}, builder.WithPredicates(componentsPredicate)). - Owns(&componentApi.FeastOperator{}, builder.WithPredicates(componentsPredicate)). - // others + _, err := reconciler.ReconcilerFor(mgr, &dscv1.DataScienceCluster{}). + Owns(&componentApi.Dashboard{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.Workbenches{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.Ray{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.ModelRegistry{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.TrustyAI{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.Kueue{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.CodeFlare{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.TrainingOperator{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.DataSciencePipelines{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.Kserve{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.ModelMeshServing{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.ModelController{}, reconciler.WithPredicates(componentsPredicate)). + Owns(&componentApi.FeastOperator{}, reconciler.WithPredicates(componentsPredicate)). Watches( &dsciv1.DSCInitialization{}, - handlers.Fn(r.watchDataScienceClusters)). - Complete(r) -} + reconciler.WithEventMapper(func(ctx context.Context, _ client.Object) []reconcile.Request { + return watchDataScienceClusters(ctx, mgr.GetClient()) + })). + WithAction(initialize). + WithAction(checkPreConditions). + WithAction(updateStatus). + WithAction(provisionComponents). + WithAction(deploy.NewAction( + deploy.WithCache()), + ). + WithAction(gc.NewAction( + gc.WithTypePredicate( + func(rr *types.ReconciliationRequest, objGVK schema.GroupVersionKind) (bool, error) { + return rr.Manager.Owns(objGVK), nil + }, + ), + )). + WithConditions(status.ConditionTypeProvisioningSucceeded, status.ConditionTypeComponentsReady). + Build(ctx) -func (r *DataScienceClusterReconciler) watchDataScienceClusters(ctx context.Context, _ client.Object) []reconcile.Request { - instanceList := &dscv1.DataScienceClusterList{} - err := r.Client.List(ctx, instanceList) if err != nil { - return nil - } - - requests := make([]reconcile.Request, len(instanceList.Items)) - for i := range instanceList.Items { - requests[i] = reconcile.Request{NamespacedName: types.NamespacedName{Name: instanceList.Items[i].Name}} + return err } - return requests + return nil } diff --git a/controllers/datasciencecluster/datasciencecluster_controller_actions.go b/controllers/datasciencecluster/datasciencecluster_controller_actions.go new file mode 100644 index 00000000000..03eb77cc9e5 --- /dev/null +++ b/controllers/datasciencecluster/datasciencecluster_controller_actions.go @@ -0,0 +1,125 @@ +package datasciencecluster + +import ( + "context" + "fmt" + + operatorv1 "github.com/openshift/api/operator/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" + odhtype "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" +) + +const ( + //TODO: remove after https://issues.redhat.com/browse/RHOAIENG-15920 + finalizerName = "datasciencecluster.opendatahub.io/finalizer" +) + +func initialize(ctx context.Context, rr *odhtype.ReconciliationRequest) error { + instance, ok := rr.Instance.(*dscv1.DataScienceCluster) + if !ok { + return fmt.Errorf("resource instance %v is not a dscv1.DataScienceCluster)", rr.Instance) + } + + if instance.Status.InstalledComponents == nil { + instance.Status.InstalledComponents = make(map[string]bool) + } + + //TODO: remove after https://issues.redhat.com/browse/RHOAIENG-15920 + if controllerutil.RemoveFinalizer(instance, finalizerName) { + if err := rr.Client.Update(ctx, instance); err != nil { + return err + } + } + + return nil +} + +func checkPreConditions(ctx context.Context, rr *odhtype.ReconciliationRequest) error { + // This case should not happen, since there is a webhook that blocks the creation + // of more than one instance of the DataScienceCluster, however one can create a + // DataScienceCluster instance while the operator is stopped, hence this extra check + + if _, err := cluster.GetDSCI(ctx, rr.Client); err != nil { + return fmt.Errorf("failed to get a valid DataScienceCluster instance, %w", err) + } + + if _, err := cluster.GetDSC(ctx, rr.Client); err != nil { + return fmt.Errorf("failed to get a valid DSCInitialization instance, %w", err) + } + + return nil +} + +func watchDataScienceClusters(ctx context.Context, cli client.Client) []reconcile.Request { + instanceList := &dscv1.DataScienceClusterList{} + err := cli.List(ctx, instanceList) + if err != nil { + return nil + } + + requests := make([]reconcile.Request, len(instanceList.Items)) + for i := range instanceList.Items { + requests[i] = reconcile.Request{NamespacedName: types.NamespacedName{Name: instanceList.Items[i].Name}} + } + + return requests +} + +func provisionComponents(_ context.Context, rr *odhtype.ReconciliationRequest) error { + instance, ok := rr.Instance.(*dscv1.DataScienceCluster) + if !ok { + return fmt.Errorf("resource instance %v is not a dscv1.DataScienceCluster)", rr.Instance) + } + + if instance.Status.InstalledComponents == nil { + instance.Status.InstalledComponents = make(map[string]bool) + } + + // force gc to run + rr.Generated = true + + err := cr.ForEach(func(component cr.ComponentHandler) error { + ms := component.GetManagementState(instance) + if ms != operatorv1.Managed { + return nil + } + + ci := component.NewCRObject(instance) + if err := rr.AddResources(ci); err != nil { + return err + } + + return nil + }) + + if err != nil { + return err + } + + return nil +} + +func updateStatus(ctx context.Context, rr *odhtype.ReconciliationRequest) error { + instance, ok := rr.Instance.(*dscv1.DataScienceCluster) + if !ok { + return fmt.Errorf("resource instance %v is not a dscv1.DataScienceCluster)", rr.Instance) + } + + if instance.Status.InstalledComponents == nil { + instance.Status.InstalledComponents = make(map[string]bool) + } + + err := computeComponentsStatus(ctx, rr, cr.DefaultRegistry()) + if err != nil { + return err + } + + return nil +} diff --git a/controllers/datasciencecluster/datasciencecluster_controller_support.go b/controllers/datasciencecluster/datasciencecluster_controller_support.go new file mode 100644 index 00000000000..bd0e8d2d7c1 --- /dev/null +++ b/controllers/datasciencecluster/datasciencecluster_controller_support.go @@ -0,0 +1,86 @@ +package datasciencecluster + +import ( + "context" + "errors" + "fmt" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" + cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" +) + +// computeComponentsStatus checks the status of all registered components in a DataScienceCluster instance +// and updates the status condition accordingly. +// +// Parameters: +// - ctx: The context for managing request deadlines and cancellation. +// - instance: The DataScienceCluster instance being reconciled. +// - reg: The registry containing all component handlers. +// +// Returns: +// - error: An error if any component status retrieval or update fails. +func computeComponentsStatus( + ctx context.Context, + 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 + + err := reg.ForEach(func(component cr.ComponentHandler) error { + cs, err := component.UpdateDSCStatus(ctx, rr) + if err != nil { + notReadyComponents = append(notReadyComponents, component.GetName()) + return err + } + + if !cr.IsManaged(component, instance) { + return nil + } + + managedComponent++ + + if cs == metav1.ConditionFalse { + notReadyComponents = append(notReadyComponents, component.GetName()) + } + + return nil + }) + + switch { + case len(notReadyComponents) > 0: + 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: + rr.Conditions.SetCondition(common.Condition{ + Type: status.ConditionTypeComponentsReady, + Status: metav1.ConditionTrue, + Severity: common.ConditionSeverityInfo, + Reason: status.NoManagedComponentsReason, + Message: status.NoManagedComponentsReason, + }) + default: + rr.Conditions.MarkTrue(status.ConditionTypeComponentsReady) + } + + if err != nil { + return err + } + + return nil +} diff --git a/controllers/services/auth/auth_controller.go b/controllers/services/auth/auth_controller.go index d1fda9aa4db..219ff623e2f 100644 --- a/controllers/services/auth/auth_controller.go +++ b/controllers/services/auth/auth_controller.go @@ -46,7 +46,6 @@ func NewServiceReconciler(ctx context.Context, mgr ctrl.Manager) error { WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(setStatus). Build(ctx) if err != nil { diff --git a/controllers/services/auth/auth_controller_actions.go b/controllers/services/auth/auth_controller_actions.go index 47fbd102741..228c84d32a9 100644 --- a/controllers/services/auth/auth_controller_actions.go +++ b/controllers/services/auth/auth_controller_actions.go @@ -143,14 +143,3 @@ func managePermissions(ctx context.Context, rr *odhtypes.ReconciliationRequest) return nil } - -func setStatus(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - ai, ok := rr.Instance.(*serviceApi.Auth) - if !ok { - return errors.New("instance is not of type *services.Auth") - } - - ai.Status.Phase = "Ready" - ai.Status.ObservedGeneration = ai.GetObjectMeta().GetGeneration() - return nil -} diff --git a/controllers/services/monitoring/monitoring.go b/controllers/services/monitoring/monitoring.go index 0b7e9772be1..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, status.ConditionTypeReady), nil + return conditions.IsStatusConditionTrue(obj.GetStatus(), status.ConditionTypeReady), nil } } diff --git a/controllers/services/monitoring/monitoring_controller.go b/controllers/services/monitoring/monitoring_controller.go index c18106b6063..872231c9bad 100644 --- a/controllers/services/monitoring/monitoring_controller.go +++ b/controllers/services/monitoring/monitoring_controller.go @@ -18,6 +18,7 @@ package monitoring import ( "context" + "errors" "fmt" ctrl "sigs.k8s.io/controller-runtime" @@ -25,9 +26,11 @@ import ( dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" serviceApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/services/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/reconciler" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" ) // NewServiceReconciler creates a ServiceReconciler for the Monitoring API. @@ -45,12 +48,22 @@ func NewServiceReconciler(ctx context.Context, mgr ctrl.Manager) error { Watches(&dscv1.DataScienceCluster{}, reconciler.WithEventHandler(handlers.ToNamed(serviceApi.MonitoringInstanceName)), reconciler.WithPredicates(resources.DSCComponentUpdatePredicate)). // actions + WithAction(deployments.NewAction( + deployments.InNamespaceFn(func(_ context.Context, rr *types.ReconciliationRequest) (string, error) { + m, ok := rr.Instance.(*serviceApi.Monitoring) + if !ok { + return "", errors.New("instance is not of type *services.Monitoring") + } + + return m.Spec.Namespace, nil + }), + )). + WithAction(deployments.NewAction()). WithAction(initialize). WithAction(updatePrometheusConfigMap). WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(updateStatus). Build(ctx) if err != nil { diff --git a/controllers/services/monitoring/monitoring_controller_actions.go b/controllers/services/monitoring/monitoring_controller_actions.go index 2154ad12a5f..8100753089e 100644 --- a/controllers/services/monitoring/monitoring_controller_actions.go +++ b/controllers/services/monitoring/monitoring_controller_actions.go @@ -2,21 +2,13 @@ package monitoring import ( "context" - "errors" "fmt" operatorv1 "github.com/openshift/api/operator/v1" - appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" - serviceApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/services/v1alpha1" - "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" ) @@ -80,53 +72,3 @@ func updatePrometheusConfigMap(ctx context.Context, rr *odhtypes.ReconciliationR } }) } - -func updateStatus(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - m, ok := rr.Instance.(*serviceApi.Monitoring) - if !ok { - return errors.New("instance is not of type *services.Monitoring") - } - - // TODO: deprecate phase - // Cannot use status.PhaseNotReady as the value here is not the - // same as the constant ("Not Ready") - m.Status.Phase = "NotReady" - - // condition - nc := common.Condition{ - Type: status.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: "NotReady", - Message: "Prometheus deployment is not ready", - } - - promDeployment := &appsv1.DeploymentList{} - err := rr.Client.List( - ctx, - promDeployment, - client.InNamespace(m.Spec.Namespace), - ) - if err != nil { - return fmt.Errorf("error fetching promethus deployments: %w", err) - } - - ready := 0 - for _, deployment := range promDeployment.Items { - if deployment.Status.ReadyReplicas == deployment.Status.Replicas { - ready++ - } - } - - if len(promDeployment.Items) == ready { - // TODO: deprecate phase - m.Status.Phase = status.PhaseReady - // condition - nc.Status = metav1.ConditionTrue - nc.Reason = status.ReconcileCompleted - nc.Message = status.ReconcileCompletedMessage - } - conditions.SetStatusCondition(&m.Status, nc) - m.Status.ObservedGeneration = m.GetObjectMeta().GetGeneration() - - return nil -} diff --git a/docs/api-overview.md b/docs/api-overview.md index a4b96031089..3166f6240f0 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -2130,9 +2130,9 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `phase` _string_ | Phase describes the Phase of DataScienceCluster reconciliation state
This is used by OLM UI to provide status information to the user | | | -| `conditions` _Condition array_ | Conditions describes the state of the DataScienceCluster resource. | | | -| `observedGeneration` _integer_ | The generation observed by the deployment controller. | | | +| `phase` _string_ | | | | +| `observedGeneration` _integer_ | The generation observed by the resource controller. | | | +| `conditions` _[Condition](#condition) array_ | | | | | `relatedObjects` _[ObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#objectreference-v1-core) array_ | RelatedObjects is a list of objects created and maintained by this operator.
Object references will be added to this list after they have been created AND found in the cluster. | | | | `errorMessage` _string_ | | | | | `installedComponents` _object (keys:string, values:boolean)_ | List of components with status if installed or not | | | diff --git a/main.go b/main.go index 81445e5e5ed..b5512fb7f04 100644 --- a/main.go +++ b/main.go @@ -315,6 +315,18 @@ func main() { //nolint:funlen,maintidx,gocyclo os.Exit(1) } + ons, err := cluster.GetOperatorNamespace() + if err != nil { + setupLog.Error(err, "unable to determine Operator Namespace") + os.Exit(1) + } + + gc.Instance = gc.New( + oc, + ons, + gc.WithUnremovables(gvk.CustomResourceDefinition, gvk.Lease), + ) + if err = (&dscictrl.DSCInitializationReconciler{ Client: oc, Scheme: mgr.GetScheme(), @@ -324,11 +336,7 @@ func main() { //nolint:funlen,maintidx,gocyclo os.Exit(1) } - if err = (&dscctrl.DataScienceClusterReconciler{ - Client: oc, - Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("datasciencecluster-controller"), - }).SetupWithManager(ctx, mgr); err != nil { + if err = dscctrl.NewDataScienceClusterReconciler(ctx, mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "DataScienceCluster") os.Exit(1) } @@ -356,18 +364,6 @@ func main() { //nolint:funlen,maintidx,gocyclo os.Exit(1) } - ons, err := cluster.GetOperatorNamespace() - if err != nil { - setupLog.Error(err, "unable to determine Operator Namespace") - os.Exit(1) - } - - gc.Instance = gc.New( - oc, - ons, - gc.WithUnremovables(gvk.CustomResourceDefinition, gvk.Lease), - ) - err = mgr.Add(gc.Instance) if err != nil { setupLog.Error(err, "unable to register GC service") diff --git a/pkg/componentsregistry/componentsregistry.go b/pkg/componentsregistry/componentsregistry.go index b438fc3eb5b..dc9b3ef14cd 100644 --- a/pkg/componentsregistry/componentsregistry.go +++ b/pkg/componentsregistry/componentsregistry.go @@ -7,11 +7,12 @@ import ( "github.com/hashicorp/go-multierror" operatorv1 "github.com/openshift/api/operator/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" ) // ComponentHandler is an interface to manage a component @@ -28,7 +29,7 @@ type ComponentHandler interface { NewCRObject(dsc *dscv1.DataScienceCluster) common.PlatformObject NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error // UpdateDSCStatus updates the component specific status part of the DSC - UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error + UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) } // Registry is a struct that maintains a list of registered ComponentHandlers. diff --git a/pkg/controller/actions/actions.go b/pkg/controller/actions/actions.go index 2d469f9085c..a637bfefba3 100644 --- a/pkg/controller/actions/actions.go +++ b/pkg/controller/actions/actions.go @@ -18,6 +18,9 @@ const ( type Fn func(ctx context.Context, rr *types.ReconciliationRequest) error +// TODO replace with type alias in GO 1.24. +type StringGetter func(context.Context, *types.ReconciliationRequest) (string, error) + func (f Fn) String() string { fn := runtime.FuncForPC(reflect.ValueOf(f).Pointer()) return fn.Name() diff --git a/pkg/controller/actions/deploy/action_deploy.go b/pkg/controller/actions/deploy/action_deploy.go index a8614f0fa4c..59155ae4aa6 100644 --- a/pkg/controller/actions/deploy/action_deploy.go +++ b/pkg/controller/actions/deploy/action_deploy.go @@ -118,6 +118,7 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er } controllerName := strings.ToLower(kind) + igvk := rr.Instance.GetObjectKind().GroupVersionKind() for i := range rr.Resources { res := rr.Resources[i] @@ -132,9 +133,9 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er case lookupErr != nil: return fmt.Errorf("failed to lookup object %s/%s: %w", res.GetNamespace(), res.GetName(), lookupErr) default: - // Remove the DSC and DSCI owner reference if set, This is required during the + // Remove the previous owner reference if set, This is required during the // transition from the old to the new operator. - if err := resources.RemoveOwnerReferences(ctx, rr.Client, current, isLegacyOwnerRef); err != nil { + if err := resources.RemoveOwnerReferences(ctx, rr.Client, current, ownedTypeIsNot(&igvk)); err != nil { return err } diff --git a/pkg/controller/actions/deploy/action_deploy_support.go b/pkg/controller/actions/deploy/action_deploy_support.go index 433b5f6473a..d1cf70c4b02 100644 --- a/pkg/controller/actions/deploy/action_deploy_support.go +++ b/pkg/controller/actions/deploy/action_deploy_support.go @@ -2,6 +2,7 @@ package deploy import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" ) @@ -16,3 +17,16 @@ func isLegacyOwnerRef(or metav1.OwnerReference) bool { return false } } + +func ownedTypeIsNot(ownerType *schema.GroupVersionKind) func(or metav1.OwnerReference) bool { + if ownerType == nil { + return func(or metav1.OwnerReference) bool { + return false + } + } + + gv := ownerType.GroupVersion().String() + return func(or metav1.OwnerReference) bool { + return ownerType.Kind != or.Kind && gv != or.APIVersion + } +} diff --git a/pkg/controller/actions/updatestatus/action_update_status.go b/pkg/controller/actions/status/deployments/action_deployments_available.go similarity index 64% rename from pkg/controller/actions/updatestatus/action_update_status.go rename to pkg/controller/actions/status/deployments/action_deployments_available.go index 6de3711f5ee..16d985fb6ba 100644 --- a/pkg/controller/actions/updatestatus/action_update_status.go +++ b/pkg/controller/actions/status/deployments/action_deployments_available.go @@ -1,4 +1,4 @@ -package updatestatus +package deployments import ( "context" @@ -6,10 +6,8 @@ import ( "strings" appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" @@ -18,13 +16,9 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" ) -const ( - DeploymentsNotReadyReason = "DeploymentsNotReady" - ReadyReason = "Ready" -) - type Action struct { - labels map[string]string + labels map[string]string + namespaceFn actions.StringGetter } type ActionOpts func(*Action) @@ -43,6 +37,23 @@ func WithSelectorLabels(values map[string]string) ActionOpts { } } +func InNamespace(ns string) ActionOpts { + return func(action *Action) { + action.namespaceFn = func(_ context.Context, _ *types.ReconciliationRequest) (string, error) { + return ns, nil + } + } +} + +func InNamespaceFn(fn actions.StringGetter) ActionOpts { + return func(action *Action) { + if fn == nil { + return + } + action.namespaceFn = fn + } +} + func (a *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error { l := make(map[string]string, len(a.labels)) for k, v := range a.labels { @@ -65,10 +76,15 @@ func (a *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error deployments := &appsv1.DeploymentList{} - err := rr.Client.List( + ns, err := a.namespaceFn(ctx, rr) + if err != nil { + return fmt.Errorf("unable to compute namespace: %w", err) + } + + err = rr.Client.List( ctx, deployments, - client.InNamespace(rr.DSCI.Spec.ApplicationsNamespace), + client.InNamespace(ns), client.MatchingLabels(l), ) @@ -84,32 +100,27 @@ func (a *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error } s := obj.GetStatus() - s.ObservedGeneration = obj.GetGeneration() - s.Phase = "Ready" - - conditionReady := common.Condition{ - Type: status.ConditionTypeReady, - Status: metav1.ConditionTrue, - Reason: ReadyReason, - Message: fmt.Sprintf("%d/%d deployments ready", ready, len(deployments.Items)), - ObservedGeneration: s.ObservedGeneration, - } - if len(deployments.Items) == 0 || (len(deployments.Items) > 0 && ready != len(deployments.Items)) { - conditionReady.Status = metav1.ConditionFalse - conditionReady.Reason = DeploymentsNotReadyReason + rr.Conditions.MarkTrue(status.ConditionDeploymentsAvailable, conditions.WithObservedGeneration(s.ObservedGeneration)) - s.Phase = "NotReady" + if len(deployments.Items) == 0 || (len(deployments.Items) > 0 && ready != len(deployments.Items)) { + rr.Conditions.MarkFalse( + status.ConditionDeploymentsAvailable, + conditions.WithObservedGeneration(s.ObservedGeneration), + conditions.WithReason(status.ConditionDeploymentsNotAvailableReason), + conditions.WithMessage("%d/%d deployments ready", ready, len(deployments.Items)), + ) } - conditions.SetStatusCondition(s, conditionReady) - return nil } func NewAction(opts ...ActionOpts) actions.Fn { action := Action{ labels: map[string]string{}, + namespaceFn: func(ctx context.Context, rr *types.ReconciliationRequest) (string, error) { + return rr.DSCI.Spec.ApplicationsNamespace, nil + }, } for _, opt := range opts { diff --git a/pkg/controller/actions/updatestatus/action_update_status_test.go b/pkg/controller/actions/status/deployments/action_deployments_available_test.go similarity index 77% rename from pkg/controller/actions/updatestatus/action_update_status_test.go rename to pkg/controller/actions/status/deployments/action_deployments_available_test.go index e52c47a6011..6a8a97e7cc3 100644 --- a/pkg/controller/actions/updatestatus/action_update_status_test.go +++ b/pkg/controller/actions/status/deployments/action_deployments_available_test.go @@ -1,5 +1,4 @@ -//nolint:dupl -package updatestatus_test +package deployments_test import ( "context" @@ -17,7 +16,8 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/deployments" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" @@ -26,8 +26,7 @@ import ( . "github.com/onsi/gomega" ) -//nolint:dupl -func TestUpdateStatusActionNotReady(t *testing.T) { +func TestDeploymentsAvailableActionNotReady(t *testing.T) { g := NewWithT(t) ctx := context.Background() @@ -72,8 +71,8 @@ func TestUpdateStatusActionNotReady(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) - action := updatestatus.NewAction( - updatestatus.WithSelectorLabel(labels.PlatformPartOf, ns)) + action := deployments.NewAction( + deployments.WithSelectorLabel(labels.PlatformPartOf, ns)) rr := types.ReconciliationRequest{ Client: cl, @@ -82,6 +81,8 @@ func TestUpdateStatusActionNotReady(t *testing.T) { Release: common.Release{Name: cluster.OpenDataHub}, } + rr.Conditions = conditions.NewManager(rr.Instance, status.ConditionTypeReady) + err = action(ctx, &rr) g.Expect(err).ShouldNot(HaveOccurred()) @@ -91,13 +92,21 @@ func TestUpdateStatusActionNotReady(t *testing.T) { matchers.ExtractStatusCondition(status.ConditionTypeReady), gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(updatestatus.DeploymentsNotReadyReason), + }), + ), + ) + g.Expect(rr.Instance).Should( + WithTransform( + matchers.ExtractStatusCondition(status.ConditionDeploymentsAvailable), + gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal(status.ConditionDeploymentsNotAvailableReason), }), ), ) } -func TestUpdateStatusActionReady(t *testing.T) { +func TestDeploymentsAvailableActionReady(t *testing.T) { g := NewWithT(t) ctx := context.Background() @@ -142,8 +151,8 @@ func TestUpdateStatusActionReady(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) - action := updatestatus.NewAction( - updatestatus.WithSelectorLabel(labels.PlatformPartOf, ns)) + action := deployments.NewAction( + deployments.WithSelectorLabel(labels.PlatformPartOf, ns)) rr := types.ReconciliationRequest{ Client: cl, @@ -152,6 +161,8 @@ func TestUpdateStatusActionReady(t *testing.T) { Release: common.Release{Name: cluster.OpenDataHub}, } + rr.Conditions = conditions.NewManager(rr.Instance, status.ConditionTypeReady) + err = action(ctx, &rr) g.Expect(err).ShouldNot(HaveOccurred()) @@ -161,13 +172,20 @@ func TestUpdateStatusActionReady(t *testing.T) { matchers.ExtractStatusCondition(status.ConditionTypeReady), gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(updatestatus.ReadyReason), + }), + ), + ) + g.Expect(rr.Instance).Should( + WithTransform( + matchers.ExtractStatusCondition(status.ConditionDeploymentsAvailable), + gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ + "Status": Equal(metav1.ConditionTrue), }), ), ) } -func TestUpdateStatusActionReadyAutoSelector(t *testing.T) { +func TestDeploymentsAvailableReadyAutoSelector(t *testing.T) { g := NewWithT(t) ctx := context.Background() @@ -212,7 +230,7 @@ func TestUpdateStatusActionReadyAutoSelector(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) - action := updatestatus.NewAction() + action := deployments.NewAction() rr := types.ReconciliationRequest{ Client: cl, @@ -221,6 +239,8 @@ func TestUpdateStatusActionReadyAutoSelector(t *testing.T) { Release: common.Release{Name: cluster.OpenDataHub}, } + rr.Conditions = conditions.NewManager(rr.Instance, status.ConditionTypeReady) + err = action(ctx, &rr) g.Expect(err).ShouldNot(HaveOccurred()) @@ -230,13 +250,20 @@ func TestUpdateStatusActionReadyAutoSelector(t *testing.T) { matchers.ExtractStatusCondition(status.ConditionTypeReady), gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(updatestatus.ReadyReason), + }), + ), + ) + g.Expect(rr.Instance).Should( + WithTransform( + matchers.ExtractStatusCondition(status.ConditionDeploymentsAvailable), + gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ + "Status": Equal(metav1.ConditionTrue), }), ), ) } -func TestUpdateStatusActionNotReadyNotFound(t *testing.T) { +func TestDeploymentsAvailableActionNotReadyNotFound(t *testing.T) { g := NewWithT(t) ctx := context.Background() @@ -281,7 +308,7 @@ func TestUpdateStatusActionNotReadyNotFound(t *testing.T) { g.Expect(err).ShouldNot(HaveOccurred()) - action := updatestatus.NewAction() + action := deployments.NewAction() rr := types.ReconciliationRequest{ Client: cl, @@ -290,6 +317,8 @@ func TestUpdateStatusActionNotReadyNotFound(t *testing.T) { Release: common.Release{Name: cluster.OpenDataHub}, } + rr.Conditions = conditions.NewManager(rr.Instance, status.ConditionTypeReady) + err = action(ctx, &rr) g.Expect(err).ShouldNot(HaveOccurred()) @@ -299,7 +328,15 @@ func TestUpdateStatusActionNotReadyNotFound(t *testing.T) { matchers.ExtractStatusCondition(status.ConditionTypeReady), gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(updatestatus.DeploymentsNotReadyReason), + }), + ), + ) + g.Expect(rr.Instance).Should( + WithTransform( + matchers.ExtractStatusCondition(status.ConditionDeploymentsAvailable), + gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal(status.ConditionDeploymentsNotAvailableReason), }), ), ) diff --git a/pkg/controller/conditions/conditions.go b/pkg/controller/conditions/conditions.go new file mode 100644 index 00000000000..7e2061fd83e --- /dev/null +++ b/pkg/controller/conditions/conditions.go @@ -0,0 +1,345 @@ +// inspired by https://github.com/knative/pkg/blob/main/apis/condition_set.go + +package conditions + +import ( + "cmp" + "fmt" + "slices" + "sort" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" +) + +type Option func(*common.Condition) + +func WithReason(value string) Option { + return func(c *common.Condition) { + c.Reason = value + } +} + +func WithMessage(msg string, opts ...any) Option { + value := msg + if len(opts) != 0 { + value = fmt.Sprintf(msg, opts...) + } + + return func(c *common.Condition) { + c.Message = value + } +} + +func WithObservedGeneration(value int64) Option { + return func(c *common.Condition) { + c.ObservedGeneration = value + } +} + +func WithSeverity(value common.ConditionSeverity) Option { + return func(c *common.Condition) { + c.Severity = value + } +} + +func WithError(err error) Option { + return func(c *common.Condition) { + c.Severity = common.ConditionSeverityError + c.Reason = common.ConditionReasonError + c.Message = err.Error() + } +} + +type Manager struct { + happy string + dependents []string + accessor common.ConditionsAccessor +} + +func NewManager(accessor common.ConditionsAccessor, happy string, dependents ...string) *Manager { + deps := make([]string, 0, len(dependents)) + for _, d := range dependents { + if d == happy || slices.Contains(deps, d) { + continue + } + deps = append(deps, d) + } + + m := Manager{ + accessor: accessor, + happy: happy, + dependents: deps, + } + + m.initializeConditions() + + return &m +} + +// initializeConditions ensures that the conditions for the manager and its dependents are properly +// initialized. Specifically, it initializes the "happy" condition and sets the initial status for +// each dependent condition. +// +// The method performs the following: +// 1. Retrieves the "happy" condition using. If it does not exist, it creates a new one with +// `ConditionUnknown` status and sets it. +// 2. Sets the status of each dependent condition based on the "happy" condition's status. If the +// "happy" condition's status is `True`, all dependent conditions are set to `True`, otherwise +// to `Unknown`. +func (r *Manager) initializeConditions() { + happy := r.GetCondition(r.happy) + if happy == nil { + happy = &common.Condition{ + Type: r.happy, + Status: metav1.ConditionUnknown, + } + r.SetCondition(*happy) + } + + status := metav1.ConditionUnknown + if happy.Status == metav1.ConditionTrue { + status = metav1.ConditionTrue + } + + for _, t := range r.dependents { + if c := r.GetCondition(t); c != nil { + continue + } + + r.SetCondition(common.Condition{ + Type: t, + Status: status, + }) + } +} + +func (r *Manager) IsHappy() bool { + if r.accessor == nil { + return false + } + + 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, t) +} + +// SetCondition sets the given condition on the manager. It updates the list of conditions and +// sorts them alphabetically. After updating, it recomputes the happiness state based on the +// provided condition type. +// +// Parameters: +// - `cond`: The condition to set. The condition will be added or updated based on its type. +func (r *Manager) SetCondition(cond common.Condition) { + if r.accessor == nil { + return + } + + if !SetStatusCondition(r.accessor, cond) { + return + } + + r.RecomputeHappiness(cond.Type) +} + +// ClearCondition removes the specified condition type from the manager's list of conditions +// and recomputes happiness. +// +// Parameters: +// - `t`: The type of the condition to remove. +// +// Returns: +// - `nil` if the condition was removed successfully or was not found. +func (r *Manager) ClearCondition(t string) error { + if r.accessor == nil { + return nil + } + + if !RemoveStatusCondition(r.accessor, t) { + return nil + } + + r.RecomputeHappiness(t) + + return nil +} + +// Mark updates the status of a specified condition type and applies optional modifications. +// +// This method allows setting a condition to any of the possible statuses (`True`, `False`, or +// `Unknown`) while also allowing additional options to modify the condition before it is stored. +// +// Parameters: +// - `t`: The type of the condition to update. +// - `status`: The new status of the condition (one of `metav1.ConditionTrue`, +// `metav1.ConditionFalse`, or `metav1.ConditionUnknown`). +// - `opts`: Variadic options that can modify attributes of the condition (e.g., reason, +// message, timestamp). +// +// Behavior: +// 1. Creates a `common.Condition` with the specified type and status. +// 2. Applies any provided options using `applyOpts(&c, opts...)`. +// 3. Sets the condition using `r.SetCondition(c)`, which updates the condition list and +// recomputes happiness. +func (r *Manager) Mark(t string, status metav1.ConditionStatus, opts ...Option) { + c := common.Condition{ + Type: t, + Status: status, + } + + applyOpts(&c, opts...) + + r.SetCondition(c) +} + +func (r *Manager) MarkTrue(t string, opts ...Option) { + r.Mark(t, metav1.ConditionTrue, opts...) +} + +func (r *Manager) MarkFalse(t string, opts ...Option) { + r.Mark(t, metav1.ConditionFalse, opts...) +} + +func (r *Manager) MarkUnknown(t string, opts ...Option) { + r.Mark(t, metav1.ConditionUnknown, opts...) +} + +func (r *Manager) MarkFrom(t string, in common.Condition) { + c := common.Condition{ + Type: t, + Status: in.Status, + Reason: in.Reason, + Message: in.Message, + Severity: in.Severity, + } + + r.SetCondition(c) +} + +// RecomputeHappiness re-evaluates the happiness state of the manager based on the current set +// of conditions. +// +// It checks if any dependent condition is unhappy (either `False` or `Unknown`). If found, the +// "happy" condition is updated to reflect the first unhappy condition's status. If no unhappy +// dependent conditions exist, it sets the "happy" condition to `True`. +// +// Parameters: +// - `t`: The type of the condition that may have triggered a recomputation of happiness. +func (r *Manager) RecomputeHappiness(t string) { + if c := r.findUnhappyDependent(); c != nil { + r.SetCondition(common.Condition{ + Type: r.happy, + Status: c.Status, + Reason: c.Reason, + Message: c.Message, + }) + } else if t != r.happy { + r.SetCondition(common.Condition{ + Type: r.happy, + Status: metav1.ConditionTrue, + }) + } +} + +// findUnhappyDependent identifies and returns the first dependent condition that is unhappy (i.e., +// False or Unknown). +// +// The method operates by filtering and sorting the current conditions, checking if they meet +// the criteria for being unhappy: +// - The condition must have a Severity level of "Error". +// - The dependent condition is either `ConditionFalse` or `ConditionUnknown`. +// +// The function performs the following steps: +// 1. It determines the number of dependents and retrieves the current conditions. +// 2. It iterates through each condition, filtering out conditions that do not meet the criteria +// (e.g., those not related to the dependents or those without "Error" severity). +// 3. It sorts the remaining conditions by the `LastTransitionTime` in descending order. +// 4. It returns the first `ConditionFalse` or `ConditionUnknown` condition found, prioritizing +// the former. +// +// If no unhappy condition is found, the function returns nil. +// +// Returns: +// - A pointer to the first unhappy condition if found, otherwise nil. +func (r *Manager) findUnhappyDependent() *common.Condition { + dn := len(r.dependents) + + conditions := slices.Clone(r.accessor.GetConditions()) + n := 0 + + for _, c := range conditions { + switch { + case dn == 0 && c.Type == r.happy: + break + case dn != 0 && !slices.Contains(r.dependents, c.Type): + break + case c.Severity != common.ConditionSeverityError: + break + default: + conditions[n] = c + n++ + } + } + + conditions = conditions[:n] + + sort.Slice(conditions, func(i, j int) bool { + return conditions[i].LastTransitionTime.After(conditions[j].LastTransitionTime.Time) + }) + + for _, c := range conditions { + if c.Status == metav1.ConditionFalse { + ret := c + return &ret + } + } + + for _, c := range conditions { + if c.Status == metav1.ConditionUnknown { + ret := c + return &ret + } + } + + return nil +} + +// Sort arranges the conditions retrieved from the accessor based on the following rules: +// 1. `happy` condition is assigned the highest priority. +// 2. `dependents` are prioritized in the order they are defined. +// 3. Conditions with priority `0` (not explicitly listed) are sorted alphabetically. +// +// The sorting is stable, ensuring consistent ordering when conditions have the same +// priority. +func (r *Manager) Sort() { + conditions := r.accessor.GetConditions() + if len(conditions) <= 1 { + return + } + + priorities := make(map[string]int) + dl := len(r.dependents) + + for i, d := range r.dependents { + priorities[d] = dl - i + } + + priorities[r.happy] = len(r.dependents) + 1 + + slices.SortStableFunc(conditions, func(a, b common.Condition) int { + ret := cmp.Compare(priorities[b.Type], priorities[a.Type]) + if ret == 0 { + ret = strings.Compare(a.Type, b.Type) + } + + return ret + }) +} diff --git a/pkg/controller/conditions/conditions_support.go b/pkg/controller/conditions/conditions_support.go index b3c7fd65d5d..0f5184d2327 100644 --- a/pkg/controller/conditions/conditions_support.go +++ b/pkg/controller/conditions/conditions_support.go @@ -25,6 +25,9 @@ func SetStatusCondition(a common.ConditionsAccessor, newCondition common.Conditi }) if idx == -1 { + if newCondition.LastTransitionTime.IsZero() { + newCondition.LastTransitionTime = metav1.NewTime(time.Now()) + } conditions = append(conditions, newCondition) a.SetConditions(conditions) return true @@ -34,20 +37,17 @@ func SetStatusCondition(a common.ConditionsAccessor, newCondition common.Conditi return false } - oldCondition := conditions[idx] + updateTransitionTime := conditions[idx].Status != newCondition.Status conditions[idx] = newCondition conditions[idx].LastHeartbeatTime = nil - // preserve transition time - conditions[idx].LastTransitionTime = oldCondition.LastTransitionTime - - if oldCondition.Status != newCondition.Status { + if updateTransitionTime { conditions[idx].LastTransitionTime = newCondition.LastTransitionTime - } - if conditions[idx].LastTransitionTime.IsZero() { - conditions[idx].LastTransitionTime = metav1.NewTime(time.Now()) + if conditions[idx].LastTransitionTime.IsZero() { + conditions[idx].LastTransitionTime = metav1.NewTime(time.Now()) + } } a.SetConditions(conditions) @@ -99,6 +99,12 @@ func IsStatusConditionPresentAndEqual(a common.ConditionsAccessor, conditionType }) } +func applyOpts(c *common.Condition, opts ...Option) { + for _, o := range opts { + o(c) + } +} + func equals(c1 common.Condition, c2 common.Condition) bool { return c1.Status == c2.Status && c1.Reason == c2.Reason && diff --git a/pkg/controller/conditions/conditions_test.go b/pkg/controller/conditions/conditions_test.go new file mode 100644 index 00000000000..7cc2685c041 --- /dev/null +++ b/pkg/controller/conditions/conditions_test.go @@ -0,0 +1,144 @@ +package conditions_test + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + + . "github.com/onsi/gomega" +) + +const ( + readyCondition = "Ready" + dependency1Condition = "Dependency1" + dependency2Condition = "Dependency2" +) + +type fakeAccessor struct { + conditions []common.Condition +} + +func (f *fakeAccessor) GetConditions() []common.Condition { + return f.conditions +} + +func (f *fakeAccessor) SetConditions(values []common.Condition) { + f.conditions = values +} + +func TestManager_InitializeConditions(t *testing.T) { + g := NewWithT(t) + + accessor := &fakeAccessor{} + manager := conditions.NewManager(accessor, readyCondition, dependency1Condition, dependency2Condition) + + g.Expect(accessor.GetConditions()).To(HaveLen(3)) + g.Expect(manager.GetCondition(readyCondition)).NotTo(BeNil()) + g.Expect(manager.GetCondition(readyCondition).Status).To(Equal(metav1.ConditionUnknown)) + g.Expect(manager.GetCondition(dependency1Condition)).NotTo(BeNil()) + g.Expect(manager.GetCondition(dependency2Condition)).NotTo(BeNil()) +} + +func TestManager_IsHappy(t *testing.T) { + g := NewWithT(t) + + accessor := &fakeAccessor{} + manager := conditions.NewManager(accessor, readyCondition, dependency1Condition, dependency2Condition) + + g.Expect(manager.IsHappy()).To(BeFalse()) + + manager.MarkFalse(dependency1Condition) + manager.MarkFalse(dependency2Condition) + + g.Expect(manager.IsHappy()).To(BeFalse()) + + manager.MarkTrue(dependency1Condition) + g.Expect(manager.IsHappy()).To(BeFalse()) + + manager.MarkTrue(dependency2Condition) + g.Expect(manager.IsHappy()).To(BeTrue()) +} + +func TestManager_IsHappy_NoDependants(t *testing.T) { + g := NewWithT(t) + + accessor := &fakeAccessor{} + accessor.SetConditions([]common.Condition{ + {Type: dependency1Condition, Status: metav1.ConditionUnknown}, + {Type: dependency2Condition, Status: metav1.ConditionUnknown}, + }) + + manager := conditions.NewManager(accessor, readyCondition) + g.Expect(manager.IsHappy()).To(BeFalse()) + + manager.MarkFalse(dependency1Condition) + g.Expect(manager.IsHappy()).To(BeFalse()) + + manager.MarkTrue(dependency1Condition) + g.Expect(manager.IsHappy()).To(BeFalse()) + + manager.MarkFalse(dependency2Condition) + g.Expect(manager.IsHappy()).To(BeFalse()) + + manager.MarkTrue(dependency2Condition) + g.Expect(manager.IsHappy()).To(BeTrue()) +} + +func TestManager_SetAndClearCondition(t *testing.T) { + g := NewWithT(t) + + accessor := &fakeAccessor{} + manager := conditions.NewManager(accessor, readyCondition, dependency1Condition) + + manager.MarkTrue(dependency1Condition) + g.Expect(manager.GetCondition(dependency1Condition)).NotTo(BeNil()) + g.Expect(manager.GetCondition(dependency1Condition).Status).To(Equal(metav1.ConditionTrue)) + + err := manager.ClearCondition(dependency1Condition) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(manager.GetCondition(dependency1Condition)).To(BeNil()) +} + +func TestManager_RecomputeHappiness(t *testing.T) { + g := NewWithT(t) + + accessor := &fakeAccessor{} + manager := conditions.NewManager(accessor, readyCondition, dependency1Condition, dependency2Condition) + + manager.MarkTrue(dependency1Condition) + manager.MarkFalse(dependency2Condition, conditions.WithSeverity(common.ConditionSeverityError)) + g.Expect(manager.IsHappy()).To(BeFalse()) + g.Expect(manager.GetTopLevelCondition().Status).To(Equal(metav1.ConditionFalse)) + + manager.MarkTrue(dependency2Condition) + g.Expect(manager.IsHappy()).To(BeTrue()) +} + +func TestManager_Sort(t *testing.T) { + g := NewWithT(t) + + accessor := &fakeAccessor{conditions: make([]common.Condition, 0)} + + manager := conditions.NewManager(accessor, "Z", "A", "C") + manager.MarkTrue("B") + manager.MarkTrue("D") + manager.MarkTrue("E") + manager.Sort() + + result := make([]string, 0, len(accessor.conditions)) + for _, c := range accessor.conditions { + result = append(result, c.Type) + } + + g.Expect(result).To(HaveExactElements( + "Z", + "A", + "C", + "B", + "D", + "E", + )) +} diff --git a/pkg/controller/predicates/dependent/dependent.go b/pkg/controller/predicates/dependent/dependent.go index ce7b37906a0..c2a871e8d7b 100644 --- a/pkg/controller/predicates/dependent/dependent.go +++ b/pkg/controller/predicates/dependent/dependent.go @@ -77,6 +77,14 @@ func (p Predicate) Update(e event.UpdateEvent) bool { if e.ObjectOld.GetResourceVersion() == e.ObjectNew.GetResourceVersion() { return false } + if !p.WatchStatus { + oldGen := e.ObjectOld.GetGeneration() + newGen := e.ObjectNew.GetGeneration() + + if oldGen == newGen && newGen != 0 { + return false + } + } oldObj, err := resources.ToUnstructured(e.ObjectOld) if err != nil { diff --git a/pkg/controller/reconciler/reconciler.go b/pkg/controller/reconciler/reconciler.go index cc22688bb20..c9bb8263ec7 100644 --- a/pkg/controller/reconciler/reconciler.go +++ b/pkg/controller/reconciler/reconciler.go @@ -7,6 +7,8 @@ import ( "reflect" "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/tools/record" @@ -18,14 +20,27 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager" "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" odhManager "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/manager" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" ) +type ReconcilerOpt func(*Reconciler) + +func WithConditionsManagerFactory(happy string, dependants ...string) ReconcilerOpt { + return func(reconciler *Reconciler) { + reconciler.conditionsManagerFactory = func(accessor common.ConditionsAccessor) *conditions.Manager { + return conditions.NewManager(accessor, happy, dependants...) + } + } +} + const platformFinalizer = "platform.opendatahub.io/finalizer" // Reconciler provides generic reconciliation functionality for ODH objects. @@ -39,13 +54,14 @@ type Reconciler struct { Recorder record.EventRecorder Release common.Release - name string - m *odhManager.Manager - instanceFactory func() (common.PlatformObject, error) + name string + m *odhManager.Manager + instanceFactory func() (common.PlatformObject, error) + conditionsManagerFactory func(common.ConditionsAccessor) *conditions.Manager } // NewReconciler creates a new reconciler for the given type. -func NewReconciler[T common.PlatformObject](mgr manager.Manager, name string, object T) (*Reconciler, error) { +func NewReconciler[T common.PlatformObject](mgr manager.Manager, name string, object T, opts ...ReconcilerOpt) (*Reconciler, error) { oc, err := odhClient.NewFromManager(mgr) if err != nil { return nil, err @@ -68,6 +84,13 @@ func NewReconciler[T common.PlatformObject](mgr manager.Manager, name string, ob return res, nil }, + conditionsManagerFactory: func(accessor common.ConditionsAccessor) *conditions.Manager { + return conditions.NewManager(accessor, status.ConditionTypeReady) + }, + } + + for _, opt := range opts { + opt(&cc) } return &cc, nil @@ -106,10 +129,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, err } - if err := r.Client.Get(ctx, client.ObjectKey{Name: req.Name}, res); err != nil { + if err := r.Client.Get(ctx, req.NamespacedName, res); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + if err := resources.EnsureGroupVersionKind(r.Client.Scheme(), res); err != nil { + return ctrl.Result{}, fmt.Errorf("unable to set GVK to instance: %w", err) + } + if !res.GetDeletionTimestamp().IsZero() { // resource is being deleted, attempt to perform clean-up logic and remove finalizer if !controllerutil.ContainsFinalizer(res, platformFinalizer) { @@ -176,11 +203,12 @@ func (r *Reconciler) delete(ctx context.Context, res common.PlatformObject) erro l.Info("delete") rr := types.ReconciliationRequest{ - Client: r.Client, - Manager: r.m, - Instance: res, - Release: r.Release, - Manifests: make([]types.ManifestInfo, 0), + Client: r.Client, + Manager: r.m, + Instance: res, + Conditions: r.conditionsManagerFactory(res), + Release: r.Release, + Manifests: make([]types.ManifestInfo, 0), // The DSCI should not be required when deleting a component, if the // component requires some additional info, then such info should be @@ -216,50 +244,97 @@ func (r *Reconciler) apply(ctx context.Context, res common.PlatformObject) error l := log.FromContext(ctx) l.Info("apply") - dsci, err := cluster.GetDSCI(ctx, r.Client) - if err != nil { - return errors.New("unable to find a DSCInitialization instance") - } - rr := types.ReconciliationRequest{ - Client: r.Client, - Manager: r.m, - Instance: res, - DSCI: dsci, - Release: r.Release, - Manifests: make([]types.ManifestInfo, 0), + Client: r.Client, + Manager: r.m, + Instance: res, + Conditions: r.conditionsManagerFactory(res), + Release: r.Release, + Manifests: make([]types.ManifestInfo, 0), } - // Execute actions - for _, action := range r.Actions { - l.Info("Executing action", "action", action) + var provisionErr error - actx := log.IntoContext( - ctx, - l.WithName(actions.ActionGroup).WithName(action.String()), - ) + dsci, dscilErr := cluster.GetDSCI(ctx, r.Client) + switch { + case dscilErr != nil: + provisionErr = fmt.Errorf("failed to get DSCInitialization: %w", dscilErr) + default: + provisionErr = nil + rr.DSCI = dsci.DeepCopy() - if err := action(actx, &rr); err != nil { - se := odherrors.StopError{} - if !errors.As(err, &se) { - l.Error(err, "Failed to execute action", "action", action) - return err - } + // Execute actions + for _, action := range r.Actions { + l.Info("Executing action", "action", action) - l.V(3).Info("detected stop marker", "action", action) - break + actx := log.IntoContext( + ctx, + l.WithName(actions.ActionGroup).WithName(action.String()), + ) + + provisionErr = action(actx, &rr) + if provisionErr != nil { + break + } } } - err = r.Client.ApplyStatus( + if provisionErr != nil { + rr.Conditions.MarkFalse( + status.ConditionTypeProvisioningSucceeded, + conditions.WithError(provisionErr), + conditions.WithObservedGeneration(rr.Instance.GetGeneration()), + ) + } else { + rr.Conditions.MarkTrue( + status.ConditionTypeProvisioningSucceeded, + conditions.WithObservedGeneration(rr.Instance.GetGeneration()), + ) + } + + is := rr.Instance.GetStatus() + is.Phase = status.PhaseNotReady + + // Update happiness to cover the case where conditions were + // not set using the provided helper functions + rr.Conditions.RecomputeHappiness("") + + // keep conditions sorted, keeping general conditions on the + // top, other conditions after + rr.Conditions.Sort() + + if rr.Conditions.IsHappy() { + is.Phase = status.PhaseReady + is.ObservedGeneration = rr.Instance.GetGeneration() + } + + err := r.Client.ApplyStatus( ctx, rr.Instance, client.FieldOwner(r.name), client.ForceOwnership, ) - if err != nil { - return client.IgnoreNotFound(err) + if err != nil && !k8serr.IsNotFound(err) { + r.Recorder.Event( + res, + corev1.EventTypeNormal, + "ReconcileError", + err.Error(), + ) + + return fmt.Errorf("reconile failed: %w", err) + } + + if provisionErr != nil { + r.Recorder.Event( + res, + corev1.EventTypeWarning, + "ProvisioningError", + provisionErr.Error(), + ) + + return fmt.Errorf("provisioning failed: %w", provisionErr) } return nil diff --git a/pkg/controller/reconciler/reconciler_support.go b/pkg/controller/reconciler/reconciler_support.go index 19027193372..a9d16b55394 100644 --- a/pkg/controller/reconciler/reconciler_support.go +++ b/pkg/controller/reconciler/reconciler_support.go @@ -17,6 +17,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates" @@ -72,19 +73,23 @@ func Dynamic(predicates ...DynamicPredicate) WatchOpts { } type ReconcilerBuilder[T common.PlatformObject] struct { - mgr ctrl.Manager - input forInput - watches []watchInput - predicates []predicate.Predicate - instanceName string - actions []actions.Fn - finalizers []actions.Fn - errors error + mgr ctrl.Manager + input forInput + watches []watchInput + predicates []predicate.Predicate + instanceName string + actions []actions.Fn + finalizers []actions.Fn + errors error + happyCondition string + dependantConditions []string } func ReconcilerFor[T common.PlatformObject](mgr ctrl.Manager, object T, opts ...builder.ForOption) *ReconcilerBuilder[T] { crb := ReconcilerBuilder[T]{ - mgr: mgr, + mgr: mgr, + happyCondition: status.ConditionTypeReady, + dependantConditions: []string{status.ConditionTypeProvisioningSucceeded}, } gvk, err := mgr.GetClient().GroupVersionKindFor(object) @@ -108,6 +113,15 @@ func ReconcilerFor[T common.PlatformObject](mgr ctrl.Manager, object T, opts ... return &crb } +func (b *ReconcilerBuilder[T]) WithConditions(dependants ...string) *ReconcilerBuilder[T] { + b.dependantConditions = append(b.dependantConditions, dependants...) + slices.Sort(b.dependantConditions) + + b.dependantConditions = slices.Compact(b.dependantConditions) + + return b +} + func (b *ReconcilerBuilder[T]) WithInstanceName(instanceName string) *ReconcilerBuilder[T] { b.instanceName = instanceName return b @@ -205,7 +219,8 @@ func (b *ReconcilerBuilder[T]) Build(_ context.Context) (*Reconciler, error) { if !ok { return nil, errors.New("invalid type for object") } - r, err := NewReconciler(b.mgr, name, obj) + + r, err := NewReconciler(b.mgr, name, obj, WithConditionsManagerFactory(b.happyCondition, b.dependantConditions...)) if err != nil { return nil, fmt.Errorf("failed to create reconciler for component %s: %w", name, err) } diff --git a/pkg/controller/reconciler/reconciler_test.go b/pkg/controller/reconciler/reconciler_test.go new file mode 100644 index 00000000000..988f0d9cc49 --- /dev/null +++ b/pkg/controller/reconciler/reconciler_test.go @@ -0,0 +1,201 @@ +//nolint:testpackage +package reconciler + +import ( + "context" + "errors" + "path/filepath" + "testing" + "time" + + gomegaTypes "github.com/onsi/gomega/types" + "github.com/rs/xid" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" + componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" + odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" + odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + odhtype "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq" + "github.com/opendatahub-io/opendatahub-operator/v2/tests/envtestutil" + + . "github.com/onsi/gomega" +) + +func createEnvTest(s *runtime.Scheme) (*envtest.Environment, error) { + utilruntime.Must(corev1.AddToScheme(s)) + utilruntime.Must(appsv1.AddToScheme(s)) + utilruntime.Must(apiextensionsv1.AddToScheme(s)) + utilruntime.Must(componentApi.AddToScheme(s)) + utilruntime.Must(dscv1.AddToScheme(s)) + utilruntime.Must(dsciv1.AddToScheme(s)) + + projectDir, err := envtestutil.FindProjectRoot() + if err != nil { + return nil, err + } + + envTest := envtest.Environment{ + CRDInstallOptions: envtest.CRDInstallOptions{ + Scheme: s, + Paths: []string{ + filepath.Join(projectDir, "config", "crd", "bases"), + }, + ErrorIfPathMissing: true, + CleanUpAfterUse: false, + }, + } + + return &envTest, nil +} + +func createReconciler(cli *odhClient.Client) *Reconciler { + return &Reconciler{ + Client: cli, + Scheme: cli.Scheme(), + Log: ctrl.Log.WithName("controllers").WithName("test"), + Release: cluster.GetRelease(), + Recorder: record.NewFakeRecorder(100), + name: "test", + instanceFactory: func() (common.PlatformObject, error) { + i := &componentApi.Dashboard{ + TypeMeta: ctrl.TypeMeta{ + APIVersion: gvk.Dashboard.GroupVersion().String(), + Kind: gvk.Dashboard.Kind, + }, + } + + return i, nil + }, + conditionsManagerFactory: func(accessor common.ConditionsAccessor) *conditions.Manager { + return conditions.NewManager(accessor, status.ConditionTypeReady) + }, + } +} + +func TestConditions(t *testing.T) { + ctx := context.Background() + + g := NewWithT(t) + s := runtime.NewScheme() + + envTest, err := createEnvTest(s) + g.Expect(err).NotTo(HaveOccurred()) + + t.Cleanup(func() { + _ = envTest.Stop() + }) + + cfg, err := envTest.Start() + g.Expect(err).NotTo(HaveOccurred()) + + envTestClient, err := client.New(cfg, client.Options{Scheme: s}) + g.Expect(err).NotTo(HaveOccurred()) + + cli, err := odhClient.NewFromConfig(cfg, envTestClient) + g.Expect(err).NotTo(HaveOccurred()) + + dsci := resources.GvkToUnstructured(gvk.DSCInitialization) + dsci.SetName(xid.New().String()) + dsci.SetGeneration(1) + + err = cli.Create(ctx, dsci) + g.Expect(err).NotTo(HaveOccurred()) + + tests := []struct { + name string + err error + matcher gomegaTypes.GomegaMatcher + }{ + { + name: "ready", + err: nil, + matcher: And( + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeReady, metav1.ConditionTrue), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeProvisioningSucceeded, metav1.ConditionTrue), + ), + }, + { + name: "stop", + err: odherrors.NewStopError("stop"), + matcher: And( + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeReady, metav1.ConditionFalse), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeProvisioningSucceeded, metav1.ConditionFalse), + ), + }, + { + name: "failure", + err: errors.New("failure"), + matcher: And( + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeReady, metav1.ConditionFalse), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeProvisioningSucceeded, metav1.ConditionFalse), + ), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dash := resources.GvkToUnstructured(gvk.Dashboard) + dash.SetName(componentApi.DashboardInstanceName) + dash.SetGeneration(1) + + err = cli.Create(ctx, dash) + g.Expect(err).NotTo(HaveOccurred()) + + req := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: componentApi.DashboardInstanceName, + }, + } + + cc := createReconciler(cli) + cc.AddAction(func(ctx context.Context, rr *odhtype.ReconciliationRequest) error { + return tt.err + }) + + result, err := cc.Reconcile(ctx, req) + if tt.err == nil { + g.Expect(err).ShouldNot(HaveOccurred()) + } else { + g.Expect(err).Should(MatchError(tt.err)) + } + + g.Expect(result.Requeue).Should(BeFalse()) + + di := dash.DeepCopy() + err = cli.Get(ctx, client.ObjectKeyFromObject(di), di) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(di).Should(tt.matcher) + + err = cli.Delete(ctx, di, client.PropagationPolicy(metav1.DeletePropagationBackground)) + g.Expect(err).ShouldNot(HaveOccurred()) + + g.Eventually(func() ([]componentApi.Dashboard, error) { + l := componentApi.DashboardList{} + if err := cli.List(ctx, &l, client.InNamespace("")); err != nil { + return nil, err + } + + return l.Items, nil + }).WithTimeout(10 * time.Second).Should(BeEmpty()) + }) + } +} diff --git a/pkg/controller/types/types.go b/pkg/controller/types/types.go index d0165805cae..2cf37653afb 100644 --- a/pkg/controller/types/types.go +++ b/pkg/controller/types/types.go @@ -14,6 +14,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" 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/manager" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" ) @@ -55,11 +56,12 @@ type TemplateInfo struct { type ReconciliationRequest struct { *odhClient.Client - Manager *manager.Manager - Instance common.PlatformObject - DSCI *dsciv1.DSCInitialization - Release common.Release - Manifests []ManifestInfo + Manager *manager.Manager + Conditions *conditions.Manager + Instance common.PlatformObject + DSCI *dsciv1.DSCInitialization + Release common.Release + Manifests []ManifestInfo // // TODO: unify templates and resources. diff --git a/tests/e2e/components_test.go b/tests/e2e/components_test.go index 4af057f72bd..bdcea09b45a 100644 --- a/tests/e2e/components_test.go +++ b/tests/e2e/components_test.go @@ -106,6 +106,7 @@ func (c *ComponentTestCtx) ValidateComponentEnabled(t *testing.T) { HaveEach(And( jq.Match(`.metadata.ownerReferences[0].kind == "%s"`, gvk.DataScienceCluster.Kind), jq.Match(`.status.conditions[] | select(.type == "Ready") | .status == "%s"`, metav1.ConditionTrue), + jq.Match(`.status.conditions[] | select(.type == "ProvisioningSucceeded") | .status == "%s"`, metav1.ConditionTrue), )), )) } diff --git a/tests/e2e/datasciencepipelines_test.go b/tests/e2e/datasciencepipelines_test.go index 62abd8ffbed..13933205de3 100644 --- a/tests/e2e/datasciencepipelines_test.go +++ b/tests/e2e/datasciencepipelines_test.go @@ -4,8 +4,14 @@ import ( "testing" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq" + + . "github.com/onsi/gomega" ) func dataSciencePipelinesTestSuite(t *testing.T) { @@ -19,6 +25,7 @@ func dataSciencePipelinesTestSuite(t *testing.T) { } t.Run("Validate component enabled", componentCtx.ValidateComponentEnabled) + t.Run("Validate component conditions", componentCtx.validateConditions) t.Run("Validate operands have OwnerReferences", componentCtx.ValidateOperandsOwnerReferences) t.Run("Validate update operand resources", componentCtx.ValidateUpdateDeploymentsResources) t.Run("Validate component disabled", componentCtx.ValidateComponentDisabled) @@ -28,3 +35,14 @@ func dataSciencePipelinesTestSuite(t *testing.T) { type DataSciencePipelinesTestCtx struct { *ComponentTestCtx } + +func (c *DataSciencePipelinesTestCtx) validateConditions(t *testing.T) { + g := c.NewWithT(t) + + g.List(gvk.DataSciencePipelines).Eventually().Should(And( + HaveLen(1), + HaveEach(And( + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionArgoWorkflowAvailable, metav1.ConditionTrue), + )), + )) +} diff --git a/tests/e2e/kserve_test.go b/tests/e2e/kserve_test.go index 20575d47e8c..5501babf613 100644 --- a/tests/e2e/kserve_test.go +++ b/tests/e2e/kserve_test.go @@ -18,6 +18,7 @@ import ( componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" featuresv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/components/modelcontroller" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/serverless" @@ -41,6 +42,7 @@ func kserveTestSuite(t *testing.T) { t.Run("Validate component enabled", componentCtx.ValidateComponentEnabled) t.Run("Validate component spec", componentCtx.validateSpec) + t.Run("Validate component conditions", componentCtx.validateConditions) t.Run("Validate FeatureTrackers", componentCtx.validateFeatureTrackers) t.Run("Validate model controller", componentCtx.validateModelControllerInstance) t.Run("Validate operands have OwnerReferences", componentCtx.ValidateOperandsOwnerReferences) @@ -121,6 +123,18 @@ func (c *KserveTestCtx) validateSpec(t *testing.T) { )) } +func (c *KserveTestCtx) validateConditions(t *testing.T) { + g := c.NewWithT(t) + + g.List(gvk.Kserve).Eventually().Should(And( + HaveLen(1), + HaveEach(And( + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionServerlessAvailable, metav1.ConditionTrue), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionServiceMeshAvailable, metav1.ConditionTrue), + )), + )) +} + func (c *KserveTestCtx) validateFeatureTrackers(t *testing.T) { g := c.NewWithT(t) ftName := types.NamespacedName{Name: c.ApplicationNamespace + "-serverless-serving-deployment"} diff --git a/tests/e2e/modelregistry_test.go b/tests/e2e/modelregistry_test.go index aa9db63d9d8..d78990446c6 100644 --- a/tests/e2e/modelregistry_test.go +++ b/tests/e2e/modelregistry_test.go @@ -6,12 +6,14 @@ import ( "github.com/rs/xid" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" modelregistryctrl "github.com/opendatahub-io/opendatahub-operator/v2/controllers/components/modelregistry" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" @@ -38,6 +40,7 @@ func modelRegistryTestSuite(t *testing.T) { t.Run("Validate component enabled", componentCtx.ValidateComponentEnabled) t.Run("Validate component spec", componentCtx.validateSpec) + t.Run("Validate component conditions", componentCtx.validateConditions) t.Run("Validate operands have OwnerReferences", componentCtx.ValidateOperandsOwnerReferences) t.Run("Validate update operand resources", componentCtx.ValidateUpdateDeploymentsResources) @@ -65,6 +68,17 @@ func (c *ModelRegistryTestCtx) validateSpec(t *testing.T) { )) } +func (c *ModelRegistryTestCtx) validateConditions(t *testing.T) { + g := c.NewWithT(t) + + g.List(gvk.ModelRegistry).Eventually().Should(And( + HaveLen(1), + HaveEach(And( + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionServerlessAvailable, metav1.ConditionTrue), + )), + )) +} + func (c *ModelRegistryTestCtx) validateOperandsWatchedResources(t *testing.T) { g := c.NewWithT(t) diff --git a/tests/e2e/workbenches_test.go b/tests/e2e/workbenches_test.go index 28466c5dba3..129a23d10e6 100644 --- a/tests/e2e/workbenches_test.go +++ b/tests/e2e/workbenches_test.go @@ -12,7 +12,6 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" "github.com/stretchr/testify/require" autoscalingv1 "k8s.io/api/autoscaling/v1" - corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -196,7 +195,7 @@ func (tc *WorkbenchesTestCtx) validateWorkbenchesReady() error { for _, c := range list.Items[0].Status.Conditions { if c.Type == workbenches.ReadyConditionType { - return c.Status == corev1.ConditionTrue, nil + return c.Status == metav1.ConditionTrue, nil } } From 584a72a2bd28eba4721689431e07c4f8711821b3 Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Wed, 19 Feb 2025 09:15:48 +0100 Subject: [PATCH 2/2] fix linter --- .../datasciencepipelines_controller_actions.go | 1 - pkg/controller/conditions/conditions_support_test.go | 12 ------------ 2 files changed, 13 deletions(-) diff --git a/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go b/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go index b85832983c5..0d1563764de 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go @@ -22,7 +22,6 @@ import ( k8serr "k8s.io/apimachinery/pkg/api/errors" - "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" diff --git a/pkg/controller/conditions/conditions_support_test.go b/pkg/controller/conditions/conditions_support_test.go index c9d408b758f..1467f589be2 100644 --- a/pkg/controller/conditions/conditions_support_test.go +++ b/pkg/controller/conditions/conditions_support_test.go @@ -11,18 +11,6 @@ import ( . "github.com/onsi/gomega" ) -type fakeAccessor struct { - conditions []common.Condition -} - -func (f *fakeAccessor) GetConditions() []common.Condition { - return f.conditions -} - -func (f *fakeAccessor) SetConditions(values []common.Condition) { - f.conditions = values -} - func TestSetStatusCondition_LastTransitionTime(t *testing.T) { a := fakeAccessor{} a.conditions = make([]common.Condition, 0)