Smaller refactoring, add tests

Author: tom1299

internal/pkg/handler/pause_deployment.go

@@ -3,7 +3,6 @@ package handler
 import (
 	"context"
 	"fmt"
-	"sync"
 	"time"
 
 	"github.com/sirupsen/logrus"
@@ -15,11 +14,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
-// Keeps track of currently active timers for paused deployments
-var (
-	activeTimers = make(map[string]*time.Timer)
-	timerLock    = sync.Mutex{}
-)
+// Keeps track of currently active timers
+var activeTimers = make(map[string]*time.Timer)
 
 // Returns unique key for the activeTimers map
 func getTimerKey(namespace, deploymentName string) string {
@@ -98,9 +94,7 @@ func PauseDeployment(deployment *app.Deployment, clients kube.Clients, deploymen
 
 		// Check if a timer already exists for this deployment
 		timerKey := getTimerKey(namespace, deploymentName)
-		timerLock.Lock()
 		_, timerExists := activeTimers[timerKey]
-		timerLock.Unlock()
 
 		if !timerExists {
 			logrus.Warnf("Timer does not exist for already paused deployment '%s' in namespace '%s', creating new one",
@@ -121,17 +115,17 @@ func HandleMissingTimer(deployment *app.Deployment, pauseDuration time.Duration,
 			deploymentName, namespace, err)
 		ResumeDeployment(deploymentName, namespace, clients)
 	} else if pauseStartTime != nil {
-		elapsedTime := time.Since(*pauseStartTime)
-		remainingTime := pauseDuration - elapsedTime
+		elapsedPauseTime := time.Since(*pauseStartTime)
+		remainingPauseTime := pauseDuration - elapsedPauseTime
 
-		if remainingTime <= 0 {
+		if remainingPauseTime <= 0 {
 			logrus.Infof("Pause period for deployment '%s' in namespace '%s' has expired. Resuming immediately",
 				deploymentName, namespace)
 			ResumeDeployment(deploymentName, namespace, clients)
 		} else {
 			logrus.Infof("Creating missing timer for already paused deployment '%s' in namespace '%s' with remaining time %s",
-				deploymentName, namespace, remainingTime)
-			CreateResumeTimer(deployment, clients, deploymentName, namespace, remainingTime)
+				deploymentName, namespace, remainingPauseTime)
+			CreateResumeTimer(deployment, clients, deploymentName, namespace, remainingPauseTime)
 		}
 	}
 }
@@ -141,7 +135,6 @@ func CreateResumeTimer(deployment *app.Deployment, clients kube.Clients, deploym
 	timerKey := getTimerKey(namespace, deploymentName)
 
 	// Check if there's an existing timer for this deployment
-	timerLock.Lock()
 	if _, exists := activeTimers[timerKey]; exists {
 		logrus.Debugf("Timer already exists for deployment '%s' in namespace '%s', Skipping creation",
 			deploymentName, namespace)
@@ -155,7 +148,6 @@ func CreateResumeTimer(deployment *app.Deployment, clients kube.Clients, deploym
 
 	// Add the new timer to the map
 	activeTimers[timerKey] = timer
-	timerLock.Unlock()
 
 	logrus.Debugf("Created pause timer for deployment '%s' in namespace '%s' with duration %s",
 		deploymentName, namespace, pauseDuration)
@@ -176,13 +168,11 @@ func ResumeDeployment(deploymentName, namespace string, clients kube.Clients) {
 
 	// Remove the timer itself
 	timerKey := getTimerKey(namespace, deploymentName)
-	timerLock.Lock()
 	if timer, exists := activeTimers[timerKey]; exists {
 		timer.Stop()
 		delete(activeTimers, timerKey)
 		logrus.Debugf("Removed pause timer for deployment '%s' in namespace '%s'", deploymentName, namespace)
 	}
-	timerLock.Unlock()
 
 	deployment.Spec.Paused = false
 	delete(deployment.Annotations, options.PauseDeploymentTimeAnnotation)

internal/pkg/handler/pause_deployment_test.go

@@ -1,14 +1,17 @@
 package handler
 
 import (
+	"context"
 	"testing"
 	"time"
 
 	"github.com/stakater/Reloader/internal/pkg/options"
+	"github.com/stakater/Reloader/pkg/kube"
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	testclient "k8s.io/client-go/kubernetes/fake"
 )
 
 func TestIsPaused(t *testing.T) {
@@ -253,3 +256,72 @@ func TestParsePauseDuration(t *testing.T) {
 		})
 	}
 }
+
+func TestHandleMissingTimerSimple(t *testing.T) {
+	tests := []struct {
+		name           string
+		deployment     *appsv1.Deployment
+		shouldBePaused bool // Should be unpaused after HandleMissingTimer ?
+	}{
+		{
+			name: "deployment paused by reloader, pause period has expired and no timer",
+			deployment: &appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-deployment",
+					Annotations: map[string]string{
+						options.PauseDeploymentTimeAnnotation: time.Now().Add(-6 * time.Minute).Format(time.RFC3339),
+						options.PauseDeploymentAnnotation:     "5m",
+					},
+				},
+				Spec: appsv1.DeploymentSpec{
+					Paused: true,
+				},
+			},
+			shouldBePaused: false,
+		},
+		{
+			name: "deployment paused by reloader, pause period expires in the future and no timer",
+			deployment: &appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-deployment",
+					Annotations: map[string]string{
+						options.PauseDeploymentTimeAnnotation: time.Now().Add(1 * time.Minute).Format(time.RFC3339),
+						options.PauseDeploymentAnnotation:     "5m",
+					},
+				},
+				Spec: appsv1.DeploymentSpec{
+					Paused: true,
+				},
+			},
+			shouldBePaused: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			fakeClient := testclient.NewSimpleClientset()
+			clients := kube.Clients{
+				KubernetesClient: fakeClient,
+			}
+
+			fakeClient.AppsV1().Deployments("default").Create(
+				context.TODO(),
+				test.deployment,
+				metav1.CreateOptions{})
+
+			pauseDuration, _ := ParsePauseDuration(test.deployment.Annotations[options.PauseDeploymentAnnotation])
+			HandleMissingTimer(test.deployment, pauseDuration, clients, test.deployment.Name, "default")
+
+			updatedDeployment, _ := fakeClient.AppsV1().Deployments("default").Get(context.TODO(), test.deployment.Name, metav1.GetOptions{})
+
+			assert.Equal(t, test.shouldBePaused, updatedDeployment.Spec.Paused,
+				"Deployment should have correct paused state after timer expiration")
+
+			if test.shouldBePaused {
+				pausedAtAnnotationValue := updatedDeployment.Annotations[options.PauseDeploymentTimeAnnotation]
+				assert.NotEmpty(t, pausedAtAnnotationValue,
+					"Pause annotation should be present and contain a value when deployment is paused")
+			}
+		})
+	}
+}