From f91f294c8bacb6e1c05893516643e428ecf652e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Tue, 26 Sep 2023 14:06:35 +0200 Subject: [PATCH 01/39] adds first version of states update --- api/v1/gardenercluster_types.go | 28 +++++++++++++++++++ .../samples/clusterinventory_v1_cluster.yaml | 0 ...astructuremanager_v1_gardenercluster.yaml} | 4 +-- .../controller/gardener_cluster_controller.go | 8 ++++++ 4 files changed, 38 insertions(+), 2 deletions(-) delete mode 100644 config/samples/clusterinventory_v1_cluster.yaml rename config/samples/{clusterinventory_v1_gardenercluster.yaml => infrastructuremanager_v1_gardenercluster.yaml} (84%) diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go index a982c145..cd66c895 100644 --- a/api/v1/gardenercluster_types.go +++ b/api/v1/gardenercluster_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1 import ( + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -73,6 +74,21 @@ const ( DeletingState State = "Deleting" ) +type ConditionReason string + +const ( + ConditionReasonSecretCreated State = "SecretCreated" + ConditionReasonSecretNotFound State = "SecretNotFound" + ConditionReasonGardenClusterNotRetrieved State = "GardenClusterNotRetrieved" + ConditionReasonGardenClusterNotFound State = "GardenClusterNotFound" +) + +type ConditionType string + +const ( + ConditionTypeUnknown State = "Unknown" +) + // GardenerClusterStatus defines the observed state of GardenerCluster type GardenerClusterStatus struct { // State signifies current state of Gardener Cluster. @@ -86,6 +102,18 @@ type GardenerClusterStatus struct { Conditions []metav1.Condition `json:"conditions,omitempty"` } +func (cluster *GardenerCluster) UpdateState(r State, s State, msg string) { + cluster.Status.State = s + condition := metav1.Condition{ + Type: string(ConditionTypeUnknown), + Status: "True", + LastTransitionTime: metav1.Now(), + Reason: string(r), + Message: msg, + } + meta.SetStatusCondition(&cluster.Status.Conditions, condition) +} + func init() { SchemeBuilder.Register(&GardenerCluster{}, &GardenerClusterList{}) } diff --git a/config/samples/clusterinventory_v1_cluster.yaml b/config/samples/clusterinventory_v1_cluster.yaml deleted file mode 100644 index e69de29b..00000000 diff --git a/config/samples/clusterinventory_v1_gardenercluster.yaml b/config/samples/infrastructuremanager_v1_gardenercluster.yaml similarity index 84% rename from config/samples/clusterinventory_v1_gardenercluster.yaml rename to config/samples/infrastructuremanager_v1_gardenercluster.yaml index d571200b..07a4b959 100644 --- a/config/samples/clusterinventory_v1_gardenercluster.yaml +++ b/config/samples/infrastructuremanager_v1_gardenercluster.yaml @@ -1,4 +1,4 @@ -apiVersion: clusterinventory.kyma-project.io/v1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: GardenerCluster metadata: labels: @@ -6,7 +6,7 @@ metadata: kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name - kyma-project.io/global-account-id: global-account-id + kyma-project.io/global-account-id: global-account-i kyma-project.io/subaccount-id: subAccount-id kyma-project.io/shoot-name: shoot-name kyma-project.io/region: region diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 7193a072..010e9da4 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -80,7 +80,10 @@ func (r *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Requ err := r.Client.Get(ctx, req.NamespacedName, &cluster) if err != nil { + cluster.UpdateState(infrastructuremanagerv1.ErrorState, infrastructuremanagerv1.ConditionReasonGardenClusterNotRetrieved, "Couldn't retrieve the Garden Cluster CR") + if k8serrors.IsNotFound(err) { + cluster.UpdateState(infrastructuremanagerv1.DeletingState, infrastructuremanagerv1.ConditionReasonGardenClusterNotFound, "CR not found, secret will be deleted") err = r.deleteSecret(req.NamespacedName.Name) if err != nil { r.log.Error(err, "failed to delete secret") @@ -96,8 +99,10 @@ func (r *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Requ secret, err := r.getSecret(cluster.Spec.Shoot.Name) if err != nil { r.log.Error(err, "could not get the Secret for "+cluster.Spec.Shoot.Name) + cluster.UpdateState(infrastructuremanagerv1.ErrorState, infrastructuremanagerv1.ConditionReasonSecretNotFound, "Secret not found, and will be created") if !k8serrors.IsNotFound(err) { + return ctrl.Result{ Requeue: true, RequeueAfter: defaultRequeuInSeconds, @@ -107,14 +112,17 @@ func (r *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Requ if secret == nil { r.log.Error(err, "Secret not found, and will be created") + cluster.UpdateState(infrastructuremanagerv1.ErrorState, infrastructuremanagerv1.ConditionReasonSecretNotFound, "Secret not found, and will be created") err = r.createSecret(ctx, cluster) if err != nil { return r.ResultWithoutRequeue(), err } + } + cluster.UpdateState(infrastructuremanagerv1.ConditionReasonSecretCreated, infrastructuremanagerv1.ReadyState, "GardenCluster is ready") return ctrl.Result{}, nil } From 669527c563522a95c5c46b10ae6c4263a8f3bee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Tue, 26 Sep 2023 14:07:14 +0200 Subject: [PATCH 02/39] small renaming --- .../controller/gardener_cluster_controller.go | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 010e9da4..7c78d332 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -22,7 +22,7 @@ import ( "time" "github.com/go-logr/logr" - infrastructuremanagerv1 "github.com/kyma-project/infrastructure-manager/api/v1" + imv1 "github.com/kyma-project/infrastructure-manager/api/v1" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -73,20 +73,20 @@ type KubeconfigProvider interface { // // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.15.0/pkg/reconcile -func (r *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:revive - r.log.Info("Starting reconciliation loop") +func (controller *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:revive + controller.log.Info("Starting reconciliation loop") - var cluster infrastructuremanagerv1.GardenerCluster + var cluster imv1.GardenerCluster - err := r.Client.Get(ctx, req.NamespacedName, &cluster) + err := controller.Client.Get(ctx, req.NamespacedName, &cluster) if err != nil { - cluster.UpdateState(infrastructuremanagerv1.ErrorState, infrastructuremanagerv1.ConditionReasonGardenClusterNotRetrieved, "Couldn't retrieve the Garden Cluster CR") + cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonGardenClusterNotRetrieved, "Couldn't retrieve the Garden Cluster CR") if k8serrors.IsNotFound(err) { - cluster.UpdateState(infrastructuremanagerv1.DeletingState, infrastructuremanagerv1.ConditionReasonGardenClusterNotFound, "CR not found, secret will be deleted") - err = r.deleteSecret(req.NamespacedName.Name) + cluster.UpdateState(imv1.DeletingState, imv1.ConditionReasonGardenClusterNotFound, "CR not found, secret will be deleted") + err = controller.deleteSecret(req.NamespacedName.Name) if err != nil { - r.log.Error(err, "failed to delete secret") + controller.log.Error(err, "failed to delete secret") } } @@ -96,10 +96,10 @@ func (r *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Requ }, err } - secret, err := r.getSecret(cluster.Spec.Shoot.Name) + secret, err := controller.getSecret(cluster.Spec.Shoot.Name) if err != nil { - r.log.Error(err, "could not get the Secret for "+cluster.Spec.Shoot.Name) - cluster.UpdateState(infrastructuremanagerv1.ErrorState, infrastructuremanagerv1.ConditionReasonSecretNotFound, "Secret not found, and will be created") + controller.log.Error(err, "could not get the Secret for "+cluster.Spec.Shoot.Name) + cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonSecretNotFound, "Secret not found, and will be created") if !k8serrors.IsNotFound(err) { @@ -111,28 +111,28 @@ func (r *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Requ } if secret == nil { - r.log.Error(err, "Secret not found, and will be created") - cluster.UpdateState(infrastructuremanagerv1.ErrorState, infrastructuremanagerv1.ConditionReasonSecretNotFound, "Secret not found, and will be created") + controller.log.Error(err, "Secret not found, and will be created") + cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonSecretNotFound, "Secret not found, and will be created") - err = r.createSecret(ctx, cluster) + err = controller.createSecret(ctx, cluster) if err != nil { - return r.ResultWithoutRequeue(), err + return controller.ResultWithoutRequeue(), err } } - cluster.UpdateState(infrastructuremanagerv1.ConditionReasonSecretCreated, infrastructuremanagerv1.ReadyState, "GardenCluster is ready") + cluster.UpdateState(imv1.ConditionReasonSecretCreated, imv1.ReadyState, "GardenCluster is ready") return ctrl.Result{}, nil } -func (r *GardenerClusterController) deleteSecret(clusterCRName string) error { +func (controller *GardenerClusterController) deleteSecret(clusterCRName string) error { selector := client.MatchingLabels(map[string]string{ clusterCRNameLabel: clusterCRName, }) var secretList corev1.SecretList - err := r.Client.List(context.TODO(), &secretList, selector) + err := controller.Client.List(context.TODO(), &secretList, selector) if err != nil { return err } @@ -141,23 +141,23 @@ func (r *GardenerClusterController) deleteSecret(clusterCRName string) error { return errors.Errorf("unexpected numer of secrets found for cluster CR `%s`", clusterCRName) } - return r.Client.Delete(context.TODO(), &secretList.Items[0]) + return controller.Client.Delete(context.TODO(), &secretList.Items[0]) } -func (r *GardenerClusterController) ResultWithoutRequeue() ctrl.Result { +func (controller *GardenerClusterController) ResultWithoutRequeue() ctrl.Result { return ctrl.Result{ Requeue: false, } } -func (r *GardenerClusterController) getSecret(shootName string) (*corev1.Secret, error) { +func (controller *GardenerClusterController) getSecret(shootName string) (*corev1.Secret, error) { var secretList corev1.SecretList shootNameSelector := client.MatchingLabels(map[string]string{ "kyma-project.io/shoot-name": shootName, }) - err := r.Client.List(context.Background(), &secretList, shootNameSelector) + err := controller.Client.List(context.Background(), &secretList, shootNameSelector) if err != nil { return nil, err } @@ -175,16 +175,16 @@ func (r *GardenerClusterController) getSecret(shootName string) (*corev1.Secret, return &secretList.Items[0], nil } -func (r *GardenerClusterController) createSecret(ctx context.Context, cluster infrastructuremanagerv1.GardenerCluster) error { - secret, err := r.newSecret(cluster) +func (controller *GardenerClusterController) createSecret(ctx context.Context, cluster imv1.GardenerCluster) error { + secret, err := controller.newSecret(cluster) if err != nil { return err } - return r.Client.Create(ctx, &secret) + return controller.Client.Create(ctx, &secret) } -func (r *GardenerClusterController) newSecret(cluster infrastructuremanagerv1.GardenerCluster) (corev1.Secret, error) { +func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerCluster) (corev1.Secret, error) { labels := map[string]string{} for key, val := range cluster.Labels { @@ -193,7 +193,7 @@ func (r *GardenerClusterController) newSecret(cluster infrastructuremanagerv1.Ga labels["operator.kyma-project.io/managed-by"] = "infrastructure-manager" labels[clusterCRNameLabel] = cluster.Name - kubeconfig, err := r.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) + kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) if err != nil { return corev1.Secret{}, err } @@ -210,8 +210,8 @@ func (r *GardenerClusterController) newSecret(cluster infrastructuremanagerv1.Ga } // SetupWithManager sets up the controller with the Manager. -func (r *GardenerClusterController) SetupWithManager(mgr ctrl.Manager) error { +func (controller *GardenerClusterController) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&infrastructuremanagerv1.GardenerCluster{}). - Complete(r) + For(&imv1.GardenerCluster{}). + Complete(controller) } From 547a9d2b5bf41ecc7298fbe7d963e066f2181113 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Tue, 26 Sep 2023 15:46:13 +0200 Subject: [PATCH 03/39] [WiP] attempt to update CR status --- internal/controller/gardener_cluster_controller.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 7c78d332..024ab50c 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -19,8 +19,6 @@ package controller import ( "context" "fmt" - "time" - "github.com/go-logr/logr" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" "github.com/pkg/errors" @@ -31,6 +29,8 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "time" ) const ( @@ -119,10 +119,13 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if err != nil { return controller.ResultWithoutRequeue(), err } - } cluster.UpdateState(imv1.ConditionReasonSecretCreated, imv1.ReadyState, "GardenCluster is ready") + + err = controller.Client.Update(ctx, &cluster, &client.UpdateOptions{ + FieldManager: "infrastructure-manager", + }) return ctrl.Result{}, nil } @@ -213,5 +216,10 @@ func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerClus func (controller *GardenerClusterController) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&imv1.GardenerCluster{}). + WithEventFilter(predicate.Or( + predicate.GenerationChangedPredicate{}, + predicate.LabelChangedPredicate{}, + predicate.AnnotationChangedPredicate{}, + )). Complete(controller) } From b463c9d50a371b6bdad368834adcab1f8b7af0b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 27 Sep 2023 07:50:23 +0200 Subject: [PATCH 04/39] [WiP] attempt to update CR status --- ...rastructuremanager_v1_gardenercluster.yaml | 8 ++--- .../controller/gardener_cluster_controller.go | 33 ++++++++++++------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/config/samples/infrastructuremanager_v1_gardenercluster.yaml b/config/samples/infrastructuremanager_v1_gardenercluster.yaml index 07a4b959..9da13a90 100644 --- a/config/samples/infrastructuremanager_v1_gardenercluster.yaml +++ b/config/samples/infrastructuremanager_v1_gardenercluster.yaml @@ -4,11 +4,11 @@ metadata: labels: kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id - kyma-project.io/broker-plan-id: plan-id - kyma-project.io/broker-plan-name: plan-name + kyma-project.io/broker-plan-id: plan-id2 + kyma-project.io/broker-plan-name: plan-name2 kyma-project.io/global-account-id: global-account-i - kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/subaccount-id: subAccount-id2 + kyma-project.io/shoot-name: shoot-name2 kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName name: runtime-id diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 024ab50c..3e589f27 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "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/predicate" "time" @@ -84,6 +85,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if k8serrors.IsNotFound(err) { cluster.UpdateState(imv1.DeletingState, imv1.ConditionReasonGardenClusterNotFound, "CR not found, secret will be deleted") + err = controller.deleteSecret(req.NamespacedName.Name) if err != nil { controller.log.Error(err, "failed to delete secret") @@ -93,20 +95,20 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req return ctrl.Result{ Requeue: true, RequeueAfter: defaultRequeuInSeconds, - }, err + }, controller.persistChange(ctx, cluster) } secret, err := controller.getSecret(cluster.Spec.Shoot.Name) if err != nil { controller.log.Error(err, "could not get the Secret for "+cluster.Spec.Shoot.Name) - cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonSecretNotFound, "Secret not found, and will be created") if !k8serrors.IsNotFound(err) { + cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonSecretNotFound, "Secret not found") return ctrl.Result{ Requeue: true, RequeueAfter: defaultRequeuInSeconds, - }, err + }, controller.persistChange(ctx, cluster) } } @@ -117,16 +119,20 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req err = controller.createSecret(ctx, cluster) if err != nil { - return controller.ResultWithoutRequeue(), err + cluster.UpdateState(imv1.ReadyState, imv1.ConditionReasonSecretCreated, "Secret has been created") + return controller.ResultWithoutRequeue(), controller.persistChange(ctx, cluster) } } cluster.UpdateState(imv1.ConditionReasonSecretCreated, imv1.ReadyState, "GardenCluster is ready") - err = controller.Client.Update(ctx, &cluster, &client.UpdateOptions{ + return ctrl.Result{}, controller.persistChange(ctx, cluster) +} + +func (controller *GardenerClusterController) persistChange(ctx context.Context, cluster imv1.GardenerCluster) error { + return controller.Client.Update(ctx, &cluster, &client.UpdateOptions{ FieldManager: "infrastructure-manager", }) - return ctrl.Result{}, nil } func (controller *GardenerClusterController) deleteSecret(clusterCRName string) error { @@ -214,12 +220,17 @@ func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerClus // SetupWithManager sets up the controller with the Manager. func (controller *GardenerClusterController) SetupWithManager(mgr ctrl.Manager) error { + omitStatusChanged := predicate.Or( + predicate.GenerationChangedPredicate{}, + predicate.LabelChangedPredicate{}, + predicate.AnnotationChangedPredicate{}, + ) + return ctrl.NewControllerManagedBy(mgr). - For(&imv1.GardenerCluster{}). - WithEventFilter(predicate.Or( - predicate.GenerationChangedPredicate{}, - predicate.LabelChangedPredicate{}, - predicate.AnnotationChangedPredicate{}, + For(&imv1.GardenerCluster{}, builder.WithPredicates( + predicate.And( + predicate.ResourceVersionChangedPredicate{}, + omitStatusChanged), )). Complete(controller) } From 79a652eeee9f2d7424d4016c19daa9f704bce0a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 27 Sep 2023 09:22:02 +0200 Subject: [PATCH 05/39] fix linter + another take on persisting CR changes --- .../controller/gardener_cluster_controller.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 3e589f27..3bcb90ef 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -19,6 +19,8 @@ package controller import ( "context" "fmt" + "time" + "github.com/go-logr/logr" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" "github.com/pkg/errors" @@ -31,7 +33,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/predicate" - "time" ) const ( @@ -95,7 +96,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req return ctrl.Result{ Requeue: true, RequeueAfter: defaultRequeuInSeconds, - }, controller.persistChange(ctx, cluster) + }, controller.persistChange(ctx, &cluster) } secret, err := controller.getSecret(cluster.Spec.Shoot.Name) @@ -108,7 +109,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req return ctrl.Result{ Requeue: true, RequeueAfter: defaultRequeuInSeconds, - }, controller.persistChange(ctx, cluster) + }, controller.persistChange(ctx, &cluster) } } @@ -120,19 +121,18 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if err != nil { cluster.UpdateState(imv1.ReadyState, imv1.ConditionReasonSecretCreated, "Secret has been created") - return controller.ResultWithoutRequeue(), controller.persistChange(ctx, cluster) + return controller.ResultWithoutRequeue(), controller.persistChange(ctx, &cluster) } } cluster.UpdateState(imv1.ConditionReasonSecretCreated, imv1.ReadyState, "GardenCluster is ready") - return ctrl.Result{}, controller.persistChange(ctx, cluster) + return ctrl.Result{}, controller.persistChange(ctx, &cluster) } -func (controller *GardenerClusterController) persistChange(ctx context.Context, cluster imv1.GardenerCluster) error { - return controller.Client.Update(ctx, &cluster, &client.UpdateOptions{ - FieldManager: "infrastructure-manager", - }) +func (controller *GardenerClusterController) persistChange(ctx context.Context, cluster *imv1.GardenerCluster) error { + err := controller.Client.Update(ctx, cluster) + return err } func (controller *GardenerClusterController) deleteSecret(clusterCRName string) error { From c6382ae56e3afe40fde890b92be6a70ea6401127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 27 Sep 2023 11:24:10 +0200 Subject: [PATCH 06/39] CR status are now persisted --- .../controller/gardener_cluster_controller.go | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 3bcb90ef..903091ce 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -93,10 +93,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req } } - return ctrl.Result{ - Requeue: true, - RequeueAfter: defaultRequeuInSeconds, - }, controller.persistChange(ctx, &cluster) + return controller.resultWithRequeue(), controller.persistChange(ctx, &cluster) } secret, err := controller.getSecret(cluster.Spec.Shoot.Name) @@ -106,10 +103,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if !k8serrors.IsNotFound(err) { cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonSecretNotFound, "Secret not found") - return ctrl.Result{ - Requeue: true, - RequeueAfter: defaultRequeuInSeconds, - }, controller.persistChange(ctx, &cluster) + return controller.resultWithRequeue(), controller.persistChange(ctx, &cluster) } } @@ -121,17 +115,30 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if err != nil { cluster.UpdateState(imv1.ReadyState, imv1.ConditionReasonSecretCreated, "Secret has been created") - return controller.ResultWithoutRequeue(), controller.persistChange(ctx, &cluster) + return controller.resultWithoutRequeue(), controller.persistChange(ctx, &cluster) } } cluster.UpdateState(imv1.ConditionReasonSecretCreated, imv1.ReadyState, "GardenCluster is ready") - return ctrl.Result{}, controller.persistChange(ctx, &cluster) + return controller.resultWithRequeue(), controller.persistChange(ctx, &cluster) +} + +func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { + return ctrl.Result{ + Requeue: true, + RequeueAfter: defaultRequeuInSeconds, + } +} + +func (controller *GardenerClusterController) resultWithoutRequeue() ctrl.Result { + return ctrl.Result{ + Requeue: false, + } } func (controller *GardenerClusterController) persistChange(ctx context.Context, cluster *imv1.GardenerCluster) error { - err := controller.Client.Update(ctx, cluster) + err := controller.Client.Status().Update(ctx, cluster) return err } @@ -153,12 +160,6 @@ func (controller *GardenerClusterController) deleteSecret(clusterCRName string) return controller.Client.Delete(context.TODO(), &secretList.Items[0]) } -func (controller *GardenerClusterController) ResultWithoutRequeue() ctrl.Result { - return ctrl.Result{ - Requeue: false, - } -} - func (controller *GardenerClusterController) getSecret(shootName string) (*corev1.Secret, error) { var secretList corev1.SecretList From ee24eb291aa638dc0feb44b56ec435e4783b88b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 27 Sep 2023 11:27:20 +0200 Subject: [PATCH 07/39] renames persistChange to persistStatusChange --- internal/controller/gardener_cluster_controller.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 903091ce..52d26cd0 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -93,7 +93,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req } } - return controller.resultWithRequeue(), controller.persistChange(ctx, &cluster) + return controller.resultWithRequeue(), controller.persistStatusChange(ctx, &cluster) } secret, err := controller.getSecret(cluster.Spec.Shoot.Name) @@ -103,7 +103,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if !k8serrors.IsNotFound(err) { cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonSecretNotFound, "Secret not found") - return controller.resultWithRequeue(), controller.persistChange(ctx, &cluster) + return controller.resultWithRequeue(), controller.persistStatusChange(ctx, &cluster) } } @@ -115,13 +115,13 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if err != nil { cluster.UpdateState(imv1.ReadyState, imv1.ConditionReasonSecretCreated, "Secret has been created") - return controller.resultWithoutRequeue(), controller.persistChange(ctx, &cluster) + return controller.resultWithoutRequeue(), controller.persistStatusChange(ctx, &cluster) } } cluster.UpdateState(imv1.ConditionReasonSecretCreated, imv1.ReadyState, "GardenCluster is ready") - return controller.resultWithRequeue(), controller.persistChange(ctx, &cluster) + return controller.resultWithRequeue(), controller.persistStatusChange(ctx, &cluster) } func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { @@ -137,7 +137,7 @@ func (controller *GardenerClusterController) resultWithoutRequeue() ctrl.Result } } -func (controller *GardenerClusterController) persistChange(ctx context.Context, cluster *imv1.GardenerCluster) error { +func (controller *GardenerClusterController) persistStatusChange(ctx context.Context, cluster *imv1.GardenerCluster) error { err := controller.Client.Status().Update(ctx, cluster) return err } From 5598c1a80d3cda71a99f32e336d6ca31b81650a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 27 Sep 2023 11:29:22 +0200 Subject: [PATCH 08/39] small clean-up on gardenercluster CR sample --- .../infrastructuremanager_v1_gardenercluster.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config/samples/infrastructuremanager_v1_gardenercluster.yaml b/config/samples/infrastructuremanager_v1_gardenercluster.yaml index 9da13a90..a699ef82 100644 --- a/config/samples/infrastructuremanager_v1_gardenercluster.yaml +++ b/config/samples/infrastructuremanager_v1_gardenercluster.yaml @@ -4,11 +4,11 @@ metadata: labels: kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id - kyma-project.io/broker-plan-id: plan-id2 - kyma-project.io/broker-plan-name: plan-name2 - kyma-project.io/global-account-id: global-account-i - kyma-project.io/subaccount-id: subAccount-id2 - kyma-project.io/shoot-name: shoot-name2 + kyma-project.io/broker-plan-id: plan-id + kyma-project.io/broker-plan-name: plan-name + kyma-project.io/global-account-id: global-account-id + kyma-project.io/subaccount-id: subAccount-id + kyma-project.io/shoot-name: shoot-name kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName name: runtime-id From d075d8a078d742f39334def7cf907c13ebf64dd7 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 13 Oct 2023 10:15:06 +0200 Subject: [PATCH 09/39] Fixes to statuses implementation --- api/v1/gardenercluster_types.go | 85 ++++++++++++++++--- .../controller/gardener_cluster_controller.go | 34 ++++---- 2 files changed, 89 insertions(+), 30 deletions(-) diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go index cd66c895..4e83d6c9 100644 --- a/api/v1/gardenercluster_types.go +++ b/api/v1/gardenercluster_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1 import ( + "fmt" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -71,22 +72,21 @@ const ( ReadyState State = "Ready" ProcessingState State = "Processing" ErrorState State = "Error" - DeletingState State = "Deleting" ) type ConditionReason string const ( - ConditionReasonSecretCreated State = "SecretCreated" - ConditionReasonSecretNotFound State = "SecretNotFound" - ConditionReasonGardenClusterNotRetrieved State = "GardenClusterNotRetrieved" - ConditionReasonGardenClusterNotFound State = "GardenClusterNotFound" + ConditionReasonKubeconfigReadingSecret ConditionReason = "KubeconfigReadingSecret" + ConditionReasonKubeconfigSecretReady ConditionReason = "KubeconfigSecretReady" + ConditionReasonFailedToCheckSecret ConditionReason = "FailedToCheckSecret" + ConditionReasonFailedToCreateSecret ConditionReason = "FailedToCreateSecret" ) type ConditionType string const ( - ConditionTypeUnknown State = "Unknown" + ConditionTypeKubeconfigManagement ConditionType = "KubeconfigManagement" ) // GardenerClusterStatus defines the observed state of GardenerCluster @@ -102,18 +102,79 @@ type GardenerClusterStatus struct { Conditions []metav1.Condition `json:"conditions,omitempty"` } -func (cluster *GardenerCluster) UpdateState(r State, s State, msg string) { - cluster.Status.State = s +func (cluster *GardenerCluster) UpdateConditionForProcessingState(conditionType ConditionType, reason ConditionReason, conditionStatus metav1.ConditionStatus) { + cluster.Status.State = ProcessingState + + condition := metav1.Condition{ + Type: string(conditionType), + Status: conditionStatus, + LastTransitionTime: metav1.Now(), + Reason: string(reason), + Message: getMessage(reason), + } + meta.SetStatusCondition(&cluster.Status.Conditions, condition) +} + +func (cluster *GardenerCluster) UpdateConditionForReadyState(conditionType ConditionType, reason ConditionReason, conditionStatus metav1.ConditionStatus) { + cluster.Status.State = ReadyState + condition := metav1.Condition{ - Type: string(ConditionTypeUnknown), - Status: "True", + Type: string(conditionType), + Status: conditionStatus, LastTransitionTime: metav1.Now(), - Reason: string(r), - Message: msg, + Reason: string(reason), + Message: getMessage(reason), } meta.SetStatusCondition(&cluster.Status.Conditions, condition) } +func (cluster *GardenerCluster) UpdateConditionForErrorState(conditionType ConditionType, reason ConditionReason, conditionStatus metav1.ConditionStatus, error error) { + cluster.Status.State = ErrorState + + condition := metav1.Condition{ + Type: string(conditionType), + Status: conditionStatus, + LastTransitionTime: metav1.Now(), + Reason: string(reason), + Message: fmt.Sprintf("%s Error: %s", getMessage(reason), error.Error()), + } + meta.SetStatusCondition(&cluster.Status.Conditions, condition) +} + +func getMessage(reason ConditionReason) string { + switch reason { + case ConditionReasonKubeconfigSecretReady: + return "Secret created successfully." + case ConditionReasonFailedToCheckSecret: + return "Failed to get secret." + default: + return "Unknown condition" + } +} + +// +//conditions: +//- lastTransitionTime: "2023-04-17T17:57:36Z" +//lastUpdateTime: "2023-08-28T08:41:32Z" +//message: ReplicaSet "faas-app-f6f5cc65b" has successfully progressed. +//reason: NewReplicaSetAvailable +//status: "True" +//type: Progressing +//- lastTransitionTime: "2023-10-05T23:16:21Z" +//lastUpdateTime: "2023-10-05T23:16:21Z" +//message: Deployment has minimum availability. +//reason: MinimumReplicasAvailable +//status: "True" +//func (kyma *Kyma) UpdateCondition(conditionType KymaConditionType, status metav1.ConditionStatus) { +// meta.SetStatusCondition(&kyma.Status.Conditions, metav1.Condition{ +// Type: string(conditionType), +// Status: status, +// Reason: string(ConditionReason), +// Message: GenerateMessage(conditionType, status), +// ObservedGeneration: kyma.GetGeneration(), +// }) +//} + func init() { SchemeBuilder.Register(&GardenerCluster{}, &GardenerClusterList{}) } diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 52d26cd0..9acef6db 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -82,44 +82,43 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req err := controller.Client.Get(ctx, req.NamespacedName, &cluster) if err != nil { - cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonGardenClusterNotRetrieved, "Couldn't retrieve the Garden Cluster CR") - if k8serrors.IsNotFound(err) { - cluster.UpdateState(imv1.DeletingState, imv1.ConditionReasonGardenClusterNotFound, "CR not found, secret will be deleted") - err = controller.deleteSecret(req.NamespacedName.Name) if err != nil { controller.log.Error(err, "failed to delete secret") } } - return controller.resultWithRequeue(), controller.persistStatusChange(ctx, &cluster) + return controller.resultWithoutRequeue(), err } + cluster.UpdateConditionForProcessingState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigReadingSecret, metav1.ConditionTrue) + secret, err := controller.getSecret(cluster.Spec.Shoot.Name) if err != nil { - controller.log.Error(err, "could not get the Secret for "+cluster.Spec.Shoot.Name) - if !k8serrors.IsNotFound(err) { - cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonSecretNotFound, "Secret not found") - - return controller.resultWithRequeue(), controller.persistStatusChange(ctx, &cluster) + controller.log.Error(err, "failed to get the Secret for "+cluster.Spec.Shoot.Name) + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCheckSecret, metav1.ConditionTrue, err) + if err := controller.persistStatusChange(ctx, &cluster); err != nil { + controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) + } + return controller.resultWithoutRequeue(), err } } if secret == nil { - controller.log.Error(err, "Secret not found, and will be created") - cluster.UpdateState(imv1.ErrorState, imv1.ConditionReasonSecretNotFound, "Secret not found, and will be created") - err = controller.createSecret(ctx, cluster) if err != nil { - cluster.UpdateState(imv1.ReadyState, imv1.ConditionReasonSecretCreated, "Secret has been created") - return controller.resultWithoutRequeue(), controller.persistStatusChange(ctx, &cluster) + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, metav1.ConditionTrue, err) + if err := controller.persistStatusChange(ctx, &cluster); err != nil { + controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) + } + return controller.resultWithoutRequeue(), err } } - cluster.UpdateState(imv1.ConditionReasonSecretCreated, imv1.ReadyState, "GardenCluster is ready") + cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) return controller.resultWithRequeue(), controller.persistStatusChange(ctx, &cluster) } @@ -138,8 +137,7 @@ func (controller *GardenerClusterController) resultWithoutRequeue() ctrl.Result } func (controller *GardenerClusterController) persistStatusChange(ctx context.Context, cluster *imv1.GardenerCluster) error { - err := controller.Client.Status().Update(ctx, cluster) - return err + return controller.Client.Status().Update(ctx, cluster) } func (controller *GardenerClusterController) deleteSecret(clusterCRName string) error { From ed4135b9596983a3271c5118c4116aeff930fe99 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 13 Oct 2023 10:46:32 +0200 Subject: [PATCH 10/39] Minor clean up --- api/v1/gardenercluster_types.go | 23 ------------------- .../controller/gardener_cluster_controller.go | 2 +- 2 files changed, 1 insertion(+), 24 deletions(-) diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go index 4e83d6c9..0e71bef2 100644 --- a/api/v1/gardenercluster_types.go +++ b/api/v1/gardenercluster_types.go @@ -152,29 +152,6 @@ func getMessage(reason ConditionReason) string { } } -// -//conditions: -//- lastTransitionTime: "2023-04-17T17:57:36Z" -//lastUpdateTime: "2023-08-28T08:41:32Z" -//message: ReplicaSet "faas-app-f6f5cc65b" has successfully progressed. -//reason: NewReplicaSetAvailable -//status: "True" -//type: Progressing -//- lastTransitionTime: "2023-10-05T23:16:21Z" -//lastUpdateTime: "2023-10-05T23:16:21Z" -//message: Deployment has minimum availability. -//reason: MinimumReplicasAvailable -//status: "True" -//func (kyma *Kyma) UpdateCondition(conditionType KymaConditionType, status metav1.ConditionStatus) { -// meta.SetStatusCondition(&kyma.Status.Conditions, metav1.Condition{ -// Type: string(conditionType), -// Status: status, -// Reason: string(ConditionReason), -// Message: GenerateMessage(conditionType, status), -// ObservedGeneration: kyma.GetGeneration(), -// }) -//} - func init() { SchemeBuilder.Register(&GardenerCluster{}, &GardenerClusterList{}) } diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index fcae064f..b7725973 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -76,7 +76,7 @@ type KubeconfigProvider interface { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.15.0/pkg/reconcile func (controller *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:revive - r.log.Info(fmt.Sprintf("Starting reconciliation loop for GardenerCluster resource: %v", req.NamespacedName)) + controller.log.Info(fmt.Sprintf("Starting reconciliation loop for GardenerCluster resource: %v", req.NamespacedName)) var cluster imv1.GardenerCluster From fd2cdcef2a8e2b49a08d00469fafd2709af2e8e2 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 13 Oct 2023 11:26:39 +0200 Subject: [PATCH 11/39] Linter issue fixed --- api/v1/gardenercluster_types.go | 1 + internal/controller/gardener_cluster_controller.go | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go index 0e71bef2..1ebd9ee3 100644 --- a/api/v1/gardenercluster_types.go +++ b/api/v1/gardenercluster_types.go @@ -18,6 +18,7 @@ package v1 import ( "fmt" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index b7725973..e2db8830 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -99,8 +99,10 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if !k8serrors.IsNotFound(err) { controller.log.Error(err, "failed to get the Secret for "+cluster.Spec.Shoot.Name) cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCheckSecret, metav1.ConditionTrue, err) - if err := controller.persistStatusChange(ctx, &cluster); err != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) + { + if err := controller.persistStatusChange(ctx, &cluster); err != nil { + controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) + } } return controller.resultWithoutRequeue(), err } @@ -111,8 +113,10 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if err != nil { cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, metav1.ConditionTrue, err) - if err := controller.persistStatusChange(ctx, &cluster); err != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) + { + if err := controller.persistStatusChange(ctx, &cluster); err != nil { + controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) + } } return controller.resultWithoutRequeue(), err } From d1196df1853539e834d5939da24d3aade70ede08 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 13 Oct 2023 11:32:05 +0200 Subject: [PATCH 12/39] Linter issue fixed --- internal/controller/gardener_cluster_controller.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index e2db8830..cea5ab50 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -99,10 +99,8 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if !k8serrors.IsNotFound(err) { controller.log.Error(err, "failed to get the Secret for "+cluster.Spec.Shoot.Name) cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCheckSecret, metav1.ConditionTrue, err) - { - if err := controller.persistStatusChange(ctx, &cluster); err != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) - } + if controller.persistStatusChange(ctx, &cluster) != nil { + controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) } return controller.resultWithoutRequeue(), err } @@ -113,10 +111,8 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if err != nil { cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, metav1.ConditionTrue, err) - { - if err := controller.persistStatusChange(ctx, &cluster); err != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) - } + if controller.persistStatusChange(ctx, &cluster) != nil { + controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) } return controller.resultWithoutRequeue(), err } From 5276a36e3ced0d27051d9d831c3071e89bc52224 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Thu, 19 Oct 2023 06:49:50 +0200 Subject: [PATCH 13/39] Rotation implemented --- api/v1/gardenercluster_types.go | 6 +- cmd/main.go | 2 +- config/rbac/role.yaml | 6 ++ .../controller/gardener_cluster_controller.go | 100 ++++++++++-------- internal/controller/predicates.go | 64 +++++++++++ internal/controller/suite_test.go | 3 +- 6 files changed, 131 insertions(+), 50 deletions(-) create mode 100644 internal/controller/predicates.go diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go index 1ebd9ee3..9a351f1d 100644 --- a/api/v1/gardenercluster_types.go +++ b/api/v1/gardenercluster_types.go @@ -80,8 +80,10 @@ type ConditionReason string const ( ConditionReasonKubeconfigReadingSecret ConditionReason = "KubeconfigReadingSecret" ConditionReasonKubeconfigSecretReady ConditionReason = "KubeconfigSecretReady" - ConditionReasonFailedToCheckSecret ConditionReason = "FailedToCheckSecret" + ConditionReasonFailedToGetSecret ConditionReason = "FailedToCheckSecret" ConditionReasonFailedToCreateSecret ConditionReason = "FailedToCreateSecret" + ConditionReasonFailedToUpdateSecret ConditionReason = "FailedToUpdateSecret" + ConditionReasonFailedToGetKubeconfig ConditionReason = "FailedToGetKubeconfig" ) type ConditionType string @@ -146,7 +148,7 @@ func getMessage(reason ConditionReason) string { switch reason { case ConditionReasonKubeconfigSecretReady: return "Secret created successfully." - case ConditionReasonFailedToCheckSecret: + case ConditionReasonFailedToGetSecret: return "Failed to get secret." default: return "Unknown condition" diff --git a/cmd/main.go b/cmd/main.go index 6004474b..7c3ce0ba 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -114,7 +114,7 @@ func main() { os.Exit(1) } - if err = (controller.NewGardenerClusterController(mgr, kubeconfigProvider, logger)).SetupWithManager(mgr); err != nil { + if err = (controller.NewGardenerClusterController(mgr, kubeconfigProvider, logger, expirationTime)).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "GardenerCluster") os.Exit(1) } diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 0f9269f5..86cf5fbc 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -33,3 +33,9 @@ rules: - gardenerclusters/finalizers verbs: - update +- apiGroups: + - infrastructuremanager.kyma-project.io + resources: + - gardenerclusters/status + verbs: + - update diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index cea5ab50..e9e085da 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -32,13 +32,15 @@ import ( 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/predicate" ) const ( - lastKubeconfigSyncAnnotation = "operator.kyma-project.io/last-sync" - clusterCRNameLabel = "operator.kyma-project.io/cluster-name" - defaultRequeuInSeconds = 60 * 1000 + lastKubeconfigSyncAnnotation = "operator.kyma-project.io/last-sync" + forceKubeconfigRotationAnnotation = "operator.kyma-project.io/force-kubeconfig-rotation" + clusterCRNameLabel = "operator.kyma-project.io/cluster-name" + // The ratio determines what is the minimal time that needs to pass to rotate certificate. + minimalRotationTimeRatio = 0.6 + defaultRequeuInSeconds = 60 * 1000 ) // GardenerClusterController reconciles a GardenerCluster object @@ -47,14 +49,18 @@ type GardenerClusterController struct { Scheme *runtime.Scheme KubeconfigProvider KubeconfigProvider log logr.Logger + requeueAfter time.Duration } -func NewGardenerClusterController(mgr ctrl.Manager, kubeconfigProvider KubeconfigProvider, logger logr.Logger) *GardenerClusterController { +func NewGardenerClusterController(mgr ctrl.Manager, kubeconfigProvider KubeconfigProvider, logger logr.Logger, kubeconfigExpirationTime time.Duration) *GardenerClusterController { + requeueTimeInMinutes := int64(minimalRotationTimeRatio * kubeconfigExpirationTime.Minutes()) + return &GardenerClusterController{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), KubeconfigProvider: kubeconfigProvider, log: logger, + requeueAfter: time.Duration(requeueTimeInMinutes) * time.Minute, } } @@ -66,6 +72,7 @@ type KubeconfigProvider interface { //+kubebuilder:rbac:groups=infrastructuremanager.kyma-project.io,resources=gardenerclusters,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;delete //+kubebuilder:rbac:groups=infrastructuremanager.kyma-project.io,resources=gardenerclusters/finalizers,verbs=update +//+kubebuilder:rbac:groups=infrastructuremanager.kyma-project.io,resources=gardenerclusters/status,verbs=update // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -94,28 +101,12 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req cluster.UpdateConditionForProcessingState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigReadingSecret, metav1.ConditionTrue) - secret, err := controller.getSecret(cluster.Spec.Shoot.Name) + err = controller.createOrRotateSecret(ctx, cluster) if err != nil { - if !k8serrors.IsNotFound(err) { - controller.log.Error(err, "failed to get the Secret for "+cluster.Spec.Shoot.Name) - cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCheckSecret, metav1.ConditionTrue, err) - if controller.persistStatusChange(ctx, &cluster) != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) - } - return controller.resultWithoutRequeue(), err - } - } - - if secret == nil { - err = controller.createSecret(ctx, cluster) - - if err != nil { - cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, metav1.ConditionTrue, err) - if controller.persistStatusChange(ctx, &cluster) != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) - } - return controller.resultWithoutRequeue(), err + if controller.persistStatusChange(ctx, &cluster) != nil { + controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) } + return controller.resultWithoutRequeue(), err } cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) @@ -126,7 +117,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { return ctrl.Result{ Requeue: true, - RequeueAfter: defaultRequeuInSeconds, + RequeueAfter: controller.requeueAfter, } } @@ -183,16 +174,46 @@ func (controller *GardenerClusterController) getSecret(shootName string) (*corev return &secretList.Items[0], nil } -func (controller *GardenerClusterController) createSecret(ctx context.Context, cluster imv1.GardenerCluster) error { - secret, err := controller.newSecret(cluster) +func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Context, cluster imv1.GardenerCluster) error { + kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) if err != nil { + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, metav1.ConditionTrue, err) + return err + } + + lastSyncTime := time.Now() + newSecret := controller.newSecret(cluster, kubeconfig, lastSyncTime) + + err = controller.Client.Create(ctx, &newSecret) + if err != nil { + if k8serrors.IsAlreadyExists(err) { + existingSecret, err := controller.getSecret(cluster.Spec.Shoot.Name) + if err != nil { + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) + return err + } + + existingSecret.Data[cluster.Spec.Kubeconfig.Secret.Key] = []byte(kubeconfig) + existingSecret.GetAnnotations()[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().String() + + err = controller.Client.Update(ctx, existingSecret) + if err != nil { + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToUpdateSecret, metav1.ConditionTrue, err) + return err + } + + return nil + } + + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, metav1.ConditionTrue, err) + return err } - return controller.Client.Create(ctx, &secret) + return nil } -func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerCluster) (corev1.Secret, error) { +func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerCluster, kubeconfig string, lastSyncTime time.Time) corev1.Secret { labels := map[string]string{} for key, val := range cluster.Labels { @@ -201,35 +222,22 @@ func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerClus labels["operator.kyma-project.io/managed-by"] = "infrastructure-manager" labels[clusterCRNameLabel] = cluster.Name - kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) - if err != nil { - return corev1.Secret{}, err - } - return corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: cluster.Spec.Kubeconfig.Secret.Name, Namespace: cluster.Spec.Kubeconfig.Secret.Namespace, Labels: labels, - Annotations: map[string]string{lastKubeconfigSyncAnnotation: time.Now().UTC().String()}, + Annotations: map[string]string{lastKubeconfigSyncAnnotation: lastSyncTime.UTC().String()}, }, StringData: map[string]string{cluster.Spec.Kubeconfig.Secret.Key: kubeconfig}, - }, nil + } } // SetupWithManager sets up the controller with the Manager. func (controller *GardenerClusterController) SetupWithManager(mgr ctrl.Manager) error { - omitStatusChanged := predicate.Or( - predicate.GenerationChangedPredicate{}, - predicate.LabelChangedPredicate{}, - predicate.AnnotationChangedPredicate{}, - ) - return ctrl.NewControllerManagedBy(mgr). For(&imv1.GardenerCluster{}, builder.WithPredicates( - predicate.And( - predicate.ResourceVersionChangedPredicate{}, - omitStatusChanged), + newRotationNotNeededPredicate(controller.requeueAfter), )). Complete(controller) } diff --git a/internal/controller/predicates.go b/internal/controller/predicates.go new file mode 100644 index 00000000..86b75100 --- /dev/null +++ b/internal/controller/predicates.go @@ -0,0 +1,64 @@ +package controller + +import ( + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "time" +) + +func newRotationNotNeededPredicate(rotationPeriod time.Duration) predicate.Predicate { + return RotationNotNeededPredicate{ + rotationPeriod: rotationPeriod, + } +} + +type RotationNotNeededPredicate struct { + rotationPeriod time.Duration +} + +func (RotationNotNeededPredicate) Create(event.CreateEvent) bool { + return true +} + +func (r RotationNotNeededPredicate) Update(e event.UpdateEvent) bool { + return kubeconfigMustBeRotated(e.ObjectNew, r.rotationPeriod) +} + +func (RotationNotNeededPredicate) Delete(event.DeleteEvent) bool { + return true +} + +func (r RotationNotNeededPredicate) Generic(e event.GenericEvent) bool { + return kubeconfigMustBeRotated(e.Object, r.rotationPeriod) +} + +func kubeconfigMustBeRotated(gardenerCluster client.Object, validityTime time.Duration) bool { + return rotationForced(gardenerCluster) || rotationTimePassed(gardenerCluster, validityTime) +} + +func rotationTimePassed(gardenerCluster client.Object, rotationPeriod time.Duration) bool { + annotations := gardenerCluster.GetAnnotations() + + _, found := annotations[lastKubeconfigSyncAnnotation] + + if !found { + return true + } + + lastSyncTimeString := annotations[lastKubeconfigSyncAnnotation] + lastSyncTime, err := time.Parse(time.RFC3339, lastSyncTimeString) + if err != nil { + return true + } + now := time.Now() + alreadyValidFor := lastSyncTime.Sub(now) + + return alreadyValidFor.Minutes() >= rotationPeriod.Minutes() +} + +func rotationForced(gardenerCluster client.Object) bool { + _, found := gardenerCluster.GetAnnotations()[forceKubeconfigRotationAnnotation] + + return found +} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 8fd56f39..6df69aa3 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -20,6 +20,7 @@ import ( "context" "path/filepath" "testing" + "time" infrastructuremanagerv1 "github.com/kyma-project/infrastructure-manager/api/v1" "github.com/kyma-project/infrastructure-manager/internal/controller/mocks" @@ -76,7 +77,7 @@ var _ = BeforeSuite(func() { kubeconfigProviderMock := &mocks.KubeconfigProvider{} setupKubeconfigProviderMock(kubeconfigProviderMock) - controller := NewGardenerClusterController(mgr, kubeconfigProviderMock, logger) + controller := NewGardenerClusterController(mgr, kubeconfigProviderMock, logger, 24*time.Hour) Expect(controller).NotTo(BeNil()) err = controller.SetupWithManager(mgr) From d73165f8eee17f06d1ff700dbe0ef0dfc68780ac Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 05:40:44 +0200 Subject: [PATCH 14/39] Rotation is working --- .../controller/gardener_cluster_controller.go | 23 +++- .../gardener_cluster_controller_test.go | 108 +++++++++++++++++- 2 files changed, 123 insertions(+), 8 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index e9e085da..2557383c 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -40,7 +40,6 @@ const ( clusterCRNameLabel = "operator.kyma-project.io/cluster-name" // The ratio determines what is the minimal time that needs to pass to rotate certificate. minimalRotationTimeRatio = 0.6 - defaultRequeuInSeconds = 60 * 1000 ) // GardenerClusterController reconciles a GardenerCluster object @@ -101,7 +100,8 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req cluster.UpdateConditionForProcessingState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigReadingSecret, metav1.ConditionTrue) - err = controller.createOrRotateSecret(ctx, cluster) + lastSyncTime := time.Now() + err = controller.createOrRotateSecret(ctx, cluster, lastSyncTime) if err != nil { if controller.persistStatusChange(ctx, &cluster) != nil { controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) @@ -110,6 +110,12 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req } cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) + annotations := cluster.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) + cluster.SetAnnotations(annotations) return controller.resultWithRequeue(), controller.persistStatusChange(ctx, &cluster) } @@ -174,14 +180,13 @@ func (controller *GardenerClusterController) getSecret(shootName string) (*corev return &secretList.Items[0], nil } -func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Context, cluster imv1.GardenerCluster) error { +func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Context, cluster imv1.GardenerCluster, lastSyncTime time.Time) error { kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) if err != nil { cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, metav1.ConditionTrue, err) return err } - lastSyncTime := time.Now() newSecret := controller.newSecret(cluster, kubeconfig, lastSyncTime) err = controller.Client.Create(ctx, &newSecret) @@ -194,7 +199,13 @@ func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Co } existingSecret.Data[cluster.Spec.Kubeconfig.Secret.Key] = []byte(kubeconfig) - existingSecret.GetAnnotations()[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().String() + annotations := existingSecret.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + + annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) + existingSecret.SetAnnotations(annotations) err = controller.Client.Update(ctx, existingSecret) if err != nil { @@ -227,7 +238,7 @@ func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerClus Name: cluster.Spec.Kubeconfig.Secret.Name, Namespace: cluster.Spec.Kubeconfig.Secret.Namespace, Labels: labels, - Annotations: map[string]string{lastKubeconfigSyncAnnotation: lastSyncTime.UTC().String()}, + Annotations: map[string]string{lastKubeconfigSyncAnnotation: lastSyncTime.UTC().Format(time.RFC3339)}, }, StringData: map[string]string{cluster.Spec.Kubeconfig.Secret.Key: kubeconfig}, } diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index 9682f666..69fb1d73 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -51,8 +51,85 @@ var _ = Describe("Gardener Cluster controller", func() { }, time.Second*30, time.Second*3).Should(BeTrue()) }) }) + + Context("Secret with kubeconfig exists", func() { + namespace := "default" + + DescribeTable("Create secret when needed", func(gardenerClusterCR imv1.GardenerCluster, secret corev1.Secret, previousTimestamp, expectedKubeconfig string) { + By("Create kubeconfig secret") + Expect(k8sClient.Create(context.Background(), &secret)).To(Succeed()) + + By("Create Cluster CR") + Expect(k8sClient.Create(context.Background(), &gardenerClusterCR)).To(Succeed()) + + var kubeconfigSecret corev1.Secret + secretKey := types.NamespacedName{Name: secret.Name, Namespace: namespace} + + Eventually(func() bool { + + err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) + if err != nil { + return false + } + + _, forceAnnotationFound := kubeconfigSecret.Annotations[forceKubeconfigRotationAnnotation] + timestampAnnotation := kubeconfigSecret.Annotations[lastKubeconfigSyncAnnotation] + + return !forceAnnotationFound && timestampAnnotation != previousTimestamp + }, time.Second*30, time.Second*3).Should(BeTrue()) + + gardenerClusterKey := types.NamespacedName{Name: gardenerClusterCR.Name, Namespace: gardenerClusterCR.Namespace} + var newGardenerCluster imv1.GardenerCluster + + Eventually(func() bool { + err := k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) + if err != nil { + return false + } + + return newGardenerCluster.Status.State == imv1.ReadyState + }, time.Second*30, time.Second*3).Should(BeTrue()) + + err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) + Expect(err).To(BeNil()) + Expect(string(kubeconfigSecret.Data["config"])).To(Equal(expectedKubeconfig)) + + err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) + Expect(err).To(BeNil()) + Expect(newGardenerCluster.Status.State).To(Equal(imv1.ReadyState)) + newTime, err := parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) + Expect(err).To(BeNil()) + previousTime, err := parseLastSyncTime(previousTimestamp) + Expect(err).To(BeNil()) + Expect(newTime.Compare(previousTime)).To(Equal(1)) + }, + Entry("Rotate kubeconfig when rotation time passed", + fixGardenerClusterCRWithLastSyncTime("kymaname2", namespace, "shootName2", "secret-name2", time.Date(2023, 10, 19, 0, 0, 0, 0, time.FixedZone("UTC", 0))), + fixNewSecret("secret-name2", namespace, "kymaname2", "shootName2", "kubeconfig2"), + "2023-10-09T23:00:00Z", + "kubeconfig2"), + //Entry("Rotate dynamic kubeconfig", + // fixClusterInventoryCR("cluster3", "default", "kymaName3", "shootName3", "dynamic-kubeconfig-secret"), + // fixSecretWithDynamicKubeconfig("dynamic-kubeconfig-secret", namespace, "kymaName3", "shootName3", "kubeconfig3", "2006-01-02T15:04:05Z07:00"), + // "2022-11-10 23:00:00 +0000", + // "kubeconfig3"), + ) + + Describe("Skip rotation", func() { + + }) + }) }) +func parseLastSyncTime(lastSyncTimeString string) (time.Time, error) { + lastSyncTime, err := time.Parse(time.RFC3339, lastSyncTimeString) + if err != nil { + return time.Time{}, err + } + + return lastSyncTime, nil +} + func fixNewSecret(name, namespace, kymaName, shootName, data string) corev1.Secret { labels := fixSecretLabels(kymaName, shootName) @@ -99,11 +176,18 @@ func fixSecretLabels(kymaName, shootName string) map[string]string { } func fixGardenerClusterCR(kymaName, namespace, shootName, secretName string) imv1.GardenerCluster { - return newTestInfrastructureManagerCR(kymaName, namespace, shootName, secretName). + return newTestGardenerClusterCR(kymaName, namespace, shootName, secretName). WithLabels(fixClusterInventoryLabels(kymaName, shootName)).ToCluster() } -func newTestInfrastructureManagerCR(name, namespace, shootName, secretName string) *TestGardenerClusterCR { +func fixGardenerClusterCRWithLastSyncTime(kymaName, namespace, shootName, secretName string, lastSyncTime time.Time) imv1.GardenerCluster { + return newTestGardenerClusterCR(kymaName, namespace, shootName, secretName). + WithLabels(fixClusterInventoryLabels(kymaName, shootName)). + WithAnnotations(fixClusterInventoryAnnotations(&lastSyncTime, false)). + ToCluster() +} + +func newTestGardenerClusterCR(name, namespace, shootName, secretName string) *TestGardenerClusterCR { return &TestGardenerClusterCR{ gardenerCluster: imv1.GardenerCluster{ ObjectMeta: metav1.ObjectMeta{ @@ -132,6 +216,12 @@ func (sb *TestGardenerClusterCR) WithLabels(labels map[string]string) *TestGarde return sb } +func (sb *TestGardenerClusterCR) WithAnnotations(annotations map[string]string) *TestGardenerClusterCR { + sb.gardenerCluster.Annotations = annotations + + return sb +} + func (sb *TestGardenerClusterCR) ToCluster() imv1.GardenerCluster { return sb.gardenerCluster } @@ -155,3 +245,17 @@ func fixClusterInventoryLabels(kymaName, shootName string) map[string]string { return labels } + +func fixClusterInventoryAnnotations(lastSyncTime *time.Time, forceRotation bool) map[string]string { + annotations := map[string]string{} + + if lastSyncTime != nil { + annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) + } + + if forceRotation { + annotations[forceKubeconfigRotationAnnotation] = "true" + } + + return annotations +} From defc1b5f4f8a7b3dbbd66594f043c3a21b8b3047 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 06:37:25 +0200 Subject: [PATCH 15/39] Add some debugging info --- .../controller/gardener_cluster_controller.go | 11 ++++++++++- .../gardener_cluster_controller_test.go | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 2557383c..3968eed1 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -103,6 +103,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req lastSyncTime := time.Now() err = controller.createOrRotateSecret(ctx, cluster, lastSyncTime) if err != nil { + controller.log.Error(err, "Failed to create, or rotate secret", err) if controller.persistStatusChange(ctx, &cluster) != nil { controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) } @@ -114,10 +115,18 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if annotations == nil { annotations = map[string]string{} } + annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) cluster.SetAnnotations(annotations) - return controller.resultWithRequeue(), controller.persistStatusChange(ctx, &cluster) + controller.log.Info("Setting lastKubeconfigSyncAnnotation annotation", annotations[lastKubeconfigSyncAnnotation]) + + if err := controller.persistStatusChange(ctx, &cluster); err != nil { + controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) + return controller.resultWithoutRequeue(), err + } + + return controller.resultWithRequeue(), nil } func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index 69fb1d73..33808369 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -34,6 +34,17 @@ var _ = Describe("Gardener Cluster controller", func() { return k8sClient.Get(context.Background(), key, &kubeconfigSecret) == nil }, time.Second*30, time.Second*3).Should(BeTrue()) + gardenerClusterKey := types.NamespacedName{Name: gardenerClusterCR.Name, Namespace: gardenerClusterCR.Namespace} + var newGardenerCluster imv1.GardenerCluster + Eventually(func() bool { + err := k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) + if err != nil { + return false + } + + return newGardenerCluster.Status.State == imv1.ReadyState + }, time.Second*30, time.Second*3).Should(BeTrue()) + err := k8sClient.Get(context.Background(), key, &kubeconfigSecret) Expect(err).To(BeNil()) expectedSecret := fixNewSecret(secretName, namespace, kymaName, shootName, "kubeconfig1") @@ -41,6 +52,11 @@ var _ = Describe("Gardener Cluster controller", func() { Expect(kubeconfigSecret.Data).To(Equal(expectedSecret.Data)) Expect(kubeconfigSecret.Annotations[lastKubeconfigSyncAnnotation]).To(Not(BeEmpty())) + err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) + Expect(err).To(BeNil()) + //_, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) + //Expect(err).To(BeNil()) + By("Delete Cluster CR") Expect(k8sClient.Delete(context.Background(), &gardenerClusterCR)).To(Succeed()) @@ -96,7 +112,6 @@ var _ = Describe("Gardener Cluster controller", func() { err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) Expect(err).To(BeNil()) - Expect(newGardenerCluster.Status.State).To(Equal(imv1.ReadyState)) newTime, err := parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) Expect(err).To(BeNil()) previousTime, err := parseLastSyncTime(previousTimestamp) From c090dc9bd54281d6a560be79ffdc35c8307518d7 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 07:25:38 +0200 Subject: [PATCH 16/39] Refactoring, and linter --- .../controller/gardener_cluster_controller.go | 66 ++++++++++--------- .../gardener_cluster_controller_test.go | 6 +- internal/controller/predicates.go | 3 +- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 3968eed1..8f352cb9 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -101,9 +101,9 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req cluster.UpdateConditionForProcessingState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigReadingSecret, metav1.ConditionTrue) lastSyncTime := time.Now() - err = controller.createOrRotateSecret(ctx, cluster, lastSyncTime) + err = controller.createOrRotateSecret(ctx, &cluster, lastSyncTime) if err != nil { - controller.log.Error(err, "Failed to create, or rotate secret", err) + controller.log.Error(err, "Failed to create, or rotate secret") if controller.persistStatusChange(ctx, &cluster) != nil { controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) } @@ -119,9 +119,9 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) cluster.SetAnnotations(annotations) - controller.log.Info("Setting lastKubeconfigSyncAnnotation annotation", annotations[lastKubeconfigSyncAnnotation]) + err = controller.persistStatusChange(ctx, &cluster) - if err := controller.persistStatusChange(ctx, &cluster); err != nil { + if err != nil { controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) return controller.resultWithoutRequeue(), err } @@ -189,48 +189,52 @@ func (controller *GardenerClusterController) getSecret(shootName string) (*corev return &secretList.Items[0], nil } -func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Context, cluster imv1.GardenerCluster, lastSyncTime time.Time) error { +func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) if err != nil { cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, metav1.ConditionTrue, err) return err } - newSecret := controller.newSecret(cluster, kubeconfig, lastSyncTime) + existingSecret, err := controller.getSecret(cluster.Spec.Shoot.Name) + if err != nil && !k8serrors.IsNotFound(err) { + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) + return err + } - err = controller.Client.Create(ctx, &newSecret) - if err != nil { - if k8serrors.IsAlreadyExists(err) { - existingSecret, err := controller.getSecret(cluster.Spec.Shoot.Name) - if err != nil { - cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) - return err - } + if existingSecret != nil { + return controller.updateExistingSecret(ctx, kubeconfig, cluster, existingSecret, lastSyncTime) + } - existingSecret.Data[cluster.Spec.Kubeconfig.Secret.Key] = []byte(kubeconfig) - annotations := existingSecret.GetAnnotations() - if annotations == nil { - annotations = map[string]string{} - } + return controller.createNewSecret(ctx, kubeconfig, cluster, lastSyncTime) +} - annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) - existingSecret.SetAnnotations(annotations) +func (controller *GardenerClusterController) createNewSecret(ctx context.Context, kubeconfig string, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { + newSecret := controller.newSecret(*cluster, kubeconfig, lastSyncTime) + err := controller.Client.Create(ctx, &newSecret) + if err != nil { + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) + } - err = controller.Client.Update(ctx, existingSecret) - if err != nil { - cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToUpdateSecret, metav1.ConditionTrue, err) - return err - } + return err +} - return nil - } +func (controller *GardenerClusterController) updateExistingSecret(ctx context.Context, kubeconfig string, cluster *imv1.GardenerCluster, existingSecret *corev1.Secret, lastSyncTime time.Time) error { + existingSecret.Data[cluster.Spec.Kubeconfig.Secret.Key] = []byte(kubeconfig) + annotations := existingSecret.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } - cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, metav1.ConditionTrue, err) + annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) + existingSecret.SetAnnotations(annotations) - return err + err := controller.Client.Update(ctx, existingSecret) + if err != nil { + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToUpdateSecret, metav1.ConditionTrue, err) } - return nil + return err } func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerCluster, kubeconfig string, lastSyncTime time.Time) corev1.Secret { diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index 33808369..7ddf6d1b 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -54,8 +54,8 @@ var _ = Describe("Gardener Cluster controller", func() { err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) Expect(err).To(BeNil()) - //_, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) - //Expect(err).To(BeNil()) + // _, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) + // Expect(err).To(BeNil()) By("Delete Cluster CR") Expect(k8sClient.Delete(context.Background(), &gardenerClusterCR)).To(Succeed()) @@ -123,7 +123,7 @@ var _ = Describe("Gardener Cluster controller", func() { fixNewSecret("secret-name2", namespace, "kymaname2", "shootName2", "kubeconfig2"), "2023-10-09T23:00:00Z", "kubeconfig2"), - //Entry("Rotate dynamic kubeconfig", + // Entry("Rotate dynamic kubeconfig", // fixClusterInventoryCR("cluster3", "default", "kymaName3", "shootName3", "dynamic-kubeconfig-secret"), // fixSecretWithDynamicKubeconfig("dynamic-kubeconfig-secret", namespace, "kymaName3", "shootName3", "kubeconfig3", "2006-01-02T15:04:05Z07:00"), // "2022-11-10 23:00:00 +0000", diff --git a/internal/controller/predicates.go b/internal/controller/predicates.go index 86b75100..3c667119 100644 --- a/internal/controller/predicates.go +++ b/internal/controller/predicates.go @@ -1,10 +1,11 @@ package controller import ( + "time" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" - "time" ) func newRotationNotNeededPredicate(rotationPeriod time.Duration) predicate.Predicate { From a490c8fb1871d6f7a6790b284f5b9259071586ea Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 07:51:54 +0200 Subject: [PATCH 17/39] Some logs added --- internal/controller/gardener_cluster_controller.go | 11 ++++++++++- .../controller/gardener_cluster_controller_test.go | 4 ++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 8f352cb9..d7ed68c3 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -100,6 +100,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req cluster.UpdateConditionForProcessingState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigReadingSecret, metav1.ConditionTrue) + controller.log.Info("About to create, or rotate secret") lastSyncTime := time.Now() err = controller.createOrRotateSecret(ctx, &cluster, lastSyncTime) if err != nil { @@ -119,8 +120,9 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) cluster.SetAnnotations(annotations) - err = controller.persistStatusChange(ctx, &cluster) + controller.log.Info(fmt.Sprintf("Annotations: %v", cluster.Annotations)) + err = controller.persistStatusChange(ctx, &cluster) if err != nil { controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) return controller.resultWithoutRequeue(), err @@ -192,12 +194,15 @@ func (controller *GardenerClusterController) getSecret(shootName string) (*corev func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) if err != nil { + controller.log.Error(err, "Failed to get kubecofnig") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, metav1.ConditionTrue, err) return err } + controller.log.Info("Getting secret") existingSecret, err := controller.getSecret(cluster.Spec.Shoot.Name) if err != nil && !k8serrors.IsNotFound(err) { + controller.log.Error(err, "Failed to get kubeconfig secret") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) return err } @@ -210,9 +215,11 @@ func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Co } func (controller *GardenerClusterController) createNewSecret(ctx context.Context, kubeconfig string, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { + controller.log.Info("Creating a new kubeconfig secret") newSecret := controller.newSecret(*cluster, kubeconfig, lastSyncTime) err := controller.Client.Create(ctx, &newSecret) if err != nil { + controller.log.Error(err, "Failed to create secret") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) } @@ -220,6 +227,7 @@ func (controller *GardenerClusterController) createNewSecret(ctx context.Context } func (controller *GardenerClusterController) updateExistingSecret(ctx context.Context, kubeconfig string, cluster *imv1.GardenerCluster, existingSecret *corev1.Secret, lastSyncTime time.Time) error { + controller.log.Info("Updating the kubeconfig secret") existingSecret.Data[cluster.Spec.Kubeconfig.Secret.Key] = []byte(kubeconfig) annotations := existingSecret.GetAnnotations() if annotations == nil { @@ -231,6 +239,7 @@ func (controller *GardenerClusterController) updateExistingSecret(ctx context.Co err := controller.Client.Update(ctx, existingSecret) if err != nil { + controller.log.Error(err, "Failed to update secret") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToUpdateSecret, metav1.ConditionTrue, err) } diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index 7ddf6d1b..a437a732 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -54,8 +54,8 @@ var _ = Describe("Gardener Cluster controller", func() { err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) Expect(err).To(BeNil()) - // _, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) - // Expect(err).To(BeNil()) + _, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) + Expect(err).To(BeNil()) By("Delete Cluster CR") Expect(k8sClient.Delete(context.Background(), &gardenerClusterCR)).To(Succeed()) From 7f69b7fb67258cc335b1925460d56320b050a520 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 07:53:47 +0200 Subject: [PATCH 18/39] Unit test fixe --- internal/controller/gardener_cluster_controller_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index a437a732..bfc54802 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -54,8 +54,8 @@ var _ = Describe("Gardener Cluster controller", func() { err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) Expect(err).To(BeNil()) - _, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) - Expect(err).To(BeNil()) + //_, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) + //Expect(err).To(BeNil()) By("Delete Cluster CR") Expect(k8sClient.Delete(context.Background(), &gardenerClusterCR)).To(Succeed()) From 22522ee25ee829d51b476d54103915b7be3b2a8a Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 07:55:30 +0200 Subject: [PATCH 19/39] Unit test fixe --- internal/controller/gardener_cluster_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index bfc54802..c3222b54 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -54,7 +54,7 @@ var _ = Describe("Gardener Cluster controller", func() { err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) Expect(err).To(BeNil()) - //_, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) + // _, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) //Expect(err).To(BeNil()) By("Delete Cluster CR") From 67125f0d47b18042fb183d868a07dc9abd797366 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 07:57:45 +0200 Subject: [PATCH 20/39] Liiinteeer --- internal/controller/gardener_cluster_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index c3222b54..7ddf6d1b 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -55,7 +55,7 @@ var _ = Describe("Gardener Cluster controller", func() { err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) Expect(err).To(BeNil()) // _, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) - //Expect(err).To(BeNil()) + // Expect(err).To(BeNil()) By("Delete Cluster CR") Expect(k8sClient.Delete(context.Background(), &gardenerClusterCR)).To(Succeed()) From cedd61508b64774ee1f1eeeee2b713d3c7bf5307 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 11:27:34 +0200 Subject: [PATCH 21/39] Fixed update problem --- .../controller/gardener_cluster_controller.go | 41 ++++++++++++++----- .../gardener_cluster_controller_test.go | 4 +- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index d7ed68c3..fcadf251 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -19,6 +19,7 @@ package controller import ( "context" "fmt" + "k8s.io/apimachinery/pkg/types" "time" "github.com/go-logr/logr" @@ -106,25 +107,22 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if err != nil { controller.log.Error(err, "Failed to create, or rotate secret") if controller.persistStatusChange(ctx, &cluster) != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) + controller.log.Error(err, "Failed to set state for GardenerCluster") } return controller.resultWithoutRequeue(), err } cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) - annotations := cluster.GetAnnotations() - if annotations == nil { - annotations = map[string]string{} - } - - annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) - cluster.SetAnnotations(annotations) - - controller.log.Info(fmt.Sprintf("Annotations: %v", cluster.Annotations)) err = controller.persistStatusChange(ctx, &cluster) if err != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster", req.NamespacedName) + controller.log.Error(err, "Failed to set state for GardenerCluster") + return controller.resultWithoutRequeue(), err + } + + err = controller.persistLastSyncTime(ctx, &cluster, lastSyncTime) + if err != nil { + controller.log.Error(err, "Failed to set state for GardenerCluster") return controller.resultWithoutRequeue(), err } @@ -148,6 +146,27 @@ func (controller *GardenerClusterController) persistStatusChange(ctx context.Con return controller.Client.Status().Update(ctx, cluster) } +func (controller *GardenerClusterController) persistLastSyncTime(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { + var clusterToUpgrade imv1.GardenerCluster + + gardenerClusterKey := types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace} + + err := controller.Client.Get(ctx, gardenerClusterKey, &clusterToUpgrade) + if err != nil { + return err + } + annotations := clusterToUpgrade.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + + annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) + + clusterToUpgrade.SetAnnotations(annotations) + + return controller.Client.Update(ctx, &clusterToUpgrade) +} + func (controller *GardenerClusterController) deleteSecret(clusterCRName string) error { selector := client.MatchingLabels(map[string]string{ clusterCRNameLabel: clusterCRName, diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index 7ddf6d1b..a437a732 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -54,8 +54,8 @@ var _ = Describe("Gardener Cluster controller", func() { err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) Expect(err).To(BeNil()) - // _, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) - // Expect(err).To(BeNil()) + _, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) + Expect(err).To(BeNil()) By("Delete Cluster CR") Expect(k8sClient.Delete(context.Background(), &gardenerClusterCR)).To(Succeed()) From 3f1a1733c7b498ef85fc671a5800023e5a9a06e3 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 11:29:47 +0200 Subject: [PATCH 22/39] Linter --- internal/controller/gardener_cluster_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index fcadf251..447881de 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -19,7 +19,6 @@ package controller import ( "context" "fmt" - "k8s.io/apimachinery/pkg/types" "time" "github.com/go-logr/logr" @@ -30,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" From 559f34a63b064f336dcb664bdfdf82e7b20cbd67 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 12:49:44 +0200 Subject: [PATCH 23/39] Attempt to fix update problem --- .../controller/gardener_cluster_controller.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 447881de..ff751b3d 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -112,15 +112,15 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req return controller.resultWithoutRequeue(), err } - cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) - - err = controller.persistStatusChange(ctx, &cluster) + err = controller.persistLastSyncTime(ctx, &cluster, lastSyncTime) if err != nil { controller.log.Error(err, "Failed to set state for GardenerCluster") return controller.resultWithoutRequeue(), err } - err = controller.persistLastSyncTime(ctx, &cluster, lastSyncTime) + cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) + + err = controller.persistStatusChange(ctx, &cluster) if err != nil { controller.log.Error(err, "Failed to set state for GardenerCluster") return controller.resultWithoutRequeue(), err @@ -147,24 +147,22 @@ func (controller *GardenerClusterController) persistStatusChange(ctx context.Con } func (controller *GardenerClusterController) persistLastSyncTime(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { - var clusterToUpgrade imv1.GardenerCluster - gardenerClusterKey := types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace} - err := controller.Client.Get(ctx, gardenerClusterKey, &clusterToUpgrade) + err := controller.Client.Get(ctx, gardenerClusterKey, cluster) if err != nil { return err } - annotations := clusterToUpgrade.GetAnnotations() + annotations := cluster.GetAnnotations() if annotations == nil { annotations = map[string]string{} } annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) - clusterToUpgrade.SetAnnotations(annotations) + cluster.SetAnnotations(annotations) - return controller.Client.Update(ctx, &clusterToUpgrade) + return controller.Client.Update(ctx, cluster) } func (controller *GardenerClusterController) deleteSecret(clusterCRName string) error { From 0dcff61f328e3d19735b865fa14b0756ca9c5ed5 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 15:07:20 +0200 Subject: [PATCH 24/39] Changed rotation to be based on secret --- .../controller/gardener_cluster_controller.go | 121 +++++++++--------- .../gardener_cluster_controller_test.go | 2 - internal/controller/predicates.go | 65 ---------- 3 files changed, 62 insertions(+), 126 deletions(-) delete mode 100644 internal/controller/predicates.go diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index ff751b3d..14d84534 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -29,7 +29,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -49,18 +48,18 @@ type GardenerClusterController struct { Scheme *runtime.Scheme KubeconfigProvider KubeconfigProvider log logr.Logger - requeueAfter time.Duration + rotationPeriod time.Duration } func NewGardenerClusterController(mgr ctrl.Manager, kubeconfigProvider KubeconfigProvider, logger logr.Logger, kubeconfigExpirationTime time.Duration) *GardenerClusterController { - requeueTimeInMinutes := int64(minimalRotationTimeRatio * kubeconfigExpirationTime.Minutes()) + rotationPeriodInMinutes := int64(minimalRotationTimeRatio * kubeconfigExpirationTime.Minutes()) return &GardenerClusterController{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), KubeconfigProvider: kubeconfigProvider, log: logger, - requeueAfter: time.Duration(requeueTimeInMinutes) * time.Minute, + rotationPeriod: time.Duration(rotationPeriodInMinutes) * time.Minute, } } @@ -103,27 +102,17 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req controller.log.Info("About to create, or rotate secret") lastSyncTime := time.Now() - err = controller.createOrRotateSecret(ctx, &cluster, lastSyncTime) - if err != nil { - controller.log.Error(err, "Failed to create, or rotate secret") - if controller.persistStatusChange(ctx, &cluster) != nil { + updateStatus, err := controller.createOrRotateSecret(ctx, &cluster, lastSyncTime) + + if updateStatus { + statusErr := controller.persistStatusChange(ctx, &cluster) + if statusErr != nil { controller.log.Error(err, "Failed to set state for GardenerCluster") + if err != nil { + return controller.resultWithoutRequeue(), err + } + return controller.resultWithoutRequeue(), statusErr } - return controller.resultWithoutRequeue(), err - } - - err = controller.persistLastSyncTime(ctx, &cluster, lastSyncTime) - if err != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster") - return controller.resultWithoutRequeue(), err - } - - cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) - - err = controller.persistStatusChange(ctx, &cluster) - if err != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster") - return controller.resultWithoutRequeue(), err } return controller.resultWithRequeue(), nil @@ -132,7 +121,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { return ctrl.Result{ Requeue: true, - RequeueAfter: controller.requeueAfter, + RequeueAfter: controller.rotationPeriod, } } @@ -146,25 +135,6 @@ func (controller *GardenerClusterController) persistStatusChange(ctx context.Con return controller.Client.Status().Update(ctx, cluster) } -func (controller *GardenerClusterController) persistLastSyncTime(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { - gardenerClusterKey := types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace} - - err := controller.Client.Get(ctx, gardenerClusterKey, cluster) - if err != nil { - return err - } - annotations := cluster.GetAnnotations() - if annotations == nil { - annotations = map[string]string{} - } - - annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) - - cluster.SetAnnotations(annotations) - - return controller.Client.Update(ctx, cluster) -} - func (controller *GardenerClusterController) deleteSecret(clusterCRName string) error { selector := client.MatchingLabels(map[string]string{ clusterCRNameLabel: clusterCRName, @@ -208,27 +178,54 @@ func (controller *GardenerClusterController) getSecret(shootName string) (*corev return &secretList.Items[0], nil } -func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { - kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) - if err != nil { - controller.log.Error(err, "Failed to get kubecofnig") - cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, metav1.ConditionTrue, err) - return err - } - +func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) (bool, error) { controller.log.Info("Getting secret") existingSecret, err := controller.getSecret(cluster.Spec.Shoot.Name) if err != nil && !k8serrors.IsNotFound(err) { controller.log.Error(err, "Failed to get kubeconfig secret") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) - return err + return true, err + } + + if !secretNeedsToBeRotated(existingSecret, controller.rotationPeriod) { + return false, nil + } + + kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) + if err != nil { + controller.log.Error(err, "Failed to get kubeconfig") + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, metav1.ConditionTrue, err) + return true, err } if existingSecret != nil { - return controller.updateExistingSecret(ctx, kubeconfig, cluster, existingSecret, lastSyncTime) + return true, controller.updateExistingSecret(ctx, kubeconfig, cluster, existingSecret, lastSyncTime) + } + + return true, controller.createNewSecret(ctx, kubeconfig, cluster, lastSyncTime) +} + +func secretNeedsToBeRotated(secret *corev1.Secret, rotationPeriod time.Duration) bool { + if secret == nil { + return true + } + annotations := secret.GetAnnotations() + + _, found := annotations[lastKubeconfigSyncAnnotation] + + if !found { + return true } - return controller.createNewSecret(ctx, kubeconfig, cluster, lastSyncTime) + lastSyncTimeString := annotations[lastKubeconfigSyncAnnotation] + lastSyncTime, err := time.Parse(time.RFC3339, lastSyncTimeString) + if err != nil { + return true + } + now := time.Now() + alreadyValidFor := lastSyncTime.Sub(now) + + return alreadyValidFor.Minutes() >= rotationPeriod.Minutes() } func (controller *GardenerClusterController) createNewSecret(ctx context.Context, kubeconfig string, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { @@ -238,9 +235,13 @@ func (controller *GardenerClusterController) createNewSecret(ctx context.Context if err != nil { controller.log.Error(err, "Failed to create secret") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) + + return err } - return err + cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) + + return nil } func (controller *GardenerClusterController) updateExistingSecret(ctx context.Context, kubeconfig string, cluster *imv1.GardenerCluster, existingSecret *corev1.Secret, lastSyncTime time.Time) error { @@ -258,9 +259,13 @@ func (controller *GardenerClusterController) updateExistingSecret(ctx context.Co if err != nil { controller.log.Error(err, "Failed to update secret") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToUpdateSecret, metav1.ConditionTrue, err) + + return err } - return err + cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) + + return nil } func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerCluster, kubeconfig string, lastSyncTime time.Time) corev1.Secret { @@ -286,8 +291,6 @@ func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerClus // SetupWithManager sets up the controller with the Manager. func (controller *GardenerClusterController) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&imv1.GardenerCluster{}, builder.WithPredicates( - newRotationNotNeededPredicate(controller.requeueAfter), - )). + For(&imv1.GardenerCluster{}, builder.WithPredicates()). Complete(controller) } diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index a437a732..78a46a31 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -54,8 +54,6 @@ var _ = Describe("Gardener Cluster controller", func() { err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) Expect(err).To(BeNil()) - _, err = parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) - Expect(err).To(BeNil()) By("Delete Cluster CR") Expect(k8sClient.Delete(context.Background(), &gardenerClusterCR)).To(Succeed()) diff --git a/internal/controller/predicates.go b/internal/controller/predicates.go deleted file mode 100644 index 3c667119..00000000 --- a/internal/controller/predicates.go +++ /dev/null @@ -1,65 +0,0 @@ -package controller - -import ( - "time" - - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/predicate" -) - -func newRotationNotNeededPredicate(rotationPeriod time.Duration) predicate.Predicate { - return RotationNotNeededPredicate{ - rotationPeriod: rotationPeriod, - } -} - -type RotationNotNeededPredicate struct { - rotationPeriod time.Duration -} - -func (RotationNotNeededPredicate) Create(event.CreateEvent) bool { - return true -} - -func (r RotationNotNeededPredicate) Update(e event.UpdateEvent) bool { - return kubeconfigMustBeRotated(e.ObjectNew, r.rotationPeriod) -} - -func (RotationNotNeededPredicate) Delete(event.DeleteEvent) bool { - return true -} - -func (r RotationNotNeededPredicate) Generic(e event.GenericEvent) bool { - return kubeconfigMustBeRotated(e.Object, r.rotationPeriod) -} - -func kubeconfigMustBeRotated(gardenerCluster client.Object, validityTime time.Duration) bool { - return rotationForced(gardenerCluster) || rotationTimePassed(gardenerCluster, validityTime) -} - -func rotationTimePassed(gardenerCluster client.Object, rotationPeriod time.Duration) bool { - annotations := gardenerCluster.GetAnnotations() - - _, found := annotations[lastKubeconfigSyncAnnotation] - - if !found { - return true - } - - lastSyncTimeString := annotations[lastKubeconfigSyncAnnotation] - lastSyncTime, err := time.Parse(time.RFC3339, lastSyncTimeString) - if err != nil { - return true - } - now := time.Now() - alreadyValidFor := lastSyncTime.Sub(now) - - return alreadyValidFor.Minutes() >= rotationPeriod.Minutes() -} - -func rotationForced(gardenerCluster client.Object) bool { - _, found := gardenerCluster.GetAnnotations()[forceKubeconfigRotationAnnotation] - - return found -} From 453ae3d1095db741100dfa93528b48fc1b4064ee Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 15:29:09 +0200 Subject: [PATCH 25/39] Added force rotation --- .../controller/gardener_cluster_controller.go | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 14d84534..134061e7 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -187,7 +187,7 @@ func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Co return true, err } - if !secretNeedsToBeRotated(existingSecret, controller.rotationPeriod) { + if !secretNeedsToBeRotated(cluster, existingSecret, controller.rotationPeriod) { return false, nil } @@ -205,10 +205,15 @@ func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Co return true, controller.createNewSecret(ctx, kubeconfig, cluster, lastSyncTime) } -func secretNeedsToBeRotated(secret *corev1.Secret, rotationPeriod time.Duration) bool { +func secretNeedsToBeRotated(cluster *imv1.GardenerCluster, secret *corev1.Secret, rotationPeriod time.Duration) bool { + return secretRotationTimePassed(secret, rotationPeriod) || secretRotationForced(cluster) +} + +func secretRotationTimePassed(secret *corev1.Secret, rotationPeriod time.Duration) bool { if secret == nil { return true } + annotations := secret.GetAnnotations() _, found := annotations[lastKubeconfigSyncAnnotation] @@ -228,6 +233,17 @@ func secretNeedsToBeRotated(secret *corev1.Secret, rotationPeriod time.Duration) return alreadyValidFor.Minutes() >= rotationPeriod.Minutes() } +func secretRotationForced(cluster *imv1.GardenerCluster) bool { + annotations := cluster.GetAnnotations() + if annotations == nil { + return false + } + + _, found := annotations[forceKubeconfigRotationAnnotation] + + return found +} + func (controller *GardenerClusterController) createNewSecret(ctx context.Context, kubeconfig string, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { controller.log.Info("Creating a new kubeconfig secret") newSecret := controller.newSecret(*cluster, kubeconfig, lastSyncTime) From 236192cc615c391a15aa66a9e0c985ea29ef68bb Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 16:39:59 +0200 Subject: [PATCH 26/39] Implemented removing operator.kyma-project.io/force-kubeconfig-rotation annotation --- .../controller/gardener_cluster_controller.go | 32 +++++++++++ .../gardener_cluster_controller_test.go | 54 ++++++------------- 2 files changed, 47 insertions(+), 39 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 134061e7..145386ad 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -29,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -113,6 +114,8 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req } return controller.resultWithoutRequeue(), statusErr } + + return controller.resultWithoutRequeue(), controller.removeForceRotationAnnotationIfNeeded(ctx, &cluster) } return controller.resultWithRequeue(), nil @@ -284,6 +287,35 @@ func (controller *GardenerClusterController) updateExistingSecret(ctx context.Co return nil } +func (controller *GardenerClusterController) removeForceRotationAnnotationIfNeeded(ctx context.Context, cluster *imv1.GardenerCluster) error { + annotations := cluster.GetAnnotations() + if annotations == nil { + return nil + } + + _, found := annotations[forceKubeconfigRotationAnnotation] + if found { + key := types.NamespacedName{ + Name: cluster.Name, + Namespace: cluster.Namespace, + } + var clusterToUpdate imv1.GardenerCluster + + err := controller.Client.Get(ctx, key, &clusterToUpdate) + if err != nil { + return err + } + + annotations := clusterToUpdate.GetAnnotations() + delete(annotations, forceKubeconfigRotationAnnotation) + clusterToUpdate.SetAnnotations(annotations) + + return controller.Client.Update(ctx, &clusterToUpdate) + } + + return nil +} + func (controller *GardenerClusterController) newSecret(cluster imv1.GardenerCluster, kubeconfig string, lastSyncTime time.Time) corev1.Secret { labels := map[string]string{} diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index 78a46a31..ffabd445 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -86,10 +86,9 @@ var _ = Describe("Gardener Cluster controller", func() { return false } - _, forceAnnotationFound := kubeconfigSecret.Annotations[forceKubeconfigRotationAnnotation] timestampAnnotation := kubeconfigSecret.Annotations[lastKubeconfigSyncAnnotation] - return !forceAnnotationFound && timestampAnnotation != previousTimestamp + return timestampAnnotation != previousTimestamp }, time.Second*30, time.Second*3).Should(BeTrue()) gardenerClusterKey := types.NamespacedName{Name: gardenerClusterCR.Name, Namespace: gardenerClusterCR.Namespace} @@ -101,7 +100,10 @@ var _ = Describe("Gardener Cluster controller", func() { return false } - return newGardenerCluster.Status.State == imv1.ReadyState + readyState := newGardenerCluster.Status.State == imv1.ReadyState + _, forceRotationAnnotationFound := newGardenerCluster.GetAnnotations()[forceKubeconfigRotationAnnotation] + + return readyState && !forceRotationAnnotationFound }, time.Second*30, time.Second*3).Should(BeTrue()) err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) @@ -110,22 +112,17 @@ var _ = Describe("Gardener Cluster controller", func() { err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) Expect(err).To(BeNil()) - newTime, err := parseLastSyncTime(newGardenerCluster.GetAnnotations()[lastKubeconfigSyncAnnotation]) - Expect(err).To(BeNil()) - previousTime, err := parseLastSyncTime(previousTimestamp) - Expect(err).To(BeNil()) - Expect(newTime.Compare(previousTime)).To(Equal(1)) }, Entry("Rotate kubeconfig when rotation time passed", - fixGardenerClusterCRWithLastSyncTime("kymaname2", namespace, "shootName2", "secret-name2", time.Date(2023, 10, 19, 0, 0, 0, 0, time.FixedZone("UTC", 0))), + fixGardenerClusterCR("kymaname2", namespace, "shootName2", "secret-name2"), fixNewSecret("secret-name2", namespace, "kymaname2", "shootName2", "kubeconfig2"), "2023-10-09T23:00:00Z", "kubeconfig2"), - // Entry("Rotate dynamic kubeconfig", - // fixClusterInventoryCR("cluster3", "default", "kymaName3", "shootName3", "dynamic-kubeconfig-secret"), - // fixSecretWithDynamicKubeconfig("dynamic-kubeconfig-secret", namespace, "kymaName3", "shootName3", "kubeconfig3", "2006-01-02T15:04:05Z07:00"), - // "2022-11-10 23:00:00 +0000", - // "kubeconfig3"), + Entry("Rotate dynamic kubeconfig", + fixGardenerClusterCRWithForceRotationAnnotation("kymaname3", namespace, "shootName3", "secret-name3"), + fixNewSecret("secret-name3", namespace, "kymaname3", "shootName3", "kubeconfig3"), + time.Now().UTC().Format(time.RFC3339), + "kubeconfig3"), ) Describe("Skip rotation", func() { @@ -134,15 +131,6 @@ var _ = Describe("Gardener Cluster controller", func() { }) }) -func parseLastSyncTime(lastSyncTimeString string) (time.Time, error) { - lastSyncTime, err := time.Parse(time.RFC3339, lastSyncTimeString) - if err != nil { - return time.Time{}, err - } - - return lastSyncTime, nil -} - func fixNewSecret(name, namespace, kymaName, shootName, data string) corev1.Secret { labels := fixSecretLabels(kymaName, shootName) @@ -193,10 +181,12 @@ func fixGardenerClusterCR(kymaName, namespace, shootName, secretName string) imv WithLabels(fixClusterInventoryLabels(kymaName, shootName)).ToCluster() } -func fixGardenerClusterCRWithLastSyncTime(kymaName, namespace, shootName, secretName string, lastSyncTime time.Time) imv1.GardenerCluster { +func fixGardenerClusterCRWithForceRotationAnnotation(kymaName, namespace, shootName, secretName string) imv1.GardenerCluster { + annotations := map[string]string{forceKubeconfigRotationAnnotation: "true"} + return newTestGardenerClusterCR(kymaName, namespace, shootName, secretName). WithLabels(fixClusterInventoryLabels(kymaName, shootName)). - WithAnnotations(fixClusterInventoryAnnotations(&lastSyncTime, false)). + WithAnnotations(annotations). ToCluster() } @@ -258,17 +248,3 @@ func fixClusterInventoryLabels(kymaName, shootName string) map[string]string { return labels } - -func fixClusterInventoryAnnotations(lastSyncTime *time.Time, forceRotation bool) map[string]string { - annotations := map[string]string{} - - if lastSyncTime != nil { - annotations[lastKubeconfigSyncAnnotation] = lastSyncTime.UTC().Format(time.RFC3339) - } - - if forceRotation { - annotations[forceKubeconfigRotationAnnotation] = "true" - } - - return annotations -} From ce590b21279e97e056eb5a017e3777d80dee381a Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 22:24:31 +0200 Subject: [PATCH 27/39] Added new testcases --- .../controller/gardener_cluster_controller.go | 8 +- .../gardener_cluster_controller_test.go | 121 +++++++++++++----- internal/controller/suite_test.go | 4 + 3 files changed, 102 insertions(+), 31 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 145386ad..6598e66a 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -115,7 +115,10 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req return controller.resultWithoutRequeue(), statusErr } - return controller.resultWithoutRequeue(), controller.removeForceRotationAnnotationIfNeeded(ctx, &cluster) + err = controller.removeForceRotationAnnotationIfNeeded(ctx, &cluster) + if err != nil { + return controller.resultWithoutRequeue(), err + } } return controller.resultWithRequeue(), nil @@ -191,6 +194,7 @@ func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Co } if !secretNeedsToBeRotated(cluster, existingSecret, controller.rotationPeriod) { + controller.log.Info("Secret rotation skipped") return false, nil } @@ -231,7 +235,7 @@ func secretRotationTimePassed(secret *corev1.Secret, rotationPeriod time.Duratio return true } now := time.Now() - alreadyValidFor := lastSyncTime.Sub(now) + alreadyValidFor := now.Sub(lastSyncTime) return alreadyValidFor.Minutes() >= rotationPeriod.Minutes() } diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index ffabd445..95393e72 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -2,25 +2,25 @@ package controller import ( "context" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "time" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) var _ = Describe("Gardener Cluster controller", func() { Context("Secret with kubeconfig doesn't exist", func() { - kymaName := "kymaname1" - secretName := "secret-name1" - shootName := "shootName1" - namespace := "default" + It("Should create secret, and set Ready status on CR", func() { + kymaName := "kymaname1" + secretName := "secret-name1" + shootName := "shootName1" + namespace := "default" - It("Create secret", func() { By("Create GardenerCluster CR") gardenerClusterCR := fixGardenerClusterCR(kymaName, namespace, shootName, secretName) @@ -28,10 +28,10 @@ var _ = Describe("Gardener Cluster controller", func() { By("Wait for secret creation") var kubeconfigSecret corev1.Secret - key := types.NamespacedName{Name: secretName, Namespace: namespace} + secretKey := types.NamespacedName{Name: secretName, Namespace: namespace} Eventually(func() bool { - return k8sClient.Get(context.Background(), key, &kubeconfigSecret) == nil + return k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) == nil }, time.Second*30, time.Second*3).Should(BeTrue()) gardenerClusterKey := types.NamespacedName{Name: gardenerClusterCR.Name, Namespace: gardenerClusterCR.Namespace} @@ -45,34 +45,74 @@ var _ = Describe("Gardener Cluster controller", func() { return newGardenerCluster.Status.State == imv1.ReadyState }, time.Second*30, time.Second*3).Should(BeTrue()) - err := k8sClient.Get(context.Background(), key, &kubeconfigSecret) + err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) Expect(err).To(BeNil()) - expectedSecret := fixNewSecret(secretName, namespace, kymaName, shootName, "kubeconfig1") + expectedSecret := fixNewSecret(secretName, namespace, kymaName, shootName, "kubeconfig1", "") Expect(kubeconfigSecret.Labels).To(Equal(expectedSecret.Labels)) Expect(kubeconfigSecret.Data).To(Equal(expectedSecret.Data)) Expect(kubeconfigSecret.Annotations[lastKubeconfigSyncAnnotation]).To(Not(BeEmpty())) + }) - err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) - Expect(err).To(BeNil()) + It("Should delete secret", func() { + kymaName := "kymaname5" + secretName := "secret-name5" + shootName := "shootName5" + namespace := "default" + + By("Create GardenerCluster CR") + + gardenerClusterCR := fixGardenerClusterCR(kymaName, namespace, shootName, secretName) + Expect(k8sClient.Create(context.Background(), &gardenerClusterCR)).To(Succeed()) + + By("Wait for secret creation") + var kubeconfigSecret corev1.Secret + secretKey := types.NamespacedName{Name: secretName, Namespace: namespace} + + Eventually(func() bool { + return k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) == nil + }, time.Second*30, time.Second*3).Should(BeTrue()) By("Delete Cluster CR") Expect(k8sClient.Delete(context.Background(), &gardenerClusterCR)).To(Succeed()) By("Wait for secret deletion") Eventually(func() bool { - err := k8sClient.Get(context.Background(), key, &kubeconfigSecret) + err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) return err != nil && k8serrors.IsNotFound(err) }, time.Second*30, time.Second*3).Should(BeTrue()) }) + + It("Should set Error status on CR if failed to fetch kubeconfig", func() { + kymaName := "kymaname6" + secretName := "secret-name6" + shootName := "shootName6" + namespace := "default" + + gardenerClusterCR := fixGardenerClusterCR(kymaName, namespace, shootName, secretName) + Expect(k8sClient.Create(context.Background(), &gardenerClusterCR)).To(Succeed()) + + gardenerClusterKey := types.NamespacedName{Name: gardenerClusterCR.Name, Namespace: gardenerClusterCR.Namespace} + var newGardenerCluster imv1.GardenerCluster + Eventually(func() bool { + err := k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) + if err != nil { + return false + } + + return newGardenerCluster.Status.State == imv1.ErrorState + }, time.Second*30, time.Second*3).Should(BeTrue()) + }) }) Context("Secret with kubeconfig exists", func() { namespace := "default" - DescribeTable("Create secret when needed", func(gardenerClusterCR imv1.GardenerCluster, secret corev1.Secret, previousTimestamp, expectedKubeconfig string) { + DescribeTable("Should update secret", func(gardenerClusterCR imv1.GardenerCluster, secret corev1.Secret, expectedKubeconfig string) { By("Create kubeconfig secret") Expect(k8sClient.Create(context.Background(), &secret)).To(Succeed()) + previousTimestamp := secret.Annotations[lastKubeconfigSyncAnnotation] + By("Create Cluster CR") Expect(k8sClient.Create(context.Background(), &gardenerClusterCR)).To(Succeed()) @@ -80,7 +120,6 @@ var _ = Describe("Gardener Cluster controller", func() { secretKey := types.NamespacedName{Name: secret.Name, Namespace: namespace} Eventually(func() bool { - err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) if err != nil { return false @@ -109,33 +148,57 @@ var _ = Describe("Gardener Cluster controller", func() { err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) Expect(err).To(BeNil()) Expect(string(kubeconfigSecret.Data["config"])).To(Equal(expectedKubeconfig)) - - err = k8sClient.Get(context.Background(), gardenerClusterKey, &newGardenerCluster) - Expect(err).To(BeNil()) }, Entry("Rotate kubeconfig when rotation time passed", fixGardenerClusterCR("kymaname2", namespace, "shootName2", "secret-name2"), - fixNewSecret("secret-name2", namespace, "kymaname2", "shootName2", "kubeconfig2"), - "2023-10-09T23:00:00Z", + fixNewSecret("secret-name2", namespace, "kymaname2", "shootName2", "kubeconfig2", "2023-10-09T23:00:00Z"), "kubeconfig2"), Entry("Rotate dynamic kubeconfig", fixGardenerClusterCRWithForceRotationAnnotation("kymaname3", namespace, "shootName3", "secret-name3"), - fixNewSecret("secret-name3", namespace, "kymaname3", "shootName3", "kubeconfig3"), - time.Now().UTC().Format(time.RFC3339), + fixNewSecret("secret-name3", namespace, "kymaname3", "shootName3", "kubeconfig3", time.Now().UTC().Format(time.RFC3339)), "kubeconfig3"), ) - Describe("Skip rotation", func() { + It("Should skip rotation", func() { + By("Create kubeconfig secret") + secret := fixNewSecret("secret-name4", namespace, "kymaname4", "shootName4", "kubeconfig4", time.Now().UTC().Format(time.RFC3339)) + Expect(k8sClient.Create(context.Background(), &secret)).To(Succeed()) + previousTimestamp := secret.Annotations[lastKubeconfigSyncAnnotation] + + By("Create Cluster CR") + gardenerClusterCR := fixGardenerClusterCRWithForceRotationAnnotation("kymaname4", namespace, "shootName4", "secret-name4") + Expect(k8sClient.Create(context.Background(), &gardenerClusterCR)).To(Succeed()) + + var kubeconfigSecret corev1.Secret + secretKey := types.NamespacedName{Name: secret.Name, Namespace: namespace} + + Consistently(func() bool { + err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) + if err != nil { + return false + } + + timestampAnnotation := kubeconfigSecret.Annotations[lastKubeconfigSyncAnnotation] + + return timestampAnnotation == previousTimestamp + }, time.Second*30, time.Second*3).Should(BeTrue()) }) }) }) -func fixNewSecret(name, namespace, kymaName, shootName, data string) corev1.Secret { +func fixNewSecret(name, namespace, kymaName, shootName, data string, lastSyncTime string) corev1.Secret { labels := fixSecretLabels(kymaName, shootName) + annotations := map[string]string{lastKubeconfigSyncAnnotation: lastSyncTime} builder := newTestSecret(name, namespace) - return builder.WithLabels(labels).WithData(data).ToSecret() + return builder.WithLabels(labels).WithAnnotations(annotations).WithData(data).ToSecret() +} + +func (sb *TestSecret) WithAnnotations(annotations map[string]string) *TestSecret { + sb.secret.Annotations = annotations + + return sb } func (sb *TestSecret) WithLabels(labels map[string]string) *TestSecret { @@ -170,7 +233,7 @@ type TestSecret struct { } func fixSecretLabels(kymaName, shootName string) map[string]string { - labels := fixClusterInventoryLabels(kymaName, shootName) + labels := fixGardenerClusterLabels(kymaName, shootName) labels["operator.kyma-project.io/managed-by"] = "infrastructure-manager" labels["operator.kyma-project.io/cluster-name"] = kymaName return labels @@ -178,14 +241,14 @@ func fixSecretLabels(kymaName, shootName string) map[string]string { func fixGardenerClusterCR(kymaName, namespace, shootName, secretName string) imv1.GardenerCluster { return newTestGardenerClusterCR(kymaName, namespace, shootName, secretName). - WithLabels(fixClusterInventoryLabels(kymaName, shootName)).ToCluster() + WithLabels(fixGardenerClusterLabels(kymaName, shootName)).ToCluster() } func fixGardenerClusterCRWithForceRotationAnnotation(kymaName, namespace, shootName, secretName string) imv1.GardenerCluster { annotations := map[string]string{forceKubeconfigRotationAnnotation: "true"} return newTestGardenerClusterCR(kymaName, namespace, shootName, secretName). - WithLabels(fixClusterInventoryLabels(kymaName, shootName)). + WithLabels(fixGardenerClusterLabels(kymaName, shootName)). WithAnnotations(annotations). ToCluster() } @@ -233,7 +296,7 @@ type TestGardenerClusterCR struct { gardenerCluster imv1.GardenerCluster } -func fixClusterInventoryLabels(kymaName, shootName string) map[string]string { +func fixGardenerClusterLabels(kymaName, shootName string) map[string]string { labels := map[string]string{} labels["kyma-project.io/instance-id"] = "instanceID" diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 6df69aa3..542c8d17 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -18,6 +18,7 @@ package controller import ( "context" + "github.com/pkg/errors" "path/filepath" "testing" "time" @@ -102,6 +103,9 @@ func setupKubeconfigProviderMock(kpMock *mocks.KubeconfigProvider) { kpMock.On("Fetch", "shootName1").Return("kubeconfig1", nil) kpMock.On("Fetch", "shootName2").Return("kubeconfig2", nil) kpMock.On("Fetch", "shootName3").Return("kubeconfig3", nil) + kpMock.On("Fetch", "shootName4").Return("kubeconfig4", nil) + kpMock.On("Fetch", "shootName5").Return("kubeconfig5", nil) + kpMock.On("Fetch", "shootName6").Return("", errors.New("failed to get kubeconfig")) } var _ = AfterSuite(func() { From 004260eb7573af5fb8ec6d01594960e2c461e1c0 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 22:25:13 +0200 Subject: [PATCH 28/39] Added new testcases --- internal/controller/gardener_cluster_controller_test.go | 2 +- internal/controller/suite_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index 95393e72..015cecc8 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -2,13 +2,13 @@ package controller import ( "context" - k8serrors "k8s.io/apimachinery/pkg/api/errors" "time" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 542c8d17..69416e81 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -18,7 +18,6 @@ package controller import ( "context" - "github.com/pkg/errors" "path/filepath" "testing" "time" @@ -27,6 +26,7 @@ import ( "github.com/kyma-project/infrastructure-manager/internal/controller/mocks" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/pkg/errors" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" From 9e17f1848891a7f284f003a89e0bea82bebe8435 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 20 Oct 2023 22:59:32 +0200 Subject: [PATCH 29/39] Tests refactored --- .../gardener_cluster_controller_test.go | 28 +++++++++---------- internal/controller/suite_test.go | 4 +-- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index 015cecc8..95f3942a 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -54,9 +54,9 @@ var _ = Describe("Gardener Cluster controller", func() { }) It("Should delete secret", func() { - kymaName := "kymaname5" - secretName := "secret-name5" - shootName := "shootName5" + kymaName := "kymaname2" + secretName := "secret-name2" + shootName := "shootName2" namespace := "default" By("Create GardenerCluster CR") @@ -83,9 +83,9 @@ var _ = Describe("Gardener Cluster controller", func() { }) It("Should set Error status on CR if failed to fetch kubeconfig", func() { - kymaName := "kymaname6" - secretName := "secret-name6" - shootName := "shootName6" + kymaName := "kymaname3" + secretName := "secret-name3" + shootName := "shootName3" namespace := "default" gardenerClusterCR := fixGardenerClusterCR(kymaName, namespace, shootName, secretName) @@ -150,24 +150,24 @@ var _ = Describe("Gardener Cluster controller", func() { Expect(string(kubeconfigSecret.Data["config"])).To(Equal(expectedKubeconfig)) }, Entry("Rotate kubeconfig when rotation time passed", - fixGardenerClusterCR("kymaname2", namespace, "shootName2", "secret-name2"), - fixNewSecret("secret-name2", namespace, "kymaname2", "shootName2", "kubeconfig2", "2023-10-09T23:00:00Z"), - "kubeconfig2"), + fixGardenerClusterCR("kymaname4", namespace, "shootName4", "secret-name4"), + fixNewSecret("secret-name4", namespace, "kymaname4", "shootName4", "kubeconfig4", "2023-10-09T23:00:00Z"), + "kubeconfig4"), Entry("Rotate dynamic kubeconfig", - fixGardenerClusterCRWithForceRotationAnnotation("kymaname3", namespace, "shootName3", "secret-name3"), - fixNewSecret("secret-name3", namespace, "kymaname3", "shootName3", "kubeconfig3", time.Now().UTC().Format(time.RFC3339)), - "kubeconfig3"), + fixGardenerClusterCRWithForceRotationAnnotation("kymaname5", namespace, "shootName5", "secret-name5"), + fixNewSecret("secret-name5", namespace, "kymaname5", "shootName5", "kubeconfig5", time.Now().UTC().Format(time.RFC3339)), + "kubeconfig5"), ) It("Should skip rotation", func() { By("Create kubeconfig secret") - secret := fixNewSecret("secret-name4", namespace, "kymaname4", "shootName4", "kubeconfig4", time.Now().UTC().Format(time.RFC3339)) + secret := fixNewSecret("secret-name6", namespace, "kymaname6", "shootName6", "kubeconfig6", time.Now().UTC().Format(time.RFC3339)) Expect(k8sClient.Create(context.Background(), &secret)).To(Succeed()) previousTimestamp := secret.Annotations[lastKubeconfigSyncAnnotation] By("Create Cluster CR") - gardenerClusterCR := fixGardenerClusterCRWithForceRotationAnnotation("kymaname4", namespace, "shootName4", "secret-name4") + gardenerClusterCR := fixGardenerClusterCRWithForceRotationAnnotation("kymaname6", namespace, "shootName6", "secret-name6") Expect(k8sClient.Create(context.Background(), &gardenerClusterCR)).To(Succeed()) var kubeconfigSecret corev1.Secret diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 69416e81..416764fb 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -102,10 +102,10 @@ var _ = BeforeSuite(func() { func setupKubeconfigProviderMock(kpMock *mocks.KubeconfigProvider) { kpMock.On("Fetch", "shootName1").Return("kubeconfig1", nil) kpMock.On("Fetch", "shootName2").Return("kubeconfig2", nil) - kpMock.On("Fetch", "shootName3").Return("kubeconfig3", nil) + kpMock.On("Fetch", "shootName3").Return("", errors.New("failed to get kubeconfig")) + kpMock.On("Fetch", "shootName6").Return("kubeconfig6", nil) kpMock.On("Fetch", "shootName4").Return("kubeconfig4", nil) kpMock.On("Fetch", "shootName5").Return("kubeconfig5", nil) - kpMock.On("Fetch", "shootName6").Return("", errors.New("failed to get kubeconfig")) } var _ = AfterSuite(func() { From 4af62b710f2f937c2eb538ab46a0ee3243d63667 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 23 Oct 2023 08:16:46 +0200 Subject: [PATCH 30/39] Refactoring --- api/v1/gardenercluster_types.go | 29 +++------- .../controller/gardener_cluster_controller.go | 54 +++++++++---------- 2 files changed, 33 insertions(+), 50 deletions(-) diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go index 9a351f1d..a7f91e29 100644 --- a/api/v1/gardenercluster_types.go +++ b/api/v1/gardenercluster_types.go @@ -70,20 +70,18 @@ type Secret struct { type State string const ( - ReadyState State = "Ready" - ProcessingState State = "Processing" - ErrorState State = "Error" + ReadyState State = "Ready" + ErrorState State = "Error" ) type ConditionReason string const ( - ConditionReasonKubeconfigReadingSecret ConditionReason = "KubeconfigReadingSecret" - ConditionReasonKubeconfigSecretReady ConditionReason = "KubeconfigSecretReady" - ConditionReasonFailedToGetSecret ConditionReason = "FailedToCheckSecret" - ConditionReasonFailedToCreateSecret ConditionReason = "FailedToCreateSecret" - ConditionReasonFailedToUpdateSecret ConditionReason = "FailedToUpdateSecret" - ConditionReasonFailedToGetKubeconfig ConditionReason = "FailedToGetKubeconfig" + ConditionReasonKubeconfigSecretReady ConditionReason = "KubeconfigSecretReady" + ConditionReasonFailedToGetSecret ConditionReason = "FailedToCheckSecret" + ConditionReasonFailedToCreateSecret ConditionReason = "ConditionReasonFailedToCreateSecret" + ConditionReasonFailedToUpdateSecret ConditionReason = "FailedToUpdateSecret" + ConditionReasonFailedToGetKubeconfig ConditionReason = "FailedToGetKubeconfig" ) type ConditionType string @@ -105,19 +103,6 @@ type GardenerClusterStatus struct { Conditions []metav1.Condition `json:"conditions,omitempty"` } -func (cluster *GardenerCluster) UpdateConditionForProcessingState(conditionType ConditionType, reason ConditionReason, conditionStatus metav1.ConditionStatus) { - cluster.Status.State = ProcessingState - - condition := metav1.Condition{ - Type: string(conditionType), - Status: conditionStatus, - LastTransitionTime: metav1.Now(), - Reason: string(reason), - Message: getMessage(reason), - } - meta.SetStatusCondition(&cluster.Status.Conditions, condition) -} - func (cluster *GardenerCluster) UpdateConditionForReadyState(conditionType ConditionType, reason ConditionReason, conditionStatus metav1.ConditionStatus) { cluster.Status.State = ReadyState diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 6598e66a..02b8e5b8 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -90,7 +90,7 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req err := controller.Client.Get(ctx, req.NamespacedName, &cluster) if err != nil { if k8serrors.IsNotFound(err) { - err = controller.deleteSecret(req.NamespacedName.Name) + err = controller.deleteKubeconfigSecret(req.NamespacedName.Name) if err != nil { controller.log.Error(err, "failed to delete secret") } @@ -99,28 +99,27 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req return controller.resultWithoutRequeue(), err } - cluster.UpdateConditionForProcessingState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigReadingSecret, metav1.ConditionTrue) - controller.log.Info("About to create, or rotate secret") lastSyncTime := time.Now() - updateStatus, err := controller.createOrRotateSecret(ctx, &cluster, lastSyncTime) + kubeconfigRotated, err := controller.createOrRotateKubeconfigSecret(ctx, &cluster, lastSyncTime) + if err != nil { + _ = controller.persistStatusChange(ctx, &cluster) - if updateStatus { - statusErr := controller.persistStatusChange(ctx, &cluster) - if statusErr != nil { - controller.log.Error(err, "Failed to set state for GardenerCluster") - if err != nil { - return controller.resultWithoutRequeue(), err - } - return controller.resultWithoutRequeue(), statusErr - } + return controller.resultWithoutRequeue(), err + } - err = controller.removeForceRotationAnnotationIfNeeded(ctx, &cluster) + if kubeconfigRotated { + err = controller.persistStatusChange(ctx, &cluster) if err != nil { return controller.resultWithoutRequeue(), err } } + err = controller.removeForceRotationAnnotation(ctx, &cluster) + if err != nil { + return controller.resultWithoutRequeue(), err + } + return controller.resultWithRequeue(), nil } @@ -132,16 +131,19 @@ func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { } func (controller *GardenerClusterController) resultWithoutRequeue() ctrl.Result { - return ctrl.Result{ - Requeue: false, - } + return ctrl.Result{} } func (controller *GardenerClusterController) persistStatusChange(ctx context.Context, cluster *imv1.GardenerCluster) error { - return controller.Client.Status().Update(ctx, cluster) + statusErr := controller.Client.Status().Update(ctx, cluster) + if statusErr != nil { + controller.log.Error(statusErr, "Failed to set state for GardenerCluster") + } + + return statusErr } -func (controller *GardenerClusterController) deleteSecret(clusterCRName string) error { +func (controller *GardenerClusterController) deleteKubeconfigSecret(clusterCRName string) error { selector := client.MatchingLabels(map[string]string{ clusterCRNameLabel: clusterCRName, }) @@ -184,7 +186,7 @@ func (controller *GardenerClusterController) getSecret(shootName string) (*corev return &secretList.Items[0], nil } -func (controller *GardenerClusterController) createOrRotateSecret(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) (bool, error) { +func (controller *GardenerClusterController) createOrRotateKubeconfigSecret(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) (bool, error) { controller.log.Info("Getting secret") existingSecret, err := controller.getSecret(cluster.Spec.Shoot.Name) if err != nil && !k8serrors.IsNotFound(err) { @@ -257,7 +259,7 @@ func (controller *GardenerClusterController) createNewSecret(ctx context.Context err := controller.Client.Create(ctx, &newSecret) if err != nil { controller.log.Error(err, "Failed to create secret") - cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) + cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, metav1.ConditionTrue, err) return err } @@ -291,14 +293,10 @@ func (controller *GardenerClusterController) updateExistingSecret(ctx context.Co return nil } -func (controller *GardenerClusterController) removeForceRotationAnnotationIfNeeded(ctx context.Context, cluster *imv1.GardenerCluster) error { - annotations := cluster.GetAnnotations() - if annotations == nil { - return nil - } +func (controller *GardenerClusterController) removeForceRotationAnnotation(ctx context.Context, cluster *imv1.GardenerCluster) error { + secretRotationForced := secretRotationForced(cluster) - _, found := annotations[forceKubeconfigRotationAnnotation] - if found { + if secretRotationForced { key := types.NamespacedName{ Name: cluster.Name, Namespace: cluster.Namespace, From f543d06eaf89b43aeae82c3e4fa9abc6eb1f470d Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 23 Oct 2023 09:10:43 +0200 Subject: [PATCH 31/39] Logging improvements --- .../controller/gardener_cluster_controller.go | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 02b8e5b8..ddb1d9b1 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -83,7 +83,7 @@ type KubeconfigProvider interface { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.15.0/pkg/reconcile func (controller *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:revive - controller.log.Info(fmt.Sprintf("Starting reconciliation loop for GardenerCluster resource: %v", req.NamespacedName)) + controller.log.Info("Starting reconciliation.", loggingContext(req)) var cluster imv1.GardenerCluster @@ -91,15 +91,15 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req if err != nil { if k8serrors.IsNotFound(err) { err = controller.deleteKubeconfigSecret(req.NamespacedName.Name) - if err != nil { - controller.log.Error(err, "failed to delete secret") - } + } + + if err == nil { + controller.log.Info("Secret has been deleted.", loggingContext(req)...) } return controller.resultWithoutRequeue(), err } - controller.log.Info("About to create, or rotate secret") lastSyncTime := time.Now() kubeconfigRotated, err := controller.createOrRotateKubeconfigSecret(ctx, &cluster, lastSyncTime) if err != nil { @@ -123,6 +123,14 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req return controller.resultWithRequeue(), nil } +func loggingContextFromCluster(cluster *imv1.GardenerCluster) []interface{} { + return []interface{}{"name", cluster.Name, "namespace", cluster.Namespace} +} + +func loggingContext(req ctrl.Request) []interface{} { + return []interface{}{"name", req.Name, "namespace", req.Namespace} +} + func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { return ctrl.Result{ Requeue: true, @@ -187,22 +195,19 @@ func (controller *GardenerClusterController) getSecret(shootName string) (*corev } func (controller *GardenerClusterController) createOrRotateKubeconfigSecret(ctx context.Context, cluster *imv1.GardenerCluster, lastSyncTime time.Time) (bool, error) { - controller.log.Info("Getting secret") existingSecret, err := controller.getSecret(cluster.Spec.Shoot.Name) if err != nil && !k8serrors.IsNotFound(err) { - controller.log.Error(err, "Failed to get kubeconfig secret") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetSecret, metav1.ConditionTrue, err) return true, err } if !secretNeedsToBeRotated(cluster, existingSecret, controller.rotationPeriod) { - controller.log.Info("Secret rotation skipped") + controller.log.Info("Secret does not need to be rotated yet.", loggingContextFromCluster(cluster)...) return false, nil } kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) if err != nil { - controller.log.Error(err, "Failed to get kubeconfig") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, metav1.ConditionTrue, err) return true, err } @@ -265,6 +270,7 @@ func (controller *GardenerClusterController) createNewSecret(ctx context.Context } cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) + controller.log.Info("New secret with kubeconfig has been created.", loggingContextFromCluster(cluster)...) return nil } @@ -289,6 +295,7 @@ func (controller *GardenerClusterController) updateExistingSecret(ctx context.Co } cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) + controller.log.Info("Secret with kubeconfig has been updated.", loggingContextFromCluster(cluster)...) return nil } From 2fe026329f164b12b11bf608bbad9d5111e5c420 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 23 Oct 2023 09:15:51 +0200 Subject: [PATCH 32/39] Linter --- internal/controller/gardener_cluster_controller.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index ddb1d9b1..589213fc 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -83,7 +83,7 @@ type KubeconfigProvider interface { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.15.0/pkg/reconcile func (controller *GardenerClusterController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:revive - controller.log.Info("Starting reconciliation.", loggingContext(req)) + controller.log.Info("Starting reconciliation.", loggingContext(req)...) var cluster imv1.GardenerCluster @@ -123,12 +123,12 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req return controller.resultWithRequeue(), nil } -func loggingContextFromCluster(cluster *imv1.GardenerCluster) []interface{} { - return []interface{}{"name", cluster.Name, "namespace", cluster.Namespace} +func loggingContextFromCluster(cluster *imv1.GardenerCluster) []any { + return []any{"name", cluster.Name, "namespace", cluster.Namespace} } -func loggingContext(req ctrl.Request) []interface{} { - return []interface{}{"name", req.Name, "namespace", req.Namespace} +func loggingContext(req ctrl.Request) []any { + return []any{"name", req.Name, "namespace", req.Namespace} } func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { From 753d71590be22bedc85aff1803652e037782ce37 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Tue, 24 Oct 2023 09:36:02 +0200 Subject: [PATCH 33/39] Changes to logging --- .../controller/gardener_cluster_controller.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 589213fc..a5a35424 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -124,11 +124,11 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req } func loggingContextFromCluster(cluster *imv1.GardenerCluster) []any { - return []any{"name", cluster.Name, "namespace", cluster.Namespace} + return []any{"GardenerCluster", cluster.Name, "namespace", cluster.Namespace} } func loggingContext(req ctrl.Request) []any { - return []any{"name", req.Name, "namespace", req.Namespace} + return []any{"GardenerCluster", req.Name, "namespace", req.Namespace} } func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { @@ -202,7 +202,8 @@ func (controller *GardenerClusterController) createOrRotateKubeconfigSecret(ctx } if !secretNeedsToBeRotated(cluster, existingSecret, controller.rotationPeriod) { - controller.log.Info("Secret does not need to be rotated yet.", loggingContextFromCluster(cluster)...) + message := fmt.Sprintf("Secret %s in namespace %s does not need to be rotated yet.", existingSecret.Name, existingSecret.Namespace) + controller.log.Info(message, loggingContextFromCluster(cluster)...) return false, nil } @@ -259,24 +260,23 @@ func secretRotationForced(cluster *imv1.GardenerCluster) bool { } func (controller *GardenerClusterController) createNewSecret(ctx context.Context, kubeconfig string, cluster *imv1.GardenerCluster, lastSyncTime time.Time) error { - controller.log.Info("Creating a new kubeconfig secret") newSecret := controller.newSecret(*cluster, kubeconfig, lastSyncTime) err := controller.Client.Create(ctx, &newSecret) if err != nil { - controller.log.Error(err, "Failed to create secret") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToCreateSecret, metav1.ConditionTrue, err) return err } cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) - controller.log.Info("New secret with kubeconfig has been created.", loggingContextFromCluster(cluster)...) + + message := fmt.Sprintf("Secret %s has been created in %s namespace.", newSecret.Name, newSecret.Namespace) + controller.log.Info(message, loggingContextFromCluster(cluster)...) return nil } func (controller *GardenerClusterController) updateExistingSecret(ctx context.Context, kubeconfig string, cluster *imv1.GardenerCluster, existingSecret *corev1.Secret, lastSyncTime time.Time) error { - controller.log.Info("Updating the kubeconfig secret") existingSecret.Data[cluster.Spec.Kubeconfig.Secret.Key] = []byte(kubeconfig) annotations := existingSecret.GetAnnotations() if annotations == nil { @@ -288,14 +288,15 @@ func (controller *GardenerClusterController) updateExistingSecret(ctx context.Co err := controller.Client.Update(ctx, existingSecret) if err != nil { - controller.log.Error(err, "Failed to update secret") cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToUpdateSecret, metav1.ConditionTrue, err) return err } cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) - controller.log.Info("Secret with kubeconfig has been updated.", loggingContextFromCluster(cluster)...) + + message := fmt.Sprintf("Secret %s has been updated in %s namespace.", existingSecret.Name, existingSecret.Namespace) + controller.log.Info(message, loggingContextFromCluster(cluster)...) return nil } From fd8a9101c58c9caa6c8c068e2a6b114ffa7bc4d7 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Tue, 24 Oct 2023 10:58:42 +0200 Subject: [PATCH 34/39] Changes to logging --- internal/controller/gardener_cluster_controller.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index a5a35424..bc846f36 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -202,11 +202,16 @@ func (controller *GardenerClusterController) createOrRotateKubeconfigSecret(ctx } if !secretNeedsToBeRotated(cluster, existingSecret, controller.rotationPeriod) { - message := fmt.Sprintf("Secret %s in namespace %s does not need to be rotated yet.", existingSecret.Name, existingSecret.Namespace) + message := fmt.Sprintf("Secret %s in namespace %s does not need to be rotated yet.", cluster.Spec.Kubeconfig.Secret.Name, cluster.Spec.Kubeconfig.Secret.Namespace) controller.log.Info(message, loggingContextFromCluster(cluster)...) return false, nil } + if secretRotationForced(cluster) { + message := fmt.Sprintf("Rotation of secret %s in namespace %s forced.", cluster.Spec.Kubeconfig.Secret.Name, cluster.Spec.Kubeconfig.Secret.Namespace) + controller.log.Info(message, loggingContextFromCluster(cluster)...) + } + kubeconfig, err := controller.KubeconfigProvider.Fetch(cluster.Spec.Shoot.Name) if err != nil { cluster.UpdateConditionForErrorState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonFailedToGetKubeconfig, metav1.ConditionTrue, err) From 13bf8dc5edf90980411a7a865dd0fb0b1b88101d Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Tue, 24 Oct 2023 11:46:39 +0200 Subject: [PATCH 35/39] Changes to logging --- internal/controller/gardener_cluster_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index bc846f36..e15f74d0 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -124,11 +124,11 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req } func loggingContextFromCluster(cluster *imv1.GardenerCluster) []any { - return []any{"GardenerCluster", cluster.Name, "namespace", cluster.Namespace} + return []any{"GardenerCluster", cluster.Name, "Namespace", cluster.Namespace} } func loggingContext(req ctrl.Request) []any { - return []any{"GardenerCluster", req.Name, "namespace", req.Namespace} + return []any{"GardenerCluster", req.Name, "Namespace", req.Namespace} } func (controller *GardenerClusterController) resultWithRequeue() ctrl.Result { From 732f400571ba8e204ff7a4d5fd9dbbf31bbd3bb3 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Tue, 24 Oct 2023 12:58:14 +0200 Subject: [PATCH 36/39] Changes to statuses --- api/v1/gardenercluster_types.go | 15 +++++++++------ cmd/main.go | 15 ++++++++++----- .../controller/gardener_cluster_controller.go | 12 ++++-------- .../gardener_cluster_controller_test.go | 7 ++++++- internal/controller/suite_test.go | 4 +++- 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go index a7f91e29..3bacd7d5 100644 --- a/api/v1/gardenercluster_types.go +++ b/api/v1/gardenercluster_types.go @@ -77,11 +77,12 @@ const ( type ConditionReason string const ( - ConditionReasonKubeconfigSecretReady ConditionReason = "KubeconfigSecretReady" - ConditionReasonFailedToGetSecret ConditionReason = "FailedToCheckSecret" - ConditionReasonFailedToCreateSecret ConditionReason = "ConditionReasonFailedToCreateSecret" - ConditionReasonFailedToUpdateSecret ConditionReason = "FailedToUpdateSecret" - ConditionReasonFailedToGetKubeconfig ConditionReason = "FailedToGetKubeconfig" + ConditionReasonKubeconfigSecretCreated ConditionReason = "KubeconfigSecretCreated" + ConditionReasonKubeconfigSecretRotated ConditionReason = "KubeconfigSecretRotated" + ConditionReasonFailedToGetSecret ConditionReason = "FailedToCheckSecret" + ConditionReasonFailedToCreateSecret ConditionReason = "ConditionReasonFailedToCreateSecret" + ConditionReasonFailedToUpdateSecret ConditionReason = "FailedToUpdateSecret" + ConditionReasonFailedToGetKubeconfig ConditionReason = "FailedToGetKubeconfig" ) type ConditionType string @@ -113,6 +114,7 @@ func (cluster *GardenerCluster) UpdateConditionForReadyState(conditionType Condi Reason: string(reason), Message: getMessage(reason), } + meta.RemoveStatusCondition(&cluster.Status.Conditions, condition.Type) meta.SetStatusCondition(&cluster.Status.Conditions, condition) } @@ -126,12 +128,13 @@ func (cluster *GardenerCluster) UpdateConditionForErrorState(conditionType Condi Reason: string(reason), Message: fmt.Sprintf("%s Error: %s", getMessage(reason), error.Error()), } + meta.RemoveStatusCondition(&cluster.Status.Conditions, condition.Type) meta.SetStatusCondition(&cluster.Status.Conditions, condition) } func getMessage(reason ConditionReason) string { switch reason { - case ConditionReasonKubeconfigSecretReady: + case ConditionReasonKubeconfigSecretCreated: return "Secret created successfully." case ConditionReasonFailedToGetSecret: return "Failed to get secret." diff --git a/cmd/main.go b/cmd/main.go index 7c3ce0ba..07c3588e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -40,6 +40,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" ) +// The ratio determines what is the minimal time that needs to pass to rotate certificate. +const minimalRotationTimeRatio = 0.6 + var ( scheme = runtime.NewScheme() //nolint:gochecknoglobals setupLog = ctrl.Log.WithName("setup") //nolint:gochecknoglobals @@ -106,15 +109,15 @@ func main() { } gardenerNamespace := fmt.Sprintf("garden-%s", gardenerProjectName) - expirationInSeconds := int64(expirationTime.Seconds()) - kubeconfigProvider, err := setupKubernetesKubeconfigProvider(gardenerKubeconfigPath, gardenerNamespace, expirationInSeconds) + kubeconfigProvider, err := setupKubernetesKubeconfigProvider(gardenerKubeconfigPath, gardenerNamespace, expirationTime) if err != nil { setupLog.Error(err, "unable to initialize kubeconfig provider", "controller", "GardenerCluster") os.Exit(1) } - if err = (controller.NewGardenerClusterController(mgr, kubeconfigProvider, logger, expirationTime)).SetupWithManager(mgr); err != nil { + rotationPeriod := time.Duration(minimalRotationTimeRatio*expirationTime.Minutes()) * time.Minute + if err = (controller.NewGardenerClusterController(mgr, kubeconfigProvider, logger, rotationPeriod)).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "GardenerCluster") os.Exit(1) } @@ -129,6 +132,8 @@ func main() { os.Exit(1) } + setupLog.Info("Starting Manager", "kubeconfigExpirationTime", expirationTime, "kubeconfigRotationPeriod", rotationPeriod) + setupLog.Info("starting manager") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") @@ -136,7 +141,7 @@ func main() { } } -func setupKubernetesKubeconfigProvider(kubeconfigPath string, namespace string, expirationInSeconds int64) (gardener.KubeconfigProvider, error) { +func setupKubernetesKubeconfigProvider(kubeconfigPath string, namespace string, expirationTime time.Duration) (gardener.KubeconfigProvider, error) { restConfig, err := gardener.NewRestConfigFromFile(kubeconfigPath) if err != nil { return gardener.KubeconfigProvider{}, err @@ -163,5 +168,5 @@ func setupKubernetesKubeconfigProvider(kubeconfigPath string, namespace string, return gardener.NewKubeconfigProvider(shootClient, dynamicKubeconfigAPI, namespace, - expirationInSeconds), nil + int64(expirationTime.Seconds())), nil } diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index e15f74d0..dc87026a 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -39,8 +39,6 @@ const ( lastKubeconfigSyncAnnotation = "operator.kyma-project.io/last-sync" forceKubeconfigRotationAnnotation = "operator.kyma-project.io/force-kubeconfig-rotation" clusterCRNameLabel = "operator.kyma-project.io/cluster-name" - // The ratio determines what is the minimal time that needs to pass to rotate certificate. - minimalRotationTimeRatio = 0.6 ) // GardenerClusterController reconciles a GardenerCluster object @@ -52,15 +50,13 @@ type GardenerClusterController struct { rotationPeriod time.Duration } -func NewGardenerClusterController(mgr ctrl.Manager, kubeconfigProvider KubeconfigProvider, logger logr.Logger, kubeconfigExpirationTime time.Duration) *GardenerClusterController { - rotationPeriodInMinutes := int64(minimalRotationTimeRatio * kubeconfigExpirationTime.Minutes()) - +func NewGardenerClusterController(mgr ctrl.Manager, kubeconfigProvider KubeconfigProvider, logger logr.Logger, rotationPeriod time.Duration) *GardenerClusterController { return &GardenerClusterController{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), KubeconfigProvider: kubeconfigProvider, log: logger, - rotationPeriod: time.Duration(rotationPeriodInMinutes) * time.Minute, + rotationPeriod: rotationPeriod, } } @@ -273,7 +269,7 @@ func (controller *GardenerClusterController) createNewSecret(ctx context.Context return err } - cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) + cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretCreated, metav1.ConditionTrue) message := fmt.Sprintf("Secret %s has been created in %s namespace.", newSecret.Name, newSecret.Namespace) controller.log.Info(message, loggingContextFromCluster(cluster)...) @@ -298,7 +294,7 @@ func (controller *GardenerClusterController) updateExistingSecret(ctx context.Co return err } - cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretReady, metav1.ConditionTrue) + cluster.UpdateConditionForReadyState(imv1.ConditionTypeKubeconfigManagement, imv1.ConditionReasonKubeconfigSecretRotated, metav1.ConditionTrue) message := fmt.Sprintf("Secret %s has been updated in %s namespace.", existingSecret.Name, existingSecret.Namespace) controller.log.Info(message, loggingContextFromCluster(cluster)...) diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index 95f3942a..ba006532 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -50,7 +50,9 @@ var _ = Describe("Gardener Cluster controller", func() { expectedSecret := fixNewSecret(secretName, namespace, kymaName, shootName, "kubeconfig1", "") Expect(kubeconfigSecret.Labels).To(Equal(expectedSecret.Labels)) Expect(kubeconfigSecret.Data).To(Equal(expectedSecret.Data)) - Expect(kubeconfigSecret.Annotations[lastKubeconfigSyncAnnotation]).To(Not(BeEmpty())) + lastSyncTime := kubeconfigSecret.Annotations[lastKubeconfigSyncAnnotation] + Expect(lastSyncTime).ToNot(BeEmpty()) + }) It("Should delete secret", func() { @@ -148,6 +150,9 @@ var _ = Describe("Gardener Cluster controller", func() { err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) Expect(err).To(BeNil()) Expect(string(kubeconfigSecret.Data["config"])).To(Equal(expectedKubeconfig)) + lastSyncTime := kubeconfigSecret.Annotations[lastKubeconfigSyncAnnotation] + Expect(lastSyncTime).ToNot(BeEmpty()) + }, Entry("Rotate kubeconfig when rotation time passed", fixGardenerClusterCR("kymaname4", namespace, "shootName4", "secret-name4"), diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 416764fb..15c7c8b3 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -47,6 +47,8 @@ var ( cancelSuiteCtx context.CancelFunc //nolint:gochecknoglobals ) +const TestKubeconfigValidityTime = 24 * time.Hour + func TestControllers(t *testing.T) { RegisterFailHandler(Fail) @@ -78,7 +80,7 @@ var _ = BeforeSuite(func() { kubeconfigProviderMock := &mocks.KubeconfigProvider{} setupKubeconfigProviderMock(kubeconfigProviderMock) - controller := NewGardenerClusterController(mgr, kubeconfigProviderMock, logger, 24*time.Hour) + controller := NewGardenerClusterController(mgr, kubeconfigProviderMock, logger, TestKubeconfigValidityTime) Expect(controller).NotTo(BeNil()) err = controller.SetupWithManager(mgr) From b780cd78cd7a25893af66081747e48bc6e3f2a7b Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Tue, 24 Oct 2023 13:26:34 +0200 Subject: [PATCH 37/39] Setting status message fixed --- api/v1/gardenercluster_types.go | 9 +++++++++ cmd/main.go | 1 - 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/api/v1/gardenercluster_types.go b/api/v1/gardenercluster_types.go index 3bacd7d5..8bb14034 100644 --- a/api/v1/gardenercluster_types.go +++ b/api/v1/gardenercluster_types.go @@ -136,8 +136,17 @@ func getMessage(reason ConditionReason) string { switch reason { case ConditionReasonKubeconfigSecretCreated: return "Secret created successfully." + case ConditionReasonKubeconfigSecretRotated: + return "Secret rotated successfully." + case ConditionReasonFailedToCreateSecret: + return "Failed to create secret." + case ConditionReasonFailedToUpdateSecret: + return "Failed to rotate secret." case ConditionReasonFailedToGetSecret: return "Failed to get secret." + case ConditionReasonFailedToGetKubeconfig: + return "Failed to get kubeconfig." + default: return "Unknown condition" } diff --git a/cmd/main.go b/cmd/main.go index 07c3588e..9fd21517 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -134,7 +134,6 @@ func main() { setupLog.Info("Starting Manager", "kubeconfigExpirationTime", expirationTime, "kubeconfigRotationPeriod", rotationPeriod) - setupLog.Info("starting manager") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") os.Exit(1) From 1d42b16219bdefa9deff4c652bea1fa45a1530d4 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Tue, 24 Oct 2023 14:29:47 +0200 Subject: [PATCH 38/39] Changed operations order: setting status is the last operation. --- .../controller/gardener_cluster_controller.go | 25 ++++++++++++++----- .../gardener_cluster_controller_test.go | 8 +++--- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index dc87026a..835dceee 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -104,6 +104,11 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req return controller.resultWithoutRequeue(), err } + err = controller.removeForceRotationAnnotation(ctx, &cluster) + if err != nil { + return controller.resultWithoutRequeue(), err + } + if kubeconfigRotated { err = controller.persistStatusChange(ctx, &cluster) if err != nil { @@ -111,11 +116,6 @@ func (controller *GardenerClusterController) Reconcile(ctx context.Context, req } } - err = controller.removeForceRotationAnnotation(ctx, &cluster) - if err != nil { - return controller.resultWithoutRequeue(), err - } - return controller.resultWithRequeue(), nil } @@ -139,7 +139,20 @@ func (controller *GardenerClusterController) resultWithoutRequeue() ctrl.Result } func (controller *GardenerClusterController) persistStatusChange(ctx context.Context, cluster *imv1.GardenerCluster) error { - statusErr := controller.Client.Status().Update(ctx, cluster) + key := types.NamespacedName{ + Name: cluster.Name, + Namespace: cluster.Namespace, + } + var clusterToUpdate imv1.GardenerCluster + + err := controller.Client.Get(ctx, key, &clusterToUpdate) + if err != nil { + return err + } + + clusterToUpdate.Status = cluster.Status + + statusErr := controller.Client.Status().Update(ctx, &clusterToUpdate) if statusErr != nil { controller.log.Error(statusErr, "Failed to set state for GardenerCluster") } diff --git a/internal/controller/gardener_cluster_controller_test.go b/internal/controller/gardener_cluster_controller_test.go index ba006532..f60ae6fd 100644 --- a/internal/controller/gardener_cluster_controller_test.go +++ b/internal/controller/gardener_cluster_controller_test.go @@ -145,7 +145,7 @@ var _ = Describe("Gardener Cluster controller", func() { _, forceRotationAnnotationFound := newGardenerCluster.GetAnnotations()[forceKubeconfigRotationAnnotation] return readyState && !forceRotationAnnotationFound - }, time.Second*30, time.Second*3).Should(BeTrue()) + }, time.Second*45, time.Second*3).Should(BeTrue()) err := k8sClient.Get(context.Background(), secretKey, &kubeconfigSecret) Expect(err).To(BeNil()) @@ -158,7 +158,7 @@ var _ = Describe("Gardener Cluster controller", func() { fixGardenerClusterCR("kymaname4", namespace, "shootName4", "secret-name4"), fixNewSecret("secret-name4", namespace, "kymaname4", "shootName4", "kubeconfig4", "2023-10-09T23:00:00Z"), "kubeconfig4"), - Entry("Rotate dynamic kubeconfig", + Entry("Force rotation", fixGardenerClusterCRWithForceRotationAnnotation("kymaname5", namespace, "shootName5", "secret-name5"), fixNewSecret("secret-name5", namespace, "kymaname5", "shootName5", "kubeconfig5", time.Now().UTC().Format(time.RFC3339)), "kubeconfig5"), @@ -172,7 +172,7 @@ var _ = Describe("Gardener Cluster controller", func() { previousTimestamp := secret.Annotations[lastKubeconfigSyncAnnotation] By("Create Cluster CR") - gardenerClusterCR := fixGardenerClusterCRWithForceRotationAnnotation("kymaname6", namespace, "shootName6", "secret-name6") + gardenerClusterCR := fixGardenerClusterCR("kymaname6", namespace, "shootName6", "secret-name6") Expect(k8sClient.Create(context.Background(), &gardenerClusterCR)).To(Succeed()) var kubeconfigSecret corev1.Secret @@ -187,7 +187,7 @@ var _ = Describe("Gardener Cluster controller", func() { timestampAnnotation := kubeconfigSecret.Annotations[lastKubeconfigSyncAnnotation] return timestampAnnotation == previousTimestamp - }, time.Second*30, time.Second*3).Should(BeTrue()) + }, time.Second*45, time.Second*3).Should(BeTrue()) }) }) }) From 0e0d1bc9b1ba930384214821b88d653927d55d96 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Tue, 24 Oct 2023 16:18:12 +0200 Subject: [PATCH 39/39] Fix in rotation problem. --- internal/controller/gardener_cluster_controller.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/controller/gardener_cluster_controller.go b/internal/controller/gardener_cluster_controller.go index 835dceee..5ea16fff 100644 --- a/internal/controller/gardener_cluster_controller.go +++ b/internal/controller/gardener_cluster_controller.go @@ -239,6 +239,8 @@ func secretNeedsToBeRotated(cluster *imv1.GardenerCluster, secret *corev1.Secret } func secretRotationTimePassed(secret *corev1.Secret, rotationPeriod time.Duration) bool { + const rotationPeriodRatio = 0.95 + if secret == nil { return true } @@ -259,7 +261,7 @@ func secretRotationTimePassed(secret *corev1.Secret, rotationPeriod time.Duratio now := time.Now() alreadyValidFor := now.Sub(lastSyncTime) - return alreadyValidFor.Minutes() >= rotationPeriod.Minutes() + return alreadyValidFor.Minutes() >= rotationPeriodRatio*rotationPeriod.Minutes() } func secretRotationForced(cluster *imv1.GardenerCluster) bool {