Merge pull request #70 from m00g3n/fix-error-state

Fix invalid condition status for errors in GardenerCluster
kyma-project · Nov 20, 2023 · d79e85b · d79e85b
2 parents 19dc469 + 5875924
commit d79e85b
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 32 deletions.
diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go
@@ -123,12 +123,12 @@ func (cluster *GardenerCluster) UpdateConditionForReadyState(conditionType Condi
 	meta.SetStatusCondition(&cluster.Status.Conditions, condition)
 }
 
-func (cluster *GardenerCluster) UpdateConditionForErrorState(conditionType ConditionType, reason ConditionReason, conditionStatus metav1.ConditionStatus, error error) {
+func (cluster *GardenerCluster) UpdateConditionForErrorState(conditionType ConditionType, reason ConditionReason, error error) {
 	cluster.Status.State = ErrorState
 
 	condition := metav1.Condition{
 		Type:               string(conditionType),
-		Status:             conditionStatus,
+		Status:             metav1.ConditionFalse,
 		LastTransitionTime: metav1.Now(),
 		Reason:             string(reason),
 		Message:            fmt.Sprintf("%s Error: %s", getMessage(reason), error.Error()),

diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go
@@ -86,7 +86,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req
 	err := controller.Get(ctx, req.NamespacedName, &cluster)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
-			err = controller.deleteKubeconfigSecret(req.Name)
+			err = controller.deleteKubeconfigSecret(ctx, req.Name)
 		}
 
 		if err == nil {
@@ -117,10 +117,8 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req
 	}
 
 	if kubeconfigStatus == ksCreated || kubeconfigStatus == ksModified {
-		err = controller.persistStatusChange(ctx, &cluster)
-		if err != nil {
-			return controller.resultWithoutRequeue(), err
-		}
+		_ = controller.persistStatusChange(ctx, &cluster)
+		return controller.resultWithoutRequeue(), nil
 	}
 
 	return controller.resultWithRequeue(), nil
@@ -146,34 +144,20 @@ func (controller *GardenerClusterController) resultWithoutRequeue() ctrl.Result
 }
 
 func (controller *GardenerClusterController) persistStatusChange(ctx context.Context, cluster *imv1.GardenerCluster) error {
-	key := types.NamespacedName{
-		Name:      cluster.Name,
-		Namespace: cluster.Namespace,
-	}
-	var clusterToUpdate imv1.GardenerCluster
-
-	err := controller.Get(ctx, key, &clusterToUpdate)
+	err := controller.Client.Status().Update(ctx, cluster)
 	if err != nil {
-		return err
+		controller.log.Error(err, "status update failed")
 	}
-
-	clusterToUpdate.Status = cluster.Status
-
-	statusErr := controller.Client.Status().Update(ctx, &clusterToUpdate)
-	if statusErr != nil {
-		controller.log.Error(statusErr, "Failed to set state for GardenerCluster")
-	}
-
-	return statusErr
+	return err
 }
 
-func (controller *GardenerClusterController) deleteKubeconfigSecret(clusterCRName string) error {
+func (controller *GardenerClusterController) deleteKubeconfigSecret(ctx context.Context, clusterCRName string) error {
 	selector := client.MatchingLabels(map[string]string{
 		clusterCRNameLabel: clusterCRName,
 	})
 
 	var secretList corev1.SecretList
-	err := controller.Client.List(context.TODO(), &secretList, selector)
+	err := controller.Client.List(ctx, &secretList, selector)
 	if err != nil {
 		return err
 	}
@@ -182,7 +166,7 @@ func (controller *GardenerClusterController) deleteKubeconfigSecret(clusterCRNam
 		return errors.Errorf("unexpected numer of secrets found for cluster CR `%s`", clusterCRName)
 	}
 
-	return controller.Client.Delete(context.TODO(), &secretList.Items[0])
+	return controller.Client.Delete(ctx, &secretList.Items[0])
 }
 
 func (controller *GardenerClusterController) getSecret(shootName string) (*corev1.Secret, error) {
@@ -222,13 +206,13 @@ const (
 func (controller *GardenerClusterController) handleKubeconfig(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) (kubeconfigStatus, error) {
 	existingSecret, err := controller.getSecret(cluster.Spec.Shoot.Name)
 	if err != nil && !k8serrors.IsNotFound(err) {
-		cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err)
+		cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, err)
 		return ksZero, err
 	}
 
 	kubeconfig, err := controller.KubeconfigProvider.Fetch(ctx, cluster.Spec.Shoot.Name)
 	if err != nil {
-		cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, metav1.ConditionTrue, err)
+		cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, err)
 		return ksZero, err
 	}
 
@@ -238,7 +222,7 @@ func (controller *GardenerClusterController) handleKubeconfig(ctx context.Contex
 
 		// delete secret containing kubeconfig to be rotated
 		if err := controller.removeKubeconfig(ctx, cluster, existingSecret); err != nil {
-			cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToDeleteSecret, metav1.ConditionTrue, err)
+			cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToDeleteSecret, err)
 			return ksZero, err
 		}
 
@@ -302,7 +286,7 @@ func (controller *GardenerClusterController) createNewSecret(ctx context.Context
 	newSecret := controller.newSecret(*cluster, kubeconfig, lastSyncTime)
 	err := controller.Create(ctx, &newSecret)
 	if err != nil {
-		cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, metav1.ConditionTrue, err)
+		cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, err)
 		return err
 	}
 
@@ -343,7 +327,7 @@ func (controller *GardenerClusterController) updateExistingSecret(ctx context.Co
 
 	err := controller.Update(ctx, existingSecret)
 	if err != nil {
-		cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToUpdateSecret, metav1.ConditionTrue, err)
+		cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToUpdateSecret, err)
 
 		return err
 	}