From 5b0bbac231242ea38caf83c2b540b8eb73be95a1 Mon Sep 17 00:00:00 2001 From: m00g3n Date: Mon, 20 Nov 2023 12:59:32 +0100 Subject: [PATCH 1/2] fix invalid condition status for errors --- api/v1/gardenercluster_types.go | 4 +-- .../controller/gardener_cluster_controller.go | 36 ++++++------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go index 4f658008..48214b8f 100644 --- 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 index 112d7593..6deb0f68 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -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,25 +144,11 @@ 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 { @@ -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 } From 58759242fe9187e0e49c03d4cc19880fff4132c4 Mon Sep 17 00:00:00 2001 From: m00g3n Date: Mon, 20 Nov 2023 13:08:58 +0100 Subject: [PATCH 2/2] fix context for kubeconfig secret deletion --- internal/controller/gardener_cluster_controller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 6deb0f68..e552dc82 100644 --- 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 { @@ -151,13 +151,13 @@ func (controller *GardenerClusterController) persistStatusChange(ctx context.Con 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 } @@ -166,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) {