Feat/rolling-upgrade-deployment (#26)

* feat: Add rolling deployment upgrade instead of deleting a pod on token secret renew
- Replaced restartPods in GithubApp spec with rolloutDeployment
- Ensures pods are gracefully terminated for a deployment
- Deployments podTemplates are updated with a the label ghApplastUpdateTime
- Disabled the test case when running in envtest since it needs the deployment controller

README.md

@@ -30,7 +30,7 @@ Key features:
     - It will default to `5m` if not set
   - `EXPIRY_THRESHOLD` - i.e. to reconcile a new access token if there is less than 10 mins left from expiry, set the value to `10m`
     - It will default to `15m` if not set
-- Optionally, you can enable restart of pods in the same namespace as the `GithubApp` that match any of the labels you define in `spec.restartPods.labels`
+- Optionally, you can enable rolling upgrade to deployments in the same namespace as the `GithubApp` that match any of the labels you define in `spec.rolloutDeployment.labels`
   - This is useful where pods need to be recreated to pickup the new secret data.
 
 ## Example creating a secret to hold a GitHub App private key
@@ -66,8 +66,8 @@ spec:
 EOF
 ```
 
-## Example GithubApp object with pod restart on token renew
-- Below example will restart the pods in the `team-1` namespace when the github token is modified, matching any of labels:
+## Example GithubApp object with pod restart (deployment rolling upgrade) on token renew
+- Below example will upgrade deployments in the `team-1` namespace when the github token is modified, matching any of labels:
   - foo: bar
   - foo2: bar2
 ```sh
@@ -81,7 +81,7 @@ spec:
   appId: 123123
   installId: 12312312
   privateKeySecret: github-app-secret
-  restartPods:
+  rolloutDeployment:
     labels:
       foo: bar
       foo2: bar2
@@ -143,7 +143,7 @@ Current integration tests cover the scenarios:
 - Reconcile of access token is valid.
 - Reconcile error is recorded in a `GithubApp` object's `status.error` field
 - The `status.error` field is cleared on succesful reconcile for a `GithubApp` object.
-- Pods are deleted matching a label if defined in `spec.restartPods.labels` for a `GithubApp`.
+- Pods are deleted matching a label if defined in `spec.rolloutDeployment.labels` for a `GithubApp`.
 
 **Run the controller in the foreground for testing:**
 ```sh

api/v1/githubapp_types.go

@@ -22,10 +22,10 @@ import (
 
 // GithubAppSpec defines the desired state of GithubApp
 type GithubAppSpec struct {
-	AppId            int              `json:"appId"`
-	InstallId        int              `json:"installId"`
-	PrivateKeySecret string           `json:"privateKeySecret"`
-	RestartPods      *RestartPodsSpec `json:"restartPods,omitempty"`
+	AppId             int                    `json:"appId"`
+	InstallId         int                    `json:"installId"`
+	PrivateKeySecret  string                 `json:"privateKeySecret"`
+	RolloutDeployment *RolloutDeploymentSpec `json:"rolloutDeployment,omitempty"`
 }
 
 // GithubAppStatus defines the observed state of GithubApp
@@ -48,8 +48,8 @@ type GithubApp struct {
 	Status GithubAppStatus `json:"status,omitempty"`
 }
 
-// RestartPodsSpec defines the specification for restarting pods
-type RestartPodsSpec struct {
+// RolloutDeploymentSpec defines the specification for restarting pods
+type RolloutDeploymentSpec struct {
 	Labels map[string]string `json:"labels,omitempty"`
 }
 

config/crd/bases/githubapp.samir.io_githubapps.yaml

@@ -45,8 +45,8 @@ spec:
                 type: integer
               privateKeySecret:
                 type: string
-              restartPods:
-                description: RestartPodsSpec defines the specification for restarting
+              rolloutDeployment:
+                description: RolloutDeploymentSpec defines the specification for restarting
                   pods
                 properties:
                   labels:

config/rbac/role.yaml

@@ -7,10 +7,8 @@ rules:
 - apiGroups:
   - ""
   resources:
-  - pods
+  - deployments
   verbs:
-  - create
-  - delete
   - get
   - list
   - patch

internal/controller/githubapp_controller.go

@@ -27,6 +27,7 @@ import (
 	"time"
 
 	githubappv1 "github-app-operator/api/v1"
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -63,7 +64,7 @@ const (
 //+kubebuilder:rbac:groups=githubapp.samir.io,resources=githubapps/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=githubapp.samir.io,resources=githubapps/finalizers,verbs=update
 //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;update;create;delete;watch;patch
-//+kubebuilder:rbac:groups="",resources=pods,verbs=get;list;update;create;delete;watch;patch
+//+kubebuilder:rbac:groups="",resources=deployments,verbs=get;list;update;watch;patch
 
 // Reconcile function
 func (r *GithubAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
@@ -393,8 +394,8 @@ func (r *GithubAppReconciler) generateOrUpdateAccessToken(ctx context.Context, g
 				return fmt.Errorf("failed after creating secret: %v", err)
 			}
 			// Restart the pods is required
-			if err := r.restartPods(ctx, githubApp); err != nil {
-				return fmt.Errorf("failed to restart pods after after creating secret: %v", err)
+			if err := r.rolloutDeployment(ctx, githubApp); err != nil {
+				return fmt.Errorf("failed to rollout deployment after after creating secret: %v", err)
 			}
 			return nil
 		}
@@ -431,8 +432,8 @@ func (r *GithubAppReconciler) generateOrUpdateAccessToken(ctx context.Context, g
 		return fmt.Errorf("failed after updating secret: %v", err)
 	}
 	// Restart the pods is required
-	if err := r.restartPods(ctx, githubApp); err != nil {
-		return fmt.Errorf("failed to restart pods after updating secret: %v", err)
+	if err := r.rolloutDeployment(ctx, githubApp); err != nil {
+		return fmt.Errorf("failed to rollout deployment after updating secret: %v", err)
 	}
 
 	l.Info("Access token updated in the existing Secret successfully")
@@ -533,40 +534,53 @@ func generateAccessToken(ctx context.Context, appID int, installationID int, pri
 	return accessToken, metav1.NewTime(expiresAt), nil
 }
 
-// Function to bounce pods as per `spec.restartPods.labels` in GithubApp (in the same namespace)
-func (r *GithubAppReconciler) restartPods(ctx context.Context, githubApp *githubappv1.GithubApp) error {
+// Function to bounce pods as per `spec.rolloutDeployment.labels` in GithubApp (in the same namespace)
+func (r *GithubAppReconciler) rolloutDeployment(ctx context.Context, githubApp *githubappv1.GithubApp) error {
 	l := log.FromContext(ctx)
 
-	// Check if restartPods field is defined
-	if githubApp.Spec.RestartPods == nil || len(githubApp.Spec.RestartPods.Labels) == 0 {
-		// No action needed if restartPods is not defined or no labels are specified
+	// Check if rolloutDeployment field is defined
+	if githubApp.Spec.RolloutDeployment == nil || len(githubApp.Spec.RolloutDeployment.Labels) == 0 {
+		// No action needed if rolloutDeployment is not defined or no labels are specified
 		return nil
 	}
 
-	// Loop through each label specified in restartPods.labels and restart pods matching each label
-	for key, value := range githubApp.Spec.RestartPods.Labels {
+	// Loop through each label specified in rolloutDeployment.labels and restart pods matching each label
+	for key, value := range githubApp.Spec.RolloutDeployment.Labels {
 		// Create a list options with label selector
 		listOptions := &client.ListOptions{
 			Namespace:     githubApp.Namespace,
 			LabelSelector: labels.SelectorFromSet(map[string]string{key: value}),
 		}
 
-		// List pods with the label selector
-		podList := &corev1.PodList{}
-		if err := r.List(ctx, podList, listOptions); err != nil {
-			return fmt.Errorf("failed to list pods with label %s=%s: %v", key, value, err)
+		// List Deployments with the label selector
+		deploymentList := &appsv1.DeploymentList{}
+		if err := r.List(ctx, deploymentList, listOptions); err != nil {
+			return fmt.Errorf("failed to list Deployments with label %s=%s: %v", key, value, err)
 		}
 
-		// Restart each pod by deleting it
-		for _, pod := range podList.Items {
-			// Set deletion timestamp on the pod
-			if err := r.Delete(ctx, &pod); err != nil {
-				return fmt.Errorf("failed to delete pod %s/%s: %v", pod.Namespace, pod.Name, err)
+		// Trigger rolling upgrade for matching deployments
+		for _, deployment := range deploymentList.Items {
+
+			// Add a timestamp label to trigger a rolling upgrade
+			deployment.Spec.Template.ObjectMeta.Labels["ghApplastUpdateTime"] = time.Now().Format("20060102150405")
+
+			// Patch the Deployment
+			if err := r.Update(ctx, &deployment); err != nil {
+				return fmt.Errorf(
+					"failed to upgrade deployment %s/%s: %v",
+					deployment.Namespace,
+					deployment.Name,
+					err,
+				)
 			}
-			// Log pod deletion
+
+			// Log deployment upgrade
 			l.Info(
-				"Pod marked for deletion to refresh secret",
-				"Pod Name", pod.Name,
+				"Deployment rolling upgrade triggered",
+				"Name",
+				deployment.Name,
+				"Namespace",
+				deployment.Namespace,
 			)
 		}
 	}

internal/controller/githubapp_controller_test.go

@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"os"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -155,8 +156,12 @@ var _ = Describe("GithubApp controller", func() {
 		})
 	})
 
-	Context("When reconciling a GithubApp with spec.restartPods.labels.foo as bar", func() {
-		It("Should eventually delete the pod with the matching label foo: bar", func() {
+	Context("When reconciling a GithubApp with spec.rolloutDeployment.labels.foo as bar", func() {
+		It("Should eventually upgrade the deployment matching label foo: bar", func() {
+			if os.Getenv("USE_EXISTING_CLUSTER") == "" {
+				fmt.Println("Skipping deployment rollout test case as not a real cluster...")
+				return // Skip the test case in envtest since required deployment controller
+			}
 			ctx := context.Background()
 
 			By("Creating a new namespace")
@@ -165,20 +170,20 @@ var _ = Describe("GithubApp controller", func() {
 			By("Creating the privateKeySecret in namespace2")
 			test_helpers.CreatePrivateKeySecret(ctx, k8sClient, namespace2, "privateKey")
 
-			By("Creating a pod with the label foo: bar")
-			pod1 := test_helpers.CreatePodWithLabel(ctx, k8sClient, "foo", namespace2, "foo", "bar")
+			By("Creating a deployment with the label foo: bar")
+			deploy1, pod1 := test_helpers.CreateDeploymentWithLabel(ctx, k8sClient, "foo", namespace2, "foo", "bar")
 
-			By("Creating a pod with the label foo: bar2")
-			pod2 := test_helpers.CreatePodWithLabel(ctx, k8sClient, "foo", namespace2, "foo", "bar2")
+			By("Creating a deployment with the label foo: bar2")
+			deploy2, pod2 := test_helpers.CreateDeploymentWithLabel(ctx, k8sClient, "foo2", namespace2, "foo", "bar2")
 
-			By("Creating a GithubApp with the spec.restartPods.labels foo: bar")
-			restartPodsSpec := &githubappv1.RestartPodsSpec{
+			By("Creating a GithubApp with the spec.rolloutDeployment.labels foo: bar")
+			rolloutDeploymentSpec := &githubappv1.RolloutDeploymentSpec{
 				Labels: map[string]string{
 					"foo": "bar",
 				},
 			}
-			// Create a GithubApp instance with the RestartPods field initialized
-			test_helpers.CreateGitHubAppAndWait(ctx, k8sClient, namespace2, githubAppName2, restartPodsSpec)
+			// Create a GithubApp instance with the RolloutDeployment field initialized
+			test_helpers.CreateGitHubAppAndWait(ctx, k8sClient, namespace2, githubAppName2, rolloutDeploymentSpec)
 
 			By("Waiting for pod1 with the label 'foo: bar' to be deleted")
 			// Wait for the pod to be deleted by the reconcile loop
@@ -198,9 +203,13 @@ var _ = Describe("GithubApp controller", func() {
 				return pod2.DeletionTimestamp == nil
 			}, "10s", "2s").Should(BeTrue(), "Pod2 is marked for deletion")
 
-			// Delete pod2
-			err := k8sClient.Delete(ctx, pod2)
-			Expect(err).ToNot(HaveOccurred(), "Failed to delete pod2: %v", err)
+			// Delete deploy1
+			err := k8sClient.Delete(ctx, deploy1)
+			Expect(err).ToNot(HaveOccurred(), "Failed to delete deploy1: %v", err)
+
+			// Delete deploy2
+			err = k8sClient.Delete(ctx, deploy2)
+			Expect(err).ToNot(HaveOccurred(), "Failed to delete deploy2: %v", err)
 
 			// Delete the GitHubApp after reconciliation
 			test_helpers.DeleteGitHubAppAndWait(ctx, k8sClient, namespace2, githubAppName2)