Merge branch 'open-cluster-management-io:main' into main

stolostron · Sep 12, 2024 · 44d20fa · 44d20fa
2 parents a96c159 + 5c7466e
commit 44d20fa
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 12 deletions.
diff --git a/.github/workflows/go-presubmit.yml b/.github/workflows/go-presubmit.yml
@@ -94,8 +94,7 @@ jobs:
              -n open-cluster-management-addon --create-namespace \
              managed-serviceaccount charts/managed-serviceaccount/ \
              --set tag=latest \
-             --set featureGates.ephemeralIdentity=true \
-             --set enableAddOnDeploymentConfig=true
+             --set featureGates.ephemeralIdentity=true
       - name: Run e2e test
         run: make test-e2e GENKGO_ARGS='--ginkgo.label-filter='\''!template-install'\'''
 
@@ -133,7 +132,6 @@ jobs:
             managed-serviceaccount charts/managed-serviceaccount/ \
             --set tag=latest \
             --set featureGates.ephemeralIdentity=true \
-            --set enableAddOnDeploymentConfig=true \
             --set hubDeployMode=AddOnTemplate \
             --set targetCluster=loopback
       - name: Run e2e test

diff --git a/charts/managed-serviceaccount/Chart.yaml b/charts/managed-serviceaccount/Chart.yaml
@@ -2,5 +2,5 @@ apiVersion: v2
 name: managed-serviceaccount
 description: A Helm chart for Managed ServiceAccount Addon
 type: application
-version: 0.5.0
+version: 0.7.0
 appVersion: 1.0.0
diff --git a/charts/managed-serviceaccount/templates/clustermanagementaddon.yaml b/charts/managed-serviceaccount/templates/clustermanagementaddon.yaml
@@ -10,11 +10,9 @@ spec:
   addOnMeta:
     displayName: managed-serviceaccount
     description: managed-serviceaccount
-{{- if .Values.enableAddOnDeploymentConfig }}
   supportedConfigs:
   - group: addon.open-cluster-management.io
     resource: addondeploymentconfigs
-{{- end }}
 {{- if eq .Values.hubDeployMode "AddOnTemplate" }}
   - group: addon.open-cluster-management.io
     resource: addontemplates

diff --git a/charts/managed-serviceaccount/values.yaml b/charts/managed-serviceaccount/values.yaml
@@ -6,8 +6,6 @@ agentInstallAll: true
 # Number of replicas
 replicas: 1
 
-enableAddOnDeploymentConfig: false
-
 featureGates:
   ephemeralIdentity: false
 

diff --git a/pkg/addon/agent/controller/token.go b/pkg/addon/agent/controller/token.go
@@ -3,6 +3,8 @@ package controller
 import (
 	"context"
 	"os"
+	"reflect"
+	"time"
 
 	"github.com/pkg/errors"
 	authv1 "k8s.io/api/authentication/v1"
@@ -137,12 +139,30 @@ func (r *TokenReconciler) Reconcile(ctx context.Context, request reconcile.Reque
 		LastRefreshTimestamp: tokenRefreshTime,
 	}
 
-	if err := r.HubClient.Status().Update(context.TODO(), msaCopy); err != nil {
-		return reconcile.Result{}, errors.Wrapf(err, "failed to update status")
+	if !reflect.DeepEqual(msa.Status, msaCopy.Status) {
+		if err := r.HubClient.Status().Update(context.TODO(), msaCopy); err != nil {
+			return reconcile.Result{}, errors.Wrapf(err, "failed to update status")
+		}
+		logger.Info("Token refreshed")
+		return reconcile.Result{}, nil
 	}
 
-	logger.Info("Refreshed token")
-	return reconcile.Result{}, nil
+	return reconcile.Result{
+		// Requeue even if the token is not refreshed, otherwise if the agent restarts
+		// at the time that the token is not expried, no chance to trigger the expiration
+		// check again
+		RequeueAfter: checkTokenRefreshAfter(now, expiring, msa.Spec.Rotation.Validity.Duration),
+	}, nil
+}
+
+func checkTokenRefreshAfter(now metav1.Time, expiring *metav1.Time, validityDuration time.Duration) time.Duration {
+	refreshThreshold := validityDuration / 5 * 1
+	lifetime := expiring.Sub(now.Time)
+	if (lifetime - refreshThreshold) > 0 {
+		return lifetime - refreshThreshold + time.Duration(5*time.Second)
+	} else {
+		return time.Duration(5 * time.Second)
+	}
 }
 
 // sync is the main logic of token rotation, it returns the expiration time of the token if the token is created/updated

diff --git a/pkg/addon/agent/controller/token_test.go b/pkg/addon/agent/controller/token_test.go
@@ -649,3 +649,36 @@ func TestReconcileCreateTokenByDefaultSecret(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckTokenRefreshAfter(t *testing.T) {
+	now := metav1.Time{Time: time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)}
+	cases := []struct {
+		name                 string
+		expiring             *metav1.Time
+		validityDuration     time.Duration
+		expectedRequeueAfter time.Duration
+	}{
+		{
+			name:                 "expired",
+			expiring:             &metav1.Time{Time: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)},
+			validityDuration:     10 * time.Hour,
+			expectedRequeueAfter: 5 * time.Second,
+		},
+		{
+			name:                 "not expired",
+			expiring:             &metav1.Time{Time: time.Date(2024, 1, 1, 9, 0, 0, 0, time.UTC)},
+			validityDuration:     10 * time.Hour,
+			expectedRequeueAfter: 7*time.Hour + 5*time.Second,
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+
+			ra := checkTokenRefreshAfter(now, c.expiring, c.validityDuration)
+			if ra != c.expectedRequeueAfter {
+				t.Errorf("expected %v but got %v", c.expectedRequeueAfter, ra)
+			}
+
+		})
+	}
+}