Improve conditions reporting

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{})
 }

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: |-

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: |-

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 @@
 	"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 @@
 	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
 }