fix: print-columns and ready condition aggregate (#104)

Added some better reasons to conditions and use `Ready` condition as aggregate for all other conditions present. For example if the poolManager is not ready `Ready=False` with a `Reason=ComponentNotReady` and more details provided in the separate `PoolManager` condition: ```yaml status: conditions: - lastTransitionTime: "2024-03-11T13:34:50Z" message: Pool Manager is not running reason: ComponentNotReady status: "False" type: Ready - lastTransitionTime: "2024-03-11T13:34:50Z" message: 'failed to update tools for repo cnt-dev/rthalho-test: fetching tools: Unauthorized' reason: PoolManagerFailure status: "False" type: PoolManager - lastTransitionTime: "2024-03-05T09:27:24Z" message: "" reason: FetchingSecretRefSuccess status: "True" type: SecretReference id: 70056770-a244-4fa4-b347-3b5d03cda874 ``` and fixed print columns, so with the `-owide` flag one gets more details about the current state ![image](https://github.com/mercedes-benz/garm-operator/assets/48678421/a3ca5b09-c12b-4557-bbb2-c7b62468addd)
mercedes-benz · May 13, 2024 · 700efd7 · 700efd7
1 parent 3816a7a
commit 700efd7
Show file tree

Hide file tree

Showing 18 changed files with 226 additions and 116 deletions.
diff --git a/Makefile b/Makefile
@@ -318,7 +318,7 @@ generate-doctoc: mdtoc ## Generate documentation table of contents
 ##@ Local Development
 
 .PHONY: kind-cluster
-kind-cluster: ## Create a new kind cluster designed for local development
+kind-cluster: kind ## Create a new kind cluster designed for local development
 	hack/kind-with-registry.sh
 
 .PHONY: delete-kind-cluster

diff --git a/api/v1alpha1/enterprise_types.go b/api/v1alpha1/enterprise_types.go
@@ -26,8 +26,9 @@ type EnterpriseStatus struct {
 //+kubebuilder:resource:path=enterprises,scope=Namespaced,categories=garm,shortName=ent
 //+kubebuilder:subresource:status
 //+kubebuilder:printcolumn:name="ID",type="string",JSONPath=".status.id",description="Enterprise ID"
-//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.poolManagerIsRunning",description="Status of the referenced pool"
-//+kubebuilder:printcolumn:name="Error",type="string",JSONPath=".status.poolManagerFailureReason",description="Error description",priority=1
+//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
+//+kubebuilder:printcolumn:name="Error",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].message",priority=1
+//+kubebuilder:printcolumn:name="Pool_Manager_Failure",type="string",JSONPath=`.status.conditions[?(@.reason=='PoolManagerFailure')].message`,priority=1
 //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Time duration since creation of Enterprise"
 
 // Enterprise is the Schema for the enterprises API

diff --git a/api/v1alpha1/organization_types.go b/api/v1alpha1/organization_types.go
@@ -26,8 +26,9 @@ type OrganizationStatus struct {
 //+kubebuilder:resource:path=organizations,scope=Namespaced,categories=garm,shortName=org
 //+kubebuilder:subresource:status
 //+kubebuilder:printcolumn:name="ID",type="string",JSONPath=".status.id",description="Organization ID"
-//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.poolManagerIsRunning",description="Status of the referenced pool"
-//+kubebuilder:printcolumn:name="Error",type="string",JSONPath=".status.poolManagerFailureReason",description="Error description",priority=1
+//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
+//+kubebuilder:printcolumn:name="Error",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].message",priority=1
+//+kubebuilder:printcolumn:name="Pool_Manager_Failure",type="string",JSONPath=`.status.conditions[?(@.reason=='PoolManagerFailure')].message`,priority=1
 //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Time duration since creation of Organization"
 
 // Organization is the Schema for the organizations API

diff --git a/api/v1alpha1/pool_types.go b/api/v1alpha1/pool_types.go
@@ -62,7 +62,7 @@ func (p *Pool) GetConditions() []metav1.Condition {
 
 //+kubebuilder:object:root=true
 //+kubebuilder:subresource:status
-//+kubebuilder:subresource:scale:specpath=.spec.minIdleRunners,statuspath=.status.idleRunners,selectorpath=.status.selector
+//+kubebuilder:subresource:scale:specpath=.spec.minIdleRunners,statuspath=.status.longRunningIdleRunners,selectorpath=.status.selector
 //+kubebuilder:resource:path=pools,scope=Namespaced,categories=garm
 //+kubebuilder:printcolumn:name="ID",type=string,JSONPath=`.status.id`
 //+kubebuilder:printcolumn:name="MinIdleRunners",type=string,JSONPath=`.spec.minIdleRunners`
@@ -72,7 +72,8 @@ func (p *Pool) GetConditions() []metav1.Condition {
 //+kubebuilder:printcolumn:name="Provider",type=string,JSONPath=`.spec.providerName`,priority=1
 //+kubebuilder:printcolumn:name="ScopeType",type=string,JSONPath=`.spec.githubScopeRef.kind`,priority=1
 //+kubebuilder:printcolumn:name="ScopeName",type=string,JSONPath=`.spec.githubScopeRef.name`,priority=1
-//+kubebuilder:printcolumn:name="Error",type=string,JSONPath=`.status.lastSyncError`,priority=1
+//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
+//+kubebuilder:printcolumn:name="Error",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].message",priority=1
 //+kubebuilder:printcolumn:name="Enabled",type=boolean,JSONPath=`.spec.enabled`,priority=1
 //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
 

diff --git a/api/v1alpha1/repository_types.go b/api/v1alpha1/repository_types.go
@@ -27,8 +27,9 @@ type RepositoryStatus struct {
 //+kubebuilder:resource:path=repositories,scope=Namespaced,categories=garm,shortName=repo
 //+kubebuilder:subresource:status
 //+kubebuilder:printcolumn:name="ID",type="string",JSONPath=".status.id",description="Repository ID"
-//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.poolManagerIsRunning",description="Status of the referenced pool"
-//+kubebuilder:printcolumn:name="Error",type="string",JSONPath=".status.poolManagerFailureReason",description="Error description",priority=1
+//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
+//+kubebuilder:printcolumn:name="Error",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].message",priority=1
+//+kubebuilder:printcolumn:name="Pool_Manager_Failure",type="string",JSONPath=`.status.conditions[?(@.reason=='PoolManagerFailure')].message`,priority=1
 //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Time duration since creation of Repository"
 
 // Repository is the Schema for the repositories API

diff --git a/config/crd/bases/garm-operator.mercedes-benz.com_enterprises.yaml b/config/crd/bases/garm-operator.mercedes-benz.com_enterprises.yaml
@@ -23,15 +23,17 @@ spec:
       jsonPath: .status.id
       name: ID
       type: string
-    - description: Status of the referenced pool
-      jsonPath: .status.poolManagerIsRunning
+    - jsonPath: .status.conditions[?(@.type=='Ready')].status
       name: Ready
       type: string
-    - description: Error description
-      jsonPath: .status.poolManagerFailureReason
+    - jsonPath: .status.conditions[?(@.type=='Ready')].message
       name: Error
       priority: 1
       type: string
+    - jsonPath: .status.conditions[?(@.reason=='PoolManagerFailure')].message
+      name: Pool_Manager_Failure
+      priority: 1
+      type: string
     - description: Time duration since creation of Enterprise
       jsonPath: .metadata.creationTimestamp
       name: Age

diff --git a/config/crd/bases/garm-operator.mercedes-benz.com_organizations.yaml b/config/crd/bases/garm-operator.mercedes-benz.com_organizations.yaml
@@ -23,15 +23,17 @@ spec:
       jsonPath: .status.id
       name: ID
       type: string
-    - description: Status of the referenced pool
-      jsonPath: .status.poolManagerIsRunning
+    - jsonPath: .status.conditions[?(@.type=='Ready')].status
       name: Ready
       type: string
-    - description: Error description
-      jsonPath: .status.poolManagerFailureReason
+    - jsonPath: .status.conditions[?(@.type=='Ready')].message
       name: Error
       priority: 1
       type: string
+    - jsonPath: .status.conditions[?(@.reason=='PoolManagerFailure')].message
+      name: Pool_Manager_Failure
+      priority: 1
+      type: string
     - description: Time duration since creation of Organization
       jsonPath: .metadata.creationTimestamp
       name: Age

diff --git a/config/crd/bases/garm-operator.mercedes-benz.com_pools.yaml b/config/crd/bases/garm-operator.mercedes-benz.com_pools.yaml
@@ -46,7 +46,10 @@ spec:
       name: ScopeName
       priority: 1
       type: string
-    - jsonPath: .status.lastSyncError
+    - jsonPath: .status.conditions[?(@.type=='Ready')].status
+      name: Ready
+      type: string
+    - jsonPath: .status.conditions[?(@.type=='Ready')].message
       name: Error
       priority: 1
       type: string
@@ -233,5 +236,5 @@ spec:
       scale:
         labelSelectorPath: .status.selector
         specReplicasPath: .spec.minIdleRunners
-        statusReplicasPath: .status.idleRunners
+        statusReplicasPath: .status.longRunningIdleRunners
       status: {}
diff --git a/config/crd/bases/garm-operator.mercedes-benz.com_repositories.yaml b/config/crd/bases/garm-operator.mercedes-benz.com_repositories.yaml
@@ -23,15 +23,17 @@ spec:
       jsonPath: .status.id
       name: ID
       type: string
-    - description: Status of the referenced pool
-      jsonPath: .status.poolManagerIsRunning
+    - jsonPath: .status.conditions[?(@.type=='Ready')].status
       name: Ready
       type: string
-    - description: Error description
-      jsonPath: .status.poolManagerFailureReason
+    - jsonPath: .status.conditions[?(@.type=='Ready')].message
       name: Error
       priority: 1
       type: string
+    - jsonPath: .status.conditions[?(@.reason=='PoolManagerFailure')].message
+      name: Pool_Manager_Failure
+      priority: 1
+      type: string
     - description: Time duration since creation of Repository
       jsonPath: .metadata.creationTimestamp
       name: Age

diff --git a/internal/controller/enterprise_controller.go b/internal/controller/enterprise_controller.go
@@ -81,8 +81,9 @@ func (r *EnterpriseReconciler) reconcileNormal(ctx context.Context, client garmC
 
 	webhookSecret, err := secret.FetchRef(ctx, r.Client, &enterprise.Spec.WebhookSecretRef, enterprise.Namespace)
 	if err != nil {
-		conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.ReconcileErrorReason, err.Error())
+		conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.FetchingSecretRefFailedReason, err.Error())
 		conditions.MarkFalse(enterprise, conditions.SecretReference, conditions.FetchingSecretRefFailedReason, err.Error())
+		conditions.MarkUnknown(enterprise, conditions.PoolManager, conditions.UnknownReason, conditions.GarmServerNotReconciledYetMsg)
 		if err := r.Status().Update(ctx, enterprise); err != nil {
 			return ctrl.Result{}, err
 		}
@@ -93,7 +94,7 @@ func (r *EnterpriseReconciler) reconcileNormal(ctx context.Context, client garmC
 	garmEnterprise, err := r.getExistingGarmEnterprise(ctx, client, enterprise)
 	if err != nil {
 		event.Error(r.Recorder, enterprise, err.Error())
-		conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.ReconcileErrorReason, err.Error())
+		conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.GarmAPIErrorReason, err.Error())
 		if err := r.Status().Update(ctx, enterprise); err != nil {
 			return ctrl.Result{}, err
 		}
@@ -105,7 +106,7 @@ func (r *EnterpriseReconciler) reconcileNormal(ctx context.Context, client garmC
 		garmEnterprise, err = r.createEnterprise(ctx, client, enterprise, webhookSecret)
 		if err != nil {
 			event.Error(r.Recorder, enterprise, err.Error())
-			conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.ReconcileErrorReason, err.Error())
+			conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.GarmAPIErrorReason, err.Error())
 			if err := r.Status().Update(ctx, enterprise); err != nil {
 				return ctrl.Result{}, err
 			}
@@ -117,7 +118,7 @@ func (r *EnterpriseReconciler) reconcileNormal(ctx context.Context, client garmC
 	garmEnterprise, err = r.updateEnterprise(ctx, client, garmEnterprise.ID, webhookSecret, enterprise.Spec.CredentialsName)
 	if err != nil {
 		event.Error(r.Recorder, enterprise, err.Error())
-		conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.ReconcileErrorReason, err.Error())
+		conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.GarmAPIErrorReason, err.Error())
 		if err := r.Status().Update(ctx, enterprise); err != nil {
 			return ctrl.Result{}, err
 		}
@@ -126,12 +127,14 @@ func (r *EnterpriseReconciler) reconcileNormal(ctx context.Context, client garmC
 
 	// set and update enterprise status
 	enterprise.Status.ID = garmEnterprise.ID
-	conditions.MarkTrue(enterprise, conditions.PoolManager, conditions.PoolManagerRunningReason, garmEnterprise.PoolManagerStatus.FailureReason)
+	conditions.MarkTrue(enterprise, conditions.ReadyCondition, conditions.SuccessfulReconcileReason, "")
+	conditions.MarkTrue(enterprise, conditions.PoolManager, conditions.PoolManagerRunningReason, "")
+
 	if !garmEnterprise.PoolManagerStatus.IsRunning {
+		conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.PoolManagerFailureReason, "Pool Manager is not running")
 		conditions.MarkFalse(enterprise, conditions.PoolManager, conditions.PoolManagerFailureReason, garmEnterprise.PoolManagerStatus.FailureReason)
 	}
 
-	conditions.MarkTrue(enterprise, conditions.ReadyCondition, conditions.SuccessfulReconcileReason, "")
 	if err := r.Status().Update(ctx, enterprise); err != nil {
 		return ctrl.Result{}, err
 	}
@@ -208,25 +211,25 @@ func (r *EnterpriseReconciler) getExistingGarmEnterprise(ctx context.Context, cl
 	return params.Enterprise{}, nil
 }
 
-func (r *EnterpriseReconciler) reconcileDelete(ctx context.Context, scope garmClient.EnterpriseClient, enterprise *garmoperatorv1alpha1.Enterprise) (ctrl.Result, error) {
+func (r *EnterpriseReconciler) reconcileDelete(ctx context.Context, client garmClient.EnterpriseClient, enterprise *garmoperatorv1alpha1.Enterprise) (ctrl.Result, error) {
 	log := log.FromContext(ctx)
 	log.WithValues("enterprise", enterprise.Name)
 
 	log.Info("starting enterprise deletion")
 	event.Deleting(r.Recorder, enterprise, "starting enterprise deletion")
-	conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.DeletingReason, "Deleting Enterprise")
+	conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.DeletingReason, conditions.DeletingEnterpriseMsg)
 	if err := r.Status().Update(ctx, enterprise); err != nil {
 		return ctrl.Result{}, err
 	}
 
-	err := scope.DeleteEnterprise(
+	err := client.DeleteEnterprise(
 		enterprises.NewDeleteEnterpriseParams().
 			WithEnterpriseID(enterprise.Status.ID),
 	)
 	if err != nil {
 		log.V(1).Info(fmt.Sprintf("client.DeleteEnterprise error: %s", err))
 		event.Error(r.Recorder, enterprise, err.Error())
-		conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.ReconcileErrorReason, err.Error())
+		conditions.MarkFalse(enterprise, conditions.ReadyCondition, conditions.GarmAPIErrorReason, err.Error())
 		if err := r.Status().Update(ctx, enterprise); err != nil {
 			return ctrl.Result{}, err
 		}

diff --git a/internal/controller/enterprise_controller_test.go b/internal/controller/enterprise_controller_test.go
@@ -89,9 +89,9 @@ func TestEnterpriseReconciler_reconcileNormal(t *testing.T) {
 					Conditions: []metav1.Condition{
 						{
 							Type:               string(conditions.ReadyCondition),
-							Reason:             string(conditions.SuccessfulReconcileReason),
-							Status:             metav1.ConditionTrue,
-							Message:            "",
+							Reason:             string(conditions.PoolManagerFailureReason),
+							Status:             metav1.ConditionFalse,
+							Message:            "Pool Manager is not running",
 							LastTransitionTime: metav1.NewTime(time.Now()),
 						},
 						{
@@ -187,9 +187,9 @@ func TestEnterpriseReconciler_reconcileNormal(t *testing.T) {
 					Conditions: []metav1.Condition{
 						{
 							Type:               string(conditions.ReadyCondition),
-							Reason:             string(conditions.SuccessfulReconcileReason),
-							Status:             metav1.ConditionTrue,
-							Message:            "",
+							Reason:             string(conditions.PoolManagerFailureReason),
+							Status:             metav1.ConditionFalse,
+							Message:            "Pool Manager is not running",
 							LastTransitionTime: metav1.NewTime(time.Now()),
 						},
 						{
@@ -274,9 +274,9 @@ func TestEnterpriseReconciler_reconcileNormal(t *testing.T) {
 					Conditions: []metav1.Condition{
 						{
 							Type:               string(conditions.ReadyCondition),
-							Reason:             string(conditions.SuccessfulReconcileReason),
-							Status:             metav1.ConditionTrue,
-							Message:            "",
+							Reason:             string(conditions.PoolManagerFailureReason),
+							Status:             metav1.ConditionFalse,
+							Message:            "Pool Manager is not running",
 							LastTransitionTime: metav1.NewTime(time.Now()),
 						},
 						{
@@ -370,9 +370,9 @@ func TestEnterpriseReconciler_reconcileNormal(t *testing.T) {
 					Conditions: []metav1.Condition{
 						{
 							Type:               string(conditions.ReadyCondition),
-							Reason:             string(conditions.SuccessfulReconcileReason),
-							Status:             metav1.ConditionTrue,
-							Message:            "",
+							Reason:             string(conditions.PoolManagerFailureReason),
+							Status:             metav1.ConditionFalse,
+							Message:            "Pool Manager is not running",
 							LastTransitionTime: metav1.NewTime(time.Now()),
 						},
 						{
@@ -475,9 +475,9 @@ func TestEnterpriseReconciler_reconcileNormal(t *testing.T) {
 					Conditions: []metav1.Condition{
 						{
 							Type:               string(conditions.ReadyCondition),
-							Reason:             string(conditions.SuccessfulReconcileReason),
-							Status:             metav1.ConditionTrue,
-							Message:            "",
+							Reason:             string(conditions.PoolManagerFailureReason),
+							Status:             metav1.ConditionFalse,
+							Message:            "Pool Manager is not running",
 							LastTransitionTime: metav1.NewTime(time.Now()),
 						},
 						{
@@ -583,9 +583,9 @@ func TestEnterpriseReconciler_reconcileNormal(t *testing.T) {
 					Conditions: []metav1.Condition{
 						{
 							Type:               string(conditions.ReadyCondition),
-							Reason:             string(conditions.SuccessfulReconcileReason),
-							Status:             metav1.ConditionTrue,
-							Message:            "",
+							Reason:             string(conditions.PoolManagerFailureReason),
+							Status:             metav1.ConditionFalse,
+							Message:            "Pool Manager is not running",
 							LastTransitionTime: metav1.NewTime(time.Now()),
 						},
 						{
@@ -676,11 +676,18 @@ func TestEnterpriseReconciler_reconcileNormal(t *testing.T) {
 					Conditions: []metav1.Condition{
 						{
 							Type:               string(conditions.ReadyCondition),
-							Reason:             string(conditions.ReconcileErrorReason),
+							Reason:             string(conditions.FetchingSecretRefFailedReason),
 							Status:             metav1.ConditionFalse,
 							Message:            "secrets \"my-webhook-secret\" not found",
 							LastTransitionTime: metav1.NewTime(time.Now()),
 						},
+						{
+							Type:               string(conditions.PoolManager),
+							Reason:             string(conditions.UnknownReason),
+							Status:             metav1.ConditionUnknown,
+							Message:            "GARM server not reconciled yet",
+							LastTransitionTime: metav1.NewTime(time.Now()),
+						},
 						{
 							Type:               string(conditions.SecretReference),
 							Reason:             string(conditions.FetchingSecretRefFailedReason),