From 8359d875226f9947f7e90ff8f9c083ee862f392c Mon Sep 17 00:00:00 2001 From: tom1299 Date: Tue, 18 Feb 2025 17:29:16 +0100 Subject: [PATCH 1/8] Add pausing deployments during upgrades --- internal/pkg/handler/pause_deployment.go | 78 +++++++++++++ internal/pkg/handler/upgrade.go | 21 ++++ internal/pkg/handler/upgrade_test.go | 142 +++++++++++++++++++++++ internal/pkg/options/flags.go | 6 + internal/pkg/testutil/kube.go | 20 ++++ 5 files changed, 267 insertions(+) create mode 100644 internal/pkg/handler/pause_deployment.go diff --git a/internal/pkg/handler/pause_deployment.go b/internal/pkg/handler/pause_deployment.go new file mode 100644 index 000000000..b6b06e6c4 --- /dev/null +++ b/internal/pkg/handler/pause_deployment.go @@ -0,0 +1,78 @@ +package handler + +import ( + "context" + "time" + + "github.com/sirupsen/logrus" + "github.com/stakater/Reloader/internal/pkg/options" + "github.com/stakater/Reloader/pkg/kube" + app "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func PauseDeployment(deployment *app.Deployment, clients kube.Clients, deploymentName, namespace, pauseIntervalValue string) error { + pauseDuration, err := ParsePauseDuration(pauseIntervalValue) + if err != nil { + return err + } + + if !deployment.Spec.Paused { + deployment.Spec.Paused = true + logrus.Infof("Pausing Deployment '%s' in namespace '%s' for %s seconds", deploymentName, namespace, pauseDuration) + + if deployment.Annotations == nil { + deployment.Annotations = make(map[string]string) + } + deployment.Annotations[options.PauseDeploymentTimeAnnotation] = time.Now().Format(time.RFC3339) + + CreateResumeTimer(deployment, clients, deploymentName, namespace, pauseDuration) + } else { + logrus.Infof("Deployment '%s' in namespace '%s' is already paused", deploymentName, namespace) + } + return nil +} + +func CreateResumeTimer(deployment *app.Deployment, clients kube.Clients, deploymentName, namespace string, pauseDuration time.Duration) { + time.AfterFunc(pauseDuration, func() { + ResumeDeployment(deploymentName, namespace, clients) + }) +} + +func ResumeDeployment(deploymentName, namespace string, clients kube.Clients) { + deployment, err := clients.KubernetesClient.AppsV1().Deployments(namespace).Get(context.TODO(), deploymentName, metav1.GetOptions{}) + if err != nil { + logrus.Errorf("Failed to get deployment '%s' in namespace '%s': %v", deploymentName, namespace, err) + return + } + + if !deployment.Spec.Paused { + logrus.Infof("Deployment '%s' in namespace '%s' not paused. Skipping resume", deploymentName, namespace) + return + } + + pausedAtAnnotationValue := deployment.Annotations[options.PauseDeploymentTimeAnnotation] + if pausedAtAnnotationValue == "" { + logrus.Infof("Deployment '%s' in namespace '%s' was not paused by Reloader. Skipping resume", deploymentName, namespace) + return + } + + deployment.Spec.Paused = false + delete(deployment.Annotations, options.PauseDeploymentTimeAnnotation) + + _, err = clients.KubernetesClient.AppsV1().Deployments(namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{}) + if err != nil { + logrus.Errorf("Failed to resume deployment '%s' in namespace '%s': %v", deploymentName, namespace, err) + } + + logrus.Infof("Successfully resumed deployment '%s' in namespace '%s'", deploymentName, namespace) +} + +func ParsePauseDuration(pauseIntervalValue string) (time.Duration, error) { + pauseDuration, err := time.ParseDuration(pauseIntervalValue) + if err != nil { + logrus.Warnf("Failed to parse pause interval value '%s': %v", pauseIntervalValue, err) + return 0, err + } + return pauseDuration, nil +} diff --git a/internal/pkg/handler/upgrade.go b/internal/pkg/handler/upgrade.go index 8365fb547..3025ff8cb 100644 --- a/internal/pkg/handler/upgrade.go +++ b/internal/pkg/handler/upgrade.go @@ -21,6 +21,7 @@ import ( "github.com/stakater/Reloader/internal/pkg/options" "github.com/stakater/Reloader/internal/pkg/util" "github.com/stakater/Reloader/pkg/kube" + app "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" @@ -219,6 +220,7 @@ func PerformAction(clients kube.Clients, config util.Config, upgradeFuncs callba typedAutoAnnotationEnabledValue, foundTypedAuto := annotations[config.TypedAutoAnnotation] excludeConfigmapAnnotationValue, foundExcludeConfigmap := annotations[options.ConfigmapExcludeReloaderAnnotation] excludeSecretAnnotationValue, foundExcludeSecret := annotations[options.SecretExcludeReloaderAnnotation] + pauseInterval, foundPauseInterval := annotations[options.PauseDeploymentAnnotation] if !found && !foundAuto && !foundTypedAuto && !foundSearchAnn { annotations = upgradeFuncs.PodAnnotationsFunc(i) @@ -274,6 +276,25 @@ func PerformAction(clients kube.Clients, config util.Config, upgradeFuncs callba } if result == constants.Updated { + + if foundPauseInterval { + + accessor, err := meta.Accessor(i) + if err != nil { + return err + } + + itemName := accessor.GetName() + itemNamespace := accessor.GetNamespace() + + deployment, ok := i.(*app.Deployment) + if !ok { + logrus.Warnf("Annotation '%s' only applicable for deployments", options.PauseDeploymentAnnotation) + } else { + PauseDeployment(deployment, clients, itemName, itemNamespace, pauseInterval) + } + } + accessor, err := meta.Accessor(i) if err != nil { return err diff --git a/internal/pkg/handler/upgrade_test.go b/internal/pkg/handler/upgrade_test.go index 2b71740d2..d87dc0fe1 100644 --- a/internal/pkg/handler/upgrade_test.go +++ b/internal/pkg/handler/upgrade_test.go @@ -17,6 +17,7 @@ import ( "github.com/stakater/Reloader/internal/pkg/testutil" "github.com/stakater/Reloader/internal/pkg/util" "github.com/stakater/Reloader/pkg/kube" + appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/meta" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -49,6 +50,7 @@ var ( arsConfigmapWithConfigMapAutoAnnotation = "testconfigmapwithconfigmapautoannotationdeployment-handler-" + testutil.RandSeq(5) arsSecretWithExcludeSecretAnnotation = "testsecretwithsecretexcludeannotationdeployment-handler-" + testutil.RandSeq(5) arsConfigmapWithExcludeConfigMapAnnotation = "testconfigmapwithconfigmapexcludeannotationdeployment-handler-" + testutil.RandSeq(5) + arsConfigmapWithPausedDeployment = "testconfigmapWithPausedDeployment-handler-" + testutil.RandSeq(5) ersNamespace = "test-handler-" + testutil.RandSeq(5) ersConfigmapName = "testconfigmap-handler-" + testutil.RandSeq(5) @@ -72,6 +74,7 @@ var ( ersConfigmapWithConfigMapAutoAnnotation = "testconfigmapwithconfigmapautoannotationdeployment-handler-" + testutil.RandSeq(5) ersSecretWithSecretExcludeAnnotation = "testsecretwithsecretexcludeannotationdeployment-handler-" + testutil.RandSeq(5) ersConfigmapWithConfigMapExcludeAnnotation = "testconfigmapwithconfigmapexcludeannotationdeployment-handler-" + testutil.RandSeq(5) + ersConfigmapWithPausedDeployment = "testconfigmapWithPausedDeployment-handler-" + testutil.RandSeq(5) ) func TestMain(m *testing.M) { @@ -200,6 +203,12 @@ func setupArs() { logrus.Errorf("Error in configmap creation: %v", err) } + // Creating configmap for testing pausing deployments + _, err = testutil.CreateConfigMap(clients.KubernetesClient, arsNamespace, arsConfigmapWithPausedDeployment, "www.google.com") + if err != nil { + logrus.Errorf("Error in configmap creation: %v", err) + } + // Creating secret used with secret auto annotation _, err = testutil.CreateSecret(clients.KubernetesClient, arsNamespace, arsSecretWithExcludeSecretAnnotation, data) if err != nil { @@ -421,6 +430,12 @@ func setupArs() { if err != nil { logrus.Errorf("Error in Deployment with both annotations: %v", err) } + + // Creating Deployment with pause annotation + _, err = testutil.CreateDeploymentWithAnnotations(clients.KubernetesClient, arsConfigmapWithPausedDeployment, arsNamespace, map[string]string{options.PauseDeploymentAnnotation: "10s"}, false) + if err != nil { + logrus.Errorf("Error in Deployment with configmap creation: %v", err) + } } func teardownArs() { @@ -622,6 +637,12 @@ func teardownArs() { logrus.Errorf("Error while deleting statefulSet with secret as env var source %v", statefulSetError) } + // Deleting Deployment with pasuse annotation + deploymentError = testutil.DeleteDeployment(clients.KubernetesClient, arsNamespace, arsConfigmapWithPausedDeployment) + if deploymentError != nil { + logrus.Errorf("Error while deleting deployment with configmap %v", deploymentError) + } + // Deleting Configmap err := testutil.DeleteConfigMap(clients.KubernetesClient, arsNamespace, arsConfigmapName) if err != nil { @@ -735,6 +756,12 @@ func teardownArs() { logrus.Errorf("Error while deleting the configmap used with configmap auto annotations: %v", err) } + // Deleting configmap for testing pausing deployments + err = testutil.DeleteConfigMap(clients.KubernetesClient, arsNamespace, arsConfigmapWithPausedDeployment) + if err != nil { + logrus.Errorf("Error while deleting the configmap: %v", err) + } + // Deleting namespace testutil.DeleteNamespace(arsNamespace, clients.KubernetesClient) @@ -794,6 +821,12 @@ func setupErs() { logrus.Errorf("Error in configmap creation: %v", err) } + // Creating configmap for testing pausing deployments + _, err = testutil.CreateConfigMap(clients.KubernetesClient, ersNamespace, ersConfigmapWithPausedDeployment, "www.google.com") + if err != nil { + logrus.Errorf("Error in configmap creation: %v", err) + } + // Creating secret _, err = testutil.CreateSecret(clients.KubernetesClient, ersNamespace, ersSecretWithInitEnv, data) if err != nil { @@ -970,6 +1003,12 @@ func setupErs() { logrus.Errorf("Error in Deployment with configmap and with configmap exclude annotation: %v", err) } + // Creating Deployment with pause annotation + _, err = testutil.CreateDeploymentWithAnnotations(clients.KubernetesClient, ersConfigmapWithPausedDeployment, ersNamespace, map[string]string{options.PauseDeploymentAnnotation: "10s"}, false) + if err != nil { + logrus.Errorf("Error in Deployment with configmap creation: %v", err) + } + // Creating DaemonSet with configmap _, err = testutil.CreateDaemonSet(clients.KubernetesClient, ersConfigmapName, ersNamespace, true) if err != nil { @@ -1254,6 +1293,12 @@ func teardownErs() { logrus.Errorf("Error while deleting statefulSet with secret as env var source %v", statefulSetError) } + // Deleting Deployment for testing pausing deployments + deploymentError = testutil.DeleteDeployment(clients.KubernetesClient, ersNamespace, ersConfigmapWithPausedDeployment) + if deploymentError != nil { + logrus.Errorf("Error while deleting deployment with configmap %v", deploymentError) + } + // Deleting Configmap err := testutil.DeleteConfigMap(clients.KubernetesClient, ersNamespace, ersConfigmapName) if err != nil { @@ -1367,6 +1412,12 @@ func teardownErs() { logrus.Errorf("Error while deleting the configmap used with configmap exclude annotation: %v", err) } + // Deleting ConfigMap for testins pausing deployments + err = testutil.DeleteConfigMap(clients.KubernetesClient, ersNamespace, ersConfigmapWithPausedDeployment) + if err != nil { + logrus.Errorf("Error while deleting the configmap: %v", err) + } + // Deleting namespace testutil.DeleteNamespace(ersNamespace, clients.KubernetesClient) @@ -3548,3 +3599,94 @@ func TestFailedRollingUpgradeUsingErs(t *testing.T) { t.Errorf("Counter by namespace was not increased") } } + +func TestPausingDeploymentUsingErs(t *testing.T) { + options.ReloadStrategy = constants.EnvVarsReloadStrategy + testPausingDeployment(t, options.ReloadStrategy, ersConfigmapWithPausedDeployment, ersNamespace) +} + +func TestPausingDeploymentUsingArs(t *testing.T) { + options.ReloadStrategy = constants.AnnotationsReloadStrategy + testPausingDeployment(t, options.ReloadStrategy, arsConfigmapWithPausedDeployment, arsNamespace) +} + +func testPausingDeployment(t *testing.T, reloadStrategy string, testName string, namespace string) { + options.ReloadStrategy = reloadStrategy + envVarPostfix := constants.ConfigmapEnvVarPostfix + + shaData := testutil.ConvertResourceToSHA(testutil.ConfigmapResourceType, namespace, testName, "pause.stakater.com") + config := getConfigWithAnnotations(envVarPostfix, testName, shaData, options.ConfigmapUpdateOnChangeAnnotation, options.ConfigmapReloaderAutoAnnotation) + deploymentFuncs := GetDeploymentRollingUpgradeFuncs() + collectors := getCollectors() + + _ = PerformAction(clients, config, deploymentFuncs, collectors, nil, invokeReloadStrategy) + + if promtestutil.ToFloat64(collectors.Reloaded.With(labelSucceeded)) != 1 { + t.Errorf("Counter was not increased") + } + + if promtestutil.ToFloat64(collectors.ReloadedByNamespace.With(prometheus.Labels{"success": "true", "namespace": namespace})) != 1 { + t.Errorf("Counter by namespace was not increased") + } + + logrus.Infof("Verifying deployment has been paused") + items := deploymentFuncs.ItemsFunc(clients, config.Namespace) + deploymentPaused, err := isDeploymentPaused(items, testName) + if err != nil { + t.Errorf(err.Error()) + } + if !deploymentPaused { + t.Errorf("Deployment has not been paused") + } + + shaData = testutil.ConvertResourceToSHA(testutil.ConfigmapResourceType, namespace, testName, "pause-changed.stakater.com") + config = getConfigWithAnnotations(envVarPostfix, testName, shaData, options.ConfigmapUpdateOnChangeAnnotation, options.ConfigmapReloaderAutoAnnotation) + + _ = PerformAction(clients, config, deploymentFuncs, collectors, nil, invokeReloadStrategy) + + if promtestutil.ToFloat64(collectors.Reloaded.With(labelSucceeded)) != 2 { + t.Errorf("Counter was not increased") + } + + if promtestutil.ToFloat64(collectors.ReloadedByNamespace.With(prometheus.Labels{"success": "true", "namespace": namespace})) != 2 { + t.Errorf("Counter by namespace was not increased") + } + + logrus.Infof("Verifying deployment is still paused") + items = deploymentFuncs.ItemsFunc(clients, config.Namespace) + deploymentPaused, err = isDeploymentPaused(items, testName) + if err != nil { + t.Errorf("%s", err.Error()) + } + if !deploymentPaused { + t.Errorf("Deployment should still be paused") + } + + logrus.Infof("Verifying deployment has been resumed after pause interval") + time.Sleep(11 * time.Second) + items = deploymentFuncs.ItemsFunc(clients, config.Namespace) + deploymentPaused, err = isDeploymentPaused(items, testName) + if err != nil { + t.Errorf("%s", err.Error()) + } + if deploymentPaused { + t.Errorf("Deployment should have been resumed after pause interval") + } +} + +func isDeploymentPaused(deployments []runtime.Object, deploymentName string) (bool, error) { + for _, deployment := range deployments { + accessor, err := meta.Accessor(deployment) + if err != nil { + return false, fmt.Errorf("Error getting accessor for item: %v", err) + } + if accessor.GetName() == deploymentName { + deploymentObj, ok := deployment.(*appsv1.Deployment) + if !ok { + return false, fmt.Errorf("Failed to cast to Deployment") + } + return deploymentObj.Spec.Paused, nil + } + } + return false, nil +} diff --git a/internal/pkg/options/flags.go b/internal/pkg/options/flags.go index 081acc3e6..ed3ae84d0 100644 --- a/internal/pkg/options/flags.go +++ b/internal/pkg/options/flags.go @@ -38,6 +38,12 @@ var ( SearchMatchAnnotation = "reloader.stakater.com/match" // RolloutStrategyAnnotation is an annotation to define rollout update strategy RolloutStrategyAnnotation = "reloader.stakater.com/rollout-strategy" + // PauseDeploymentAnnotation is an annotation to define the time period to pause a deployment after + // a configmap/secret change has been detected. Valid values are described here: https://pkg.go.dev/time#ParseDuration + // only positive values are allowed + PauseDeploymentAnnotation = "deployment.reloader.stakater.com/pause-period" + // Annotation set by reloader to indicate that the deployment has been paused + PauseDeploymentTimeAnnotation = "deployment.reloader.stakater.com/paused-at" // LogFormat is the log format to use (json, or empty string for default) LogFormat = "" // LogLevel is the log level to use (trace, debug, info, warning, error, fatal and panic) diff --git a/internal/pkg/testutil/kube.go b/internal/pkg/testutil/kube.go index 1f779abce..79c226949 100644 --- a/internal/pkg/testutil/kube.go +++ b/internal/pkg/testutil/kube.go @@ -794,6 +794,26 @@ func CreateDeployment(client kubernetes.Interface, deploymentName string, namesp return deployment, err } +// CreateDeployment creates a deployment in given namespace and returns the Deployment +func CreateDeploymentWithAnnotations(client kubernetes.Interface, deploymentName string, namespace string, additionalAnnotations map[string]string, volumeMount bool) (*appsv1.Deployment, error) { + logrus.Infof("Creating Deployment") + deploymentClient := client.AppsV1().Deployments(namespace) + var deploymentObj *appsv1.Deployment + if volumeMount { + deploymentObj = GetDeployment(namespace, deploymentName) + } else { + deploymentObj = GetDeploymentWithEnvVars(namespace, deploymentName) + } + + for annotationKey, annotationValue := range additionalAnnotations { + deploymentObj.Annotations[annotationKey] = annotationValue + } + + deployment, err := deploymentClient.Create(context.TODO(), deploymentObj, metav1.CreateOptions{}) + time.Sleep(3 * time.Second) + return deployment, err +} + // CreateDeploymentConfig creates a deploymentConfig in given namespace and returns the DeploymentConfig func CreateDeploymentConfig(client appsclient.Interface, deploymentName string, namespace string, volumeMount bool) (*openshiftv1.DeploymentConfig, error) { logrus.Infof("Creating DeploymentConfig") From eba3ff34297e478a1a0ae01e350263818c118c52 Mon Sep 17 00:00:00 2001 From: tom1299 Date: Wed, 19 Feb 2025 06:38:52 +0100 Subject: [PATCH 2/8] Fix linting errors --- internal/pkg/handler/upgrade.go | 6 +++++- internal/pkg/handler/upgrade_test.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/pkg/handler/upgrade.go b/internal/pkg/handler/upgrade.go index 3025ff8cb..504587f03 100644 --- a/internal/pkg/handler/upgrade.go +++ b/internal/pkg/handler/upgrade.go @@ -291,7 +291,11 @@ func PerformAction(clients kube.Clients, config util.Config, upgradeFuncs callba if !ok { logrus.Warnf("Annotation '%s' only applicable for deployments", options.PauseDeploymentAnnotation) } else { - PauseDeployment(deployment, clients, itemName, itemNamespace, pauseInterval) + err = PauseDeployment(deployment, clients, itemName, itemNamespace, pauseInterval) + if err != nil { + logrus.Errorf("Failed to pause deployment '%s' in namespace '%s': %v", itemName, itemNamespace, err) + return err + } } } diff --git a/internal/pkg/handler/upgrade_test.go b/internal/pkg/handler/upgrade_test.go index d87dc0fe1..e8d87faa4 100644 --- a/internal/pkg/handler/upgrade_test.go +++ b/internal/pkg/handler/upgrade_test.go @@ -3633,7 +3633,7 @@ func testPausingDeployment(t *testing.T, reloadStrategy string, testName string, items := deploymentFuncs.ItemsFunc(clients, config.Namespace) deploymentPaused, err := isDeploymentPaused(items, testName) if err != nil { - t.Errorf(err.Error()) + t.Errorf("%s", err.Error()) } if !deploymentPaused { t.Errorf("Deployment has not been paused") From 785e7bb1a29b99047c25a2c4518c1d565202aac1 Mon Sep 17 00:00:00 2001 From: tom1299 Date: Sat, 26 Apr 2025 08:53:43 +0200 Subject: [PATCH 3/8] Align annotation names, refactoring, additional tests --- internal/pkg/handler/pause_deployment.go | 71 ++++- internal/pkg/handler/pause_deployment_test.go | 255 ++++++++++++++++++ internal/pkg/handler/upgrade_test.go | 18 +- 3 files changed, 320 insertions(+), 24 deletions(-) create mode 100644 internal/pkg/handler/pause_deployment_test.go diff --git a/internal/pkg/handler/pause_deployment.go b/internal/pkg/handler/pause_deployment.go index b6b06e6c4..e8250e4fe 100644 --- a/internal/pkg/handler/pause_deployment.go +++ b/internal/pkg/handler/pause_deployment.go @@ -2,24 +2,77 @@ package handler import ( "context" + "fmt" "time" "github.com/sirupsen/logrus" "github.com/stakater/Reloader/internal/pkg/options" "github.com/stakater/Reloader/pkg/kube" app "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ) +// IsPaused checks if a deployment is currently paused +func IsPaused(deployment *app.Deployment) bool { + return deployment.Spec.Paused +} + +// IsPausedByReloader checks if a deployment was paused by reloader +func IsPausedByReloader(deployment *app.Deployment) bool { + if !deployment.Spec.Paused { + return false + } + + pausedAtAnnotationValue := deployment.Annotations[options.PauseDeploymentTimeAnnotation] + return pausedAtAnnotationValue != "" +} + +// FindDeploymentByName locates a deployment by name from a list of api objects +func FindDeploymentByName(deployments []runtime.Object, deploymentName string) (*app.Deployment, error) { + for _, deployment := range deployments { + accessor, err := meta.Accessor(deployment) + if err != nil { + return nil, fmt.Errorf("error getting accessor for item: %v", err) + } + if accessor.GetName() == deploymentName { + deploymentObj, ok := deployment.(*app.Deployment) + if !ok { + return nil, fmt.Errorf("failed to cast to Deployment") + } + return deploymentObj, nil + } + } + return nil, fmt.Errorf("deployment '%s' not found", deploymentName) +} + +// GetPauseStartTime returns when the deployment was paused by reloader, nil otherwise +func GetPauseStartTime(deployment *app.Deployment) (*time.Time, error) { + if !IsPausedByReloader(deployment) { + return nil, nil + } + + pausedAtStr := deployment.Annotations[options.PauseDeploymentTimeAnnotation] + parsedTime, err := time.Parse(time.RFC3339, pausedAtStr) + if err != nil { + return nil, err + } + + return &parsedTime, nil +} + +// PauseDeployment pauses a deployment for a specified duration and creates a timer to resume it +// after the specified duration func PauseDeployment(deployment *app.Deployment, clients kube.Clients, deploymentName, namespace, pauseIntervalValue string) error { pauseDuration, err := ParsePauseDuration(pauseIntervalValue) if err != nil { return err } - if !deployment.Spec.Paused { + if !IsPaused(deployment) { deployment.Spec.Paused = true - logrus.Infof("Pausing Deployment '%s' in namespace '%s' for %s seconds", deploymentName, namespace, pauseDuration) + logrus.Infof("Pausing Deployment '%s' in namespace '%s' for %s", deploymentName, namespace, pauseDuration) if deployment.Annotations == nil { deployment.Annotations = make(map[string]string) @@ -33,12 +86,14 @@ func PauseDeployment(deployment *app.Deployment, clients kube.Clients, deploymen return nil } +// CreateResumeTimer creates a timer to resume the deployment after the specified duration func CreateResumeTimer(deployment *app.Deployment, clients kube.Clients, deploymentName, namespace string, pauseDuration time.Duration) { time.AfterFunc(pauseDuration, func() { ResumeDeployment(deploymentName, namespace, clients) }) } +// ResumeDeployment resumes a deployment that has been paused by reloader func ResumeDeployment(deploymentName, namespace string, clients kube.Clients) { deployment, err := clients.KubernetesClient.AppsV1().Deployments(namespace).Get(context.TODO(), deploymentName, metav1.GetOptions{}) if err != nil { @@ -46,14 +101,8 @@ func ResumeDeployment(deploymentName, namespace string, clients kube.Clients) { return } - if !deployment.Spec.Paused { - logrus.Infof("Deployment '%s' in namespace '%s' not paused. Skipping resume", deploymentName, namespace) - return - } - - pausedAtAnnotationValue := deployment.Annotations[options.PauseDeploymentTimeAnnotation] - if pausedAtAnnotationValue == "" { - logrus.Infof("Deployment '%s' in namespace '%s' was not paused by Reloader. Skipping resume", deploymentName, namespace) + if !IsPausedByReloader(deployment) { + logrus.Infof("Deployment '%s' in namespace '%s' not paused by Reloader. Skipping resume", deploymentName, namespace) return } @@ -63,11 +112,13 @@ func ResumeDeployment(deploymentName, namespace string, clients kube.Clients) { _, err = clients.KubernetesClient.AppsV1().Deployments(namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{}) if err != nil { logrus.Errorf("Failed to resume deployment '%s' in namespace '%s': %v", deploymentName, namespace, err) + return } logrus.Infof("Successfully resumed deployment '%s' in namespace '%s'", deploymentName, namespace) } +// ParsePauseDuration parses the pause interval value and returns a time.Duration func ParsePauseDuration(pauseIntervalValue string) (time.Duration, error) { pauseDuration, err := time.ParseDuration(pauseIntervalValue) if err != nil { diff --git a/internal/pkg/handler/pause_deployment_test.go b/internal/pkg/handler/pause_deployment_test.go new file mode 100644 index 000000000..ad2857737 --- /dev/null +++ b/internal/pkg/handler/pause_deployment_test.go @@ -0,0 +1,255 @@ +package handler + +import ( + "testing" + "time" + + "github.com/stakater/Reloader/internal/pkg/options" + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func TestIsPaused(t *testing.T) { + tests := []struct { + name string + deployment *appsv1.Deployment + paused bool + }{ + { + name: "paused deployment", + deployment: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Paused: true, + }, + }, + paused: true, + }, + { + name: "unpaused deployment", + deployment: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + paused: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := IsPaused(test.deployment) + assert.Equal(t, test.paused, result) + }) + } +} + +func TestIsPausedByReloader(t *testing.T) { + tests := []struct { + name string + deployment *appsv1.Deployment + pausedByReloader bool + }{ + { + name: "paused by reloader", + deployment: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Paused: true, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + options.PauseDeploymentTimeAnnotation: time.Now().Format(time.RFC3339), + }, + }, + }, + pausedByReloader: true, + }, + { + name: "not paused by reloader", + deployment: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Paused: true, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + }, + pausedByReloader: false, + }, + { + name: "not paused", + deployment: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + pausedByReloader: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + pausedByReloader := IsPausedByReloader(test.deployment) + assert.Equal(t, test.pausedByReloader, pausedByReloader) + }) + } +} + +func TestFindDeploymentByName(t *testing.T) { + testDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + }, + } + + additionalDeployment1 := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "non-matching-deployment-1", + }, + } + + additionalDeployment2 := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "non-matching-deployment-2", + }, + } + + tests := []struct { + name string + deployments []runtime.Object + deploymentName string + expectedDeployment *appsv1.Deployment + deploymentFound bool + }{ + { + name: "deployment found", + deployments: []runtime.Object{ + additionalDeployment1, + testDeployment, + additionalDeployment2, + }, + deploymentName: "test-deployment", + expectedDeployment: testDeployment, + deploymentFound: true, + }, + { + name: "deployment not found", + deployments: []runtime.Object{ + additionalDeployment1, + testDeployment, + additionalDeployment2, + }, + deploymentName: "non-existent-deployment", + expectedDeployment: nil, + deploymentFound: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + foundDeployment, err := FindDeploymentByName(test.deployments, test.deploymentName) + + if test.deploymentFound { + assert.NoError(t, err) + assert.Equal(t, test.expectedDeployment, foundDeployment) + } else { + assert.Error(t, err) + assert.Nil(t, foundDeployment) + } + }) + } +} + +func TestGetPauseStartTime(t *testing.T) { + now := time.Now() + nowStr := now.Format(time.RFC3339) + + tests := []struct { + name string + deployment *appsv1.Deployment + pausedByReloader bool + expectedStartTime time.Time + }{ + { + name: "valid pause time", + deployment: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Paused: true, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + options.PauseDeploymentTimeAnnotation: nowStr, + }, + }, + }, + pausedByReloader: true, + expectedStartTime: now, + }, + { + name: "not paused by reloader", + deployment: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + pausedByReloader: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actualStartTime, err := GetPauseStartTime(test.deployment) + + assert.NoError(t, err) + + if !test.pausedByReloader { + assert.Nil(t, actualStartTime) + } else { + assert.NotNil(t, actualStartTime) + assert.WithinDuration(t, test.expectedStartTime, *actualStartTime, time.Second) + } + }) + } +} + +func TestParsePauseDuration(t *testing.T) { + tests := []struct { + name string + pauseIntervalValue string + expectedDuration time.Duration + invalidDuration bool + }{ + { + name: "valid duration", + pauseIntervalValue: "10s", + expectedDuration: 10 * time.Second, + invalidDuration: false, + }, + { + name: "valid minute duration", + pauseIntervalValue: "2m", + expectedDuration: 2 * time.Minute, + invalidDuration: false, + }, + { + name: "invalid duration", + pauseIntervalValue: "invalid", + expectedDuration: 0, + invalidDuration: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actualDuration, err := ParsePauseDuration(test.pauseIntervalValue) + + if test.invalidDuration { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, test.expectedDuration, actualDuration) + } + }) + } +} diff --git a/internal/pkg/handler/upgrade_test.go b/internal/pkg/handler/upgrade_test.go index e8d87faa4..610570f9f 100644 --- a/internal/pkg/handler/upgrade_test.go +++ b/internal/pkg/handler/upgrade_test.go @@ -17,7 +17,6 @@ import ( "github.com/stakater/Reloader/internal/pkg/testutil" "github.com/stakater/Reloader/internal/pkg/util" "github.com/stakater/Reloader/pkg/kube" - appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/meta" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -3675,18 +3674,9 @@ func testPausingDeployment(t *testing.T, reloadStrategy string, testName string, } func isDeploymentPaused(deployments []runtime.Object, deploymentName string) (bool, error) { - for _, deployment := range deployments { - accessor, err := meta.Accessor(deployment) - if err != nil { - return false, fmt.Errorf("Error getting accessor for item: %v", err) - } - if accessor.GetName() == deploymentName { - deploymentObj, ok := deployment.(*appsv1.Deployment) - if !ok { - return false, fmt.Errorf("Failed to cast to Deployment") - } - return deploymentObj.Spec.Paused, nil - } + deployment, err := FindDeploymentByName(deployments, deploymentName) + if err != nil { + return false, err } - return false, nil + return IsPaused(deployment), nil } From 6f6886d265d26fe36302cc358fa0e124ac6edcd5 Mon Sep 17 00:00:00 2001 From: tom1299 Date: Mon, 28 Apr 2025 10:57:43 +0200 Subject: [PATCH 4/8] Add missing timer handling, refactoring, tests --- internal/pkg/handler/pause_deployment.go | 148 +++++++--- internal/pkg/handler/pause_deployment_test.go | 261 +++++++++++++----- internal/pkg/handler/upgrade.go | 2 +- 3 files changed, 300 insertions(+), 111 deletions(-) diff --git a/internal/pkg/handler/pause_deployment.go b/internal/pkg/handler/pause_deployment.go index e8250e4fe..324d5ac39 100644 --- a/internal/pkg/handler/pause_deployment.go +++ b/internal/pkg/handler/pause_deployment.go @@ -9,45 +9,32 @@ import ( "github.com/stakater/Reloader/internal/pkg/options" "github.com/stakater/Reloader/pkg/kube" app "k8s.io/api/apps/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" ) -// IsPaused checks if a deployment is currently paused +// 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 { + return fmt.Sprintf("%s/%s", namespace, deploymentName) +} + +// Checks if a deployment is currently paused func IsPaused(deployment *app.Deployment) bool { return deployment.Spec.Paused } -// IsPausedByReloader checks if a deployment was paused by reloader +// Deployment paused by reloader ? func IsPausedByReloader(deployment *app.Deployment) bool { - if !deployment.Spec.Paused { - return false + if IsPaused(deployment) { + pausedAtAnnotationValue := deployment.Annotations[options.PauseDeploymentTimeAnnotation] + return pausedAtAnnotationValue != "" } - - pausedAtAnnotationValue := deployment.Annotations[options.PauseDeploymentTimeAnnotation] - return pausedAtAnnotationValue != "" + return false } -// FindDeploymentByName locates a deployment by name from a list of api objects -func FindDeploymentByName(deployments []runtime.Object, deploymentName string) (*app.Deployment, error) { - for _, deployment := range deployments { - accessor, err := meta.Accessor(deployment) - if err != nil { - return nil, fmt.Errorf("error getting accessor for item: %v", err) - } - if accessor.GetName() == deploymentName { - deploymentObj, ok := deployment.(*app.Deployment) - if !ok { - return nil, fmt.Errorf("failed to cast to Deployment") - } - return deploymentObj, nil - } - } - return nil, fmt.Errorf("deployment '%s' not found", deploymentName) -} - -// GetPauseStartTime returns when the deployment was paused by reloader, nil otherwise +// Returns the time, the deployment was paused by reloader, nil otherwise func GetPauseStartTime(deployment *app.Deployment) (*time.Time, error) { if !IsPausedByReloader(deployment) { return nil, nil @@ -62,12 +49,22 @@ func GetPauseStartTime(deployment *app.Deployment) (*time.Time, error) { return &parsedTime, nil } -// PauseDeployment pauses a deployment for a specified duration and creates a timer to resume it +// ParsePauseDuration parses the pause interval value and returns a time.Duration +func ParsePauseDuration(pauseIntervalValue string) (time.Duration, error) { + pauseDuration, err := time.ParseDuration(pauseIntervalValue) + if err != nil { + logrus.Warnf("Failed to parse pause interval value '%s': %v", pauseIntervalValue, err) + return 0, err + } + return pauseDuration, nil +} + +// Pauses a deployment for a specified duration and creates a timer to resume it // after the specified duration -func PauseDeployment(deployment *app.Deployment, clients kube.Clients, deploymentName, namespace, pauseIntervalValue string) error { +func PauseDeployment(deployment *app.Deployment, clients kube.Clients, deploymentName, namespace, pauseIntervalValue string) (*app.Deployment, error) { pauseDuration, err := ParsePauseDuration(pauseIntervalValue) if err != nil { - return err + return nil, err } if !IsPaused(deployment) { @@ -78,19 +75,82 @@ func PauseDeployment(deployment *app.Deployment, clients kube.Clients, deploymen deployment.Annotations = make(map[string]string) } deployment.Annotations[options.PauseDeploymentTimeAnnotation] = time.Now().Format(time.RFC3339) + deployment.Annotations[options.PauseDeploymentAnnotation] = pauseIntervalValue CreateResumeTimer(deployment, clients, deploymentName, namespace, pauseDuration) - } else { - logrus.Infof("Deployment '%s' in namespace '%s' is already paused", deploymentName, namespace) + return deployment, nil + } + + if !IsPausedByReloader(deployment) { + logrus.Infof("Deployment '%s' in namespace '%s' already paused", deploymentName, namespace) + return deployment, nil + } + + // Deployment has already been paused by reloader, check for timer + logrus.Debugf("Deployment '%s' in namespace '%s' is already paused by reloader", deploymentName, namespace) + + timerKey := getTimerKey(namespace, deploymentName) + _, timerExists := activeTimers[timerKey] + + if !timerExists { + logrus.Warnf("Timer does not exist for already paused deployment '%s' in namespace '%s', creating new one", + deploymentName, namespace) + HandleMissingTimer(deployment, pauseDuration, clients, deploymentName, namespace) + } + return deployment, nil +} + +// Handles the case where missing timers for deployments that have been paused by reloader. +// Could occur after new leader election or reloader restart +func HandleMissingTimer(deployment *app.Deployment, pauseDuration time.Duration, clients kube.Clients, deploymentName, namespace string) { + pauseStartTime, err := GetPauseStartTime(deployment) + if err != nil { + logrus.Errorf("Error parsing pause start time for deployment '%s' in namespace '%s': %v. Resuming deployment immediately", + deploymentName, namespace, err) + ResumeDeployment(deploymentName, namespace, clients) + return + } + + if pauseStartTime == nil { + return } - return nil + + elapsedPauseTime := time.Since(*pauseStartTime) + remainingPauseTime := pauseDuration - elapsedPauseTime + + if remainingPauseTime <= 0 { + logrus.Infof("Pause period for deployment '%s' in namespace '%s' has expired. Resuming immediately", + deploymentName, namespace) + ResumeDeployment(deploymentName, namespace, clients) + return + } + + logrus.Infof("Creating missing timer for already paused deployment '%s' in namespace '%s' with remaining time %s", + deploymentName, namespace, remainingPauseTime) + CreateResumeTimer(deployment, clients, deploymentName, namespace, remainingPauseTime) } // CreateResumeTimer creates a timer to resume the deployment after the specified duration func CreateResumeTimer(deployment *app.Deployment, clients kube.Clients, deploymentName, namespace string, pauseDuration time.Duration) { - time.AfterFunc(pauseDuration, func() { + timerKey := getTimerKey(namespace, deploymentName) + + // Check if there's an existing timer for this deployment + if _, exists := activeTimers[timerKey]; exists { + logrus.Debugf("Timer already exists for deployment '%s' in namespace '%s', Skipping creation", + deploymentName, namespace) + return + } + + // Create and store the new timer + timer := time.AfterFunc(pauseDuration, func() { ResumeDeployment(deploymentName, namespace, clients) }) + + // Add the new timer to the map + activeTimers[timerKey] = timer + + logrus.Debugf("Created pause timer for deployment '%s' in namespace '%s' with duration %s", + deploymentName, namespace, pauseDuration) } // ResumeDeployment resumes a deployment that has been paused by reloader @@ -106,6 +166,14 @@ func ResumeDeployment(deploymentName, namespace string, clients kube.Clients) { return } + // Remove the timer + timerKey := getTimerKey(namespace, deploymentName) + if timer, exists := activeTimers[timerKey]; exists { + timer.Stop() + delete(activeTimers, timerKey) + logrus.Debugf("Removed pause timer for deployment '%s' in namespace '%s'", deploymentName, namespace) + } + deployment.Spec.Paused = false delete(deployment.Annotations, options.PauseDeploymentTimeAnnotation) @@ -117,13 +185,3 @@ func ResumeDeployment(deploymentName, namespace string, clients kube.Clients) { logrus.Infof("Successfully resumed deployment '%s' in namespace '%s'", deploymentName, namespace) } - -// ParsePauseDuration parses the pause interval value and returns a time.Duration -func ParsePauseDuration(pauseIntervalValue string) (time.Duration, error) { - pauseDuration, err := time.ParseDuration(pauseIntervalValue) - if err != nil { - logrus.Warnf("Failed to parse pause interval value '%s': %v", pauseIntervalValue, err) - return 0, err - } - return pauseDuration, nil -} diff --git a/internal/pkg/handler/pause_deployment_test.go b/internal/pkg/handler/pause_deployment_test.go index ad2857737..3def125f0 100644 --- a/internal/pkg/handler/pause_deployment_test.go +++ b/internal/pkg/handler/pause_deployment_test.go @@ -1,14 +1,20 @@ package handler import ( + "context" + "fmt" "testing" "time" "github.com/stakater/Reloader/internal/pkg/options" + "github.com/stakater/Reloader/pkg/kube" "github.com/stretchr/testify/assert" + app "k8s.io/api/apps/v1" appsv1 "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/api/meta" 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) { @@ -96,71 +102,6 @@ func TestIsPausedByReloader(t *testing.T) { } } -func TestFindDeploymentByName(t *testing.T) { - testDeployment := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-deployment", - }, - } - - additionalDeployment1 := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "non-matching-deployment-1", - }, - } - - additionalDeployment2 := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "non-matching-deployment-2", - }, - } - - tests := []struct { - name string - deployments []runtime.Object - deploymentName string - expectedDeployment *appsv1.Deployment - deploymentFound bool - }{ - { - name: "deployment found", - deployments: []runtime.Object{ - additionalDeployment1, - testDeployment, - additionalDeployment2, - }, - deploymentName: "test-deployment", - expectedDeployment: testDeployment, - deploymentFound: true, - }, - { - name: "deployment not found", - deployments: []runtime.Object{ - additionalDeployment1, - testDeployment, - additionalDeployment2, - }, - deploymentName: "non-existent-deployment", - expectedDeployment: nil, - deploymentFound: false, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - foundDeployment, err := FindDeploymentByName(test.deployments, test.deploymentName) - - if test.deploymentFound { - assert.NoError(t, err) - assert.Equal(t, test.expectedDeployment, foundDeployment) - } else { - assert.Error(t, err) - assert.Nil(t, foundDeployment) - } - }) - } -} - func TestGetPauseStartTime(t *testing.T) { now := time.Now() nowStr := now.Format(time.RFC3339) @@ -253,3 +194,193 @@ 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-1", + 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-2", + 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 { + // Clean up any timers at the end of the test + defer func() { + for key, timer := range activeTimers { + timer.Stop() + delete(activeTimers, key) + } + }() + + t.Run(test.name, func(t *testing.T) { + fakeClient := testclient.NewSimpleClientset() + clients := kube.Clients{ + KubernetesClient: fakeClient, + } + + _, err := fakeClient.AppsV1().Deployments("default").Create( + context.TODO(), + test.deployment, + metav1.CreateOptions{}) + assert.NoError(t, err, "Expected no error when creating deployment") + + 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") + } + }) + } +} + +func TestPauseDeployment(t *testing.T) { + tests := []struct { + name string + deployment *appsv1.Deployment + expectedError bool + expectedPaused bool + expectedAnnotation bool // Should have pause time annotation + pauseInterval string + }{ + { + name: "deployment without pause annotation", + deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + Annotations: map[string]string{}, + }, + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + expectedError: true, + expectedPaused: false, + expectedAnnotation: false, + pauseInterval: "", + }, + { + name: "deployment already paused but not by reloader", + deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + Annotations: map[string]string{ + options.PauseDeploymentAnnotation: "5m", + }, + }, + Spec: appsv1.DeploymentSpec{ + Paused: true, + }, + }, + expectedError: false, + expectedPaused: true, + expectedAnnotation: false, + pauseInterval: "5m", + }, + { + name: "deployment unpaused that needs to be paused by reloader", + deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment-3", + Annotations: map[string]string{ + options.PauseDeploymentAnnotation: "5m", + }, + }, + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + expectedError: false, + expectedPaused: true, + expectedAnnotation: true, + pauseInterval: "5m", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeClient := testclient.NewSimpleClientset() + clients := kube.Clients{ + KubernetesClient: fakeClient, + } + + updatedDeployment, err := PauseDeployment(test.deployment, clients, test.deployment.Name, "default", test.pauseInterval) + if test.expectedError { + assert.Error(t, err, "Expected an error pausing the deployment") + return + } else { + assert.NoError(t, err, "Expected no error pausing the deployment") + } + + assert.Equal(t, test.expectedPaused, updatedDeployment.Spec.Paused, + "Deployment should have correct paused state after pause") + + if test.expectedAnnotation { + pausedAtAnnotationValue := updatedDeployment.Annotations[options.PauseDeploymentTimeAnnotation] + assert.NotEmpty(t, pausedAtAnnotationValue, + "Pause annotation should be present and contain a value when deployment is paused") + } else { + pausedAtAnnotationValue := updatedDeployment.Annotations[options.PauseDeploymentTimeAnnotation] + assert.Empty(t, pausedAtAnnotationValue, + "Pause annotation should not be present when deployment has not been paused by reloader") + } + }) + } +} + +// Simple helper function for test cases +func FindDeploymentByName(deployments []runtime.Object, deploymentName string) (*app.Deployment, error) { + for _, deployment := range deployments { + accessor, err := meta.Accessor(deployment) + if err != nil { + return nil, fmt.Errorf("error getting accessor for item: %v", err) + } + if accessor.GetName() == deploymentName { + deploymentObj, ok := deployment.(*app.Deployment) + if !ok { + return nil, fmt.Errorf("failed to cast to Deployment") + } + return deploymentObj, nil + } + } + return nil, fmt.Errorf("deployment '%s' not found", deploymentName) +} diff --git a/internal/pkg/handler/upgrade.go b/internal/pkg/handler/upgrade.go index 504587f03..79ea49c45 100644 --- a/internal/pkg/handler/upgrade.go +++ b/internal/pkg/handler/upgrade.go @@ -291,7 +291,7 @@ func PerformAction(clients kube.Clients, config util.Config, upgradeFuncs callba if !ok { logrus.Warnf("Annotation '%s' only applicable for deployments", options.PauseDeploymentAnnotation) } else { - err = PauseDeployment(deployment, clients, itemName, itemNamespace, pauseInterval) + _, err = PauseDeployment(deployment, clients, itemName, itemNamespace, pauseInterval) if err != nil { logrus.Errorf("Failed to pause deployment '%s' in namespace '%s': %v", itemName, itemNamespace, err) return err From ff1a80eafe4981f9834c99b71c1b9ef8582d701b Mon Sep 17 00:00:00 2001 From: tom1299 Date: Fri, 4 Jul 2025 10:28:15 +0200 Subject: [PATCH 5/8] More tests use original upgrade --- internal/pkg/callbacks/pause_deployment.go | 40 +-- .../pkg/callbacks/pause_deployment_test.go | 272 ++++++++++++++++++ internal/pkg/handler/upgrade.go | 1 - 3 files changed, 292 insertions(+), 21 deletions(-) diff --git a/internal/pkg/callbacks/pause_deployment.go b/internal/pkg/callbacks/pause_deployment.go index c32b581d1..a1a80f983 100644 --- a/internal/pkg/callbacks/pause_deployment.go +++ b/internal/pkg/callbacks/pause_deployment.go @@ -51,7 +51,7 @@ func GetPauseStartTime(deployment *app.Deployment) (*time.Time, error) { return &parsedTime, nil } -// ParsePauseDuration parses the pause interval value and returns a time.Duration +// Parses the pause interval value and returns a time.Duration func ParsePauseDuration(pauseIntervalValue string) (time.Duration, error) { pauseDuration, err := time.ParseDuration(pauseIntervalValue) if err != nil { @@ -61,7 +61,7 @@ func ParsePauseDuration(pauseIntervalValue string) (time.Duration, error) { return pauseDuration, nil } -// GetPauseDuration returns the duration for which the deployment is paused +// Returns the duration for which the deployment is paused func GetPauseDuration(deployment app.Deployment) (string, time.Duration) { pauseDurationStr := deployment.Annotations[options.PauseDeploymentAnnotation] if pauseDurationStr == "" { @@ -105,30 +105,22 @@ func HandleMissingTimer(deployment *app.Deployment, pauseDuration time.Duration, CreateResumeTimer(deployment, clients, remainingPauseTime) } -// CreateResumeTimer creates a timer to resume the deployment after the specified duration +// Creates a timer to resume the deployment after the specified duration func CreateResumeTimer(deployment *app.Deployment, clients kube.Clients, pauseDuration time.Duration) { timerKey := getTimerKey(deployment.Namespace, deployment.Name) - // Check if there's an existing timer for this deployment if _, exists := activeTimers[timerKey]; exists { - logrus.Debugf("Timer already exists for deployment '%s' in namespace '%s', Skipping creation", - deployment.Name, deployment.Namespace) return } - // Create and store the new timer timer := time.AfterFunc(pauseDuration, func() { ResumeDeployment(deployment.Name, deployment.Namespace, clients) }) - // Add the new timer to the map activeTimers[timerKey] = timer - - logrus.Debugf("Created pause timer for deployment '%s' in namespace '%s' with duration %s", - deployment.Name, deployment.Namespace, pauseDuration) } -// ResumeDeployment resumes a deployment that has been paused by reloader +// Resumes a deployment that has been paused by reloader func ResumeDeployment(deploymentName, namespace string, clients kube.Clients) { deployment, err := clients.KubernetesClient.AppsV1().Deployments(namespace).Get(context.TODO(), deploymentName, metav1.GetOptions{}) if err != nil { @@ -161,6 +153,7 @@ func ResumeDeployment(deploymentName, namespace string, clients kube.Clients) { logrus.Infof("Successfully resumed deployment '%s' in namespace '%s'", deploymentName, namespace) } +// Creates a patch to pause the deployment func CreatePausePatch(pauseIntervalValue string) string { timeAnnotation := time.Now().Format(time.RFC3339) return fmt.Sprintf(`{ @@ -170,6 +163,7 @@ func CreatePausePatch(pauseIntervalValue string) string { options.PauseDeploymentAnnotation, pauseIntervalValue) } +// Applies the pause annotation to the deployment directly func ApplyPauseAnnotation(deployment *app.Deployment, pauseIntervalValue string) { if deployment.Annotations == nil { deployment.Annotations = make(map[string]string) @@ -178,6 +172,7 @@ func ApplyPauseAnnotation(deployment *app.Deployment, pauseIntervalValue string) deployment.Annotations[options.PauseDeploymentAnnotation] = pauseIntervalValue } +// Checks if a resume timer exists for the deployment func ResumeTimerExists(deployment *app.Deployment) bool { timerKey := getTimerKey(deployment.Namespace, deployment.Name) _, exists := activeTimers[timerKey] @@ -187,6 +182,7 @@ func ResumeTimerExists(deployment *app.Deployment) bool { return exists } +// Main function for pausing a deployment func PauseDeployment(deployment *app.Deployment, clients kube.Clients, doPatch bool) bool { pauseDurationString, pauseDuration := GetPauseDuration(*deployment) @@ -197,31 +193,34 @@ func PauseDeployment(deployment *app.Deployment, clients kube.Clients, doPatch b pausedByReloader := IsPausedByReloader(deployment) if pausedByReloader { // In case of leader election or reloader restart, check for missing timers - return checkForMissingTimer(deployment, pauseDuration, clients) + return CheckForMissingTimer(deployment, pauseDuration, clients) } - return updateDeployment(deployment, clients, pauseDurationString, pauseDuration, doPatch) + return PerformUpdate(deployment, clients, pauseDurationString, pauseDuration, doPatch) } -func checkForMissingTimer(deployment *app.Deployment, pauseDuration time.Duration, clients kube.Clients) bool { +// Checks if a resume timer exists +func CheckForMissingTimer(deployment *app.Deployment, pauseDuration time.Duration, clients kube.Clients) bool { if !ResumeTimerExists(deployment) { HandleMissingTimer(deployment, pauseDuration, clients) } return true } -func updateDeployment(deployment *app.Deployment, clients kube.Clients, pauseDurationString string, pauseDuration time.Duration, doPatch bool) bool { +// Does the actual update of the deployment (patching or directly) +func PerformUpdate(deployment *app.Deployment, clients kube.Clients, pauseDurationString string, pauseDuration time.Duration, doPatch bool) bool { logrus.Infof("Pausing Deployment '%s' in namespace '%s' for %s", deployment.Name, deployment.Namespace, pauseDuration) if doPatch { - return pauseWithPatch(deployment, clients, pauseDurationString, pauseDuration) + return PauseWithPatch(deployment, clients, pauseDurationString, pauseDuration) } - updateDeploymentObject(deployment, pauseDurationString, pauseDuration) + ApplyPauseToDeployment(deployment, pauseDurationString, pauseDuration) return true } -func pauseWithPatch(deployment *app.Deployment, clients kube.Clients, pauseDurationString string, pauseDuration time.Duration) bool { +// Pauses the deployment using patch +func PauseWithPatch(deployment *app.Deployment, clients kube.Clients, pauseDurationString string, pauseDuration time.Duration) bool { pausePatch := CreatePausePatch(pauseDurationString) _, err := clients.KubernetesClient.AppsV1().Deployments(deployment.Namespace).Patch( @@ -241,7 +240,8 @@ func pauseWithPatch(deployment *app.Deployment, clients kube.Clients, pauseDurat return true } -func updateDeploymentObject(deployment *app.Deployment, pauseDurationString string, pauseDuration time.Duration) { +// Applies the pause annotation directly +func ApplyPauseToDeployment(deployment *app.Deployment, pauseDurationString string, pauseDuration time.Duration) { ApplyPauseAnnotation(deployment, pauseDurationString) deployment.Spec.Paused = true } diff --git a/internal/pkg/callbacks/pause_deployment_test.go b/internal/pkg/callbacks/pause_deployment_test.go index a92c8129e..d43f5242a 100644 --- a/internal/pkg/callbacks/pause_deployment_test.go +++ b/internal/pkg/callbacks/pause_deployment_test.go @@ -371,6 +371,278 @@ func TestPauseDeployment(t *testing.T) { } } +func TestPauseWithPatch(t *testing.T) { + tests := []struct { + name string + deployment *appsv1.Deployment + pauseDurationString string + pauseDuration time.Duration + expectedResult bool + expectTimer bool + shouldPatchSucceed bool + }{ + { + name: "successful pause deployment with patch", + deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment-1", + Namespace: "default", + Annotations: map[string]string{ + options.PauseDeploymentAnnotation: "5m", + }, + }, + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + pauseDurationString: "5m", + pauseDuration: 5 * time.Minute, + expectedResult: true, + expectTimer: true, + shouldPatchSucceed: true, + }, + { + name: "deployment without annotations", + deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment-2", + Namespace: "default", + Annotations: nil, + }, + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + pauseDurationString: "30s", + pauseDuration: 30 * time.Second, + expectedResult: true, + expectTimer: true, + shouldPatchSucceed: true, + }, + } + + for _, test := range tests { + // Clean up any timers at the end of each test + defer func() { + for key, timer := range activeTimers { + timer.Stop() + delete(activeTimers, key) + } + }() + + t.Run(test.name, func(t *testing.T) { + fakeClient := testclient.NewSimpleClientset() + clients := kube.Clients{ + KubernetesClient: fakeClient, + } + + _, err := fakeClient.AppsV1().Deployments(test.deployment.Namespace).Create( + context.TODO(), + test.deployment, + metav1.CreateOptions{}) + assert.NoError(t, err, "Expected no error when creating deployment") + + result := PauseWithPatch(test.deployment, clients, test.pauseDurationString, test.pauseDuration) + + assert.Equal(t, test.expectedResult, result, + "PauseWithPatch should return correct success/failure state") + + // Check if timer was created + timerKey := getTimerKey(test.deployment.Namespace, test.deployment.Name) + _, timerExists := activeTimers[timerKey] + assert.Equal(t, test.expectTimer, timerExists, + "Timer should exist if pause was successful") + + if test.shouldPatchSucceed { + updatedDeployment, err := fakeClient.AppsV1().Deployments(test.deployment.Namespace).Get( + context.TODO(), + test.deployment.Name, + metav1.GetOptions{}) + + assert.NoError(t, err, "Should be able to get updated deployment") + assert.True(t, updatedDeployment.Spec.Paused, "Deployment should be paused after patch") + + pauseTimeAnnotation := updatedDeployment.Annotations[options.PauseDeploymentTimeAnnotation] + assert.NotEmpty(t, pauseTimeAnnotation, "Pause time annotation should be set") + + pauseIntervalAnnotation := updatedDeployment.Annotations[options.PauseDeploymentAnnotation] + assert.Equal(t, test.pauseDurationString, pauseIntervalAnnotation, "Pause interval annotation should match") + } + }) + } +} + +func TestApplyPauseToDeployment(t *testing.T) { + tests := []struct { + name string + deployment *appsv1.Deployment + pauseDurationString string + pauseDuration time.Duration + expectPaused bool + expectAnnotations bool + }{ + { + name: "apply pause to unpaused deployment", + deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment-1", + Namespace: "default", + }, + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + pauseDurationString: "5m", + pauseDuration: 5 * time.Minute, + expectPaused: true, + expectAnnotations: true, + }, + { + name: "apply pause to deployment with no annotations map", + deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment-3", + Namespace: "default", + Annotations: nil, + }, + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + pauseDurationString: "30s", + pauseDuration: 30 * time.Second, + expectPaused: true, + expectAnnotations: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ApplyPauseToDeployment(test.deployment, test.pauseDurationString, test.pauseDuration) + + assert.Equal(t, test.expectPaused, test.deployment.Spec.Paused, + "Deployment should be paused after applying pause") + + if test.expectAnnotations { + assert.NotNil(t, test.deployment.Annotations, + "Annotations map should be created if it didn't exist") + + pauseTimeAnnotation := test.deployment.Annotations[options.PauseDeploymentTimeAnnotation] + assert.NotEmpty(t, pauseTimeAnnotation, + "Pause time annotation should be set") + + pauseIntervalAnnotation := test.deployment.Annotations[options.PauseDeploymentAnnotation] + assert.Equal(t, test.pauseDurationString, pauseIntervalAnnotation, + "Pause interval annotation should match the input") + + // If there were existing annotations, make sure they are preserved + if test.deployment.Annotations != nil { + for key, value := range test.deployment.Annotations { + if key != options.PauseDeploymentTimeAnnotation && key != options.PauseDeploymentAnnotation { + assert.Equal(t, value, test.deployment.Annotations[key], + "Existing annotations should be preserved") + } + } + } + } + }) + } +} + +func TestUpdateDeployment(t *testing.T) { + tests := []struct { + name string + deployment *appsv1.Deployment + pauseDurationString string + pauseDuration time.Duration + doPatch bool + expectedResult bool + }{ + { + name: "update deployment with patch", + deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment-patch", + Namespace: "default", + }, + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + pauseDurationString: "5m", + pauseDuration: 5 * time.Minute, + doPatch: true, + expectedResult: true, + }, + { + name: "update deployment without patch", + deployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment-no-patch", + Namespace: "default", + }, + Spec: appsv1.DeploymentSpec{ + Paused: false, + }, + }, + pauseDurationString: "10m", + pauseDuration: 10 * time.Minute, + doPatch: false, + expectedResult: true, + }, + } + + for _, test := range tests { + defer func() { + for key, timer := range activeTimers { + timer.Stop() + delete(activeTimers, key) + } + }() + + t.Run(test.name, func(t *testing.T) { + fakeClient := testclient.NewSimpleClientset() + clients := kube.Clients{ + KubernetesClient: fakeClient, + } + + _, err := fakeClient.AppsV1().Deployments(test.deployment.Namespace).Create( + context.TODO(), + test.deployment, + metav1.CreateOptions{}) + assert.NoError(t, err, "Expected no error when creating deployment") + + result := PerformUpdate(test.deployment, clients, test.pauseDurationString, test.pauseDuration, test.doPatch) + + assert.Equal(t, test.expectedResult, result, + "updateDeployment should return correct result") + + if test.doPatch { + // For patch, get deployment again + updatedDeployment, err := fakeClient.AppsV1().Deployments(test.deployment.Namespace).Get( + context.TODO(), + test.deployment.Name, + metav1.GetOptions{}) + + assert.NoError(t, err, "Should be able to get updated deployment") + assert.True(t, updatedDeployment.Spec.Paused, "Deployment should be paused") + + assert.NotEmpty(t, updatedDeployment.Annotations[options.PauseDeploymentTimeAnnotation], + "Pause time annotation should be present") + assert.Equal(t, test.pauseDurationString, updatedDeployment.Annotations[options.PauseDeploymentAnnotation], + "Pause interval annotation should match") + } else { + assert.True(t, test.deployment.Spec.Paused, "Deployment should be paused") + + assert.NotEmpty(t, test.deployment.Annotations[options.PauseDeploymentTimeAnnotation], + "Pause time annotation should be present") + assert.Equal(t, test.pauseDurationString, test.deployment.Annotations[options.PauseDeploymentAnnotation], + "Pause interval annotation should match") + } + }) + } +} + // Simple helper function for test cases func FindDeploymentByName(deployments []runtime.Object, deploymentName string) (*app.Deployment, error) { for _, deployment := range deployments { diff --git a/internal/pkg/handler/upgrade.go b/internal/pkg/handler/upgrade.go index 7a33dd33b..508f92688 100644 --- a/internal/pkg/handler/upgrade.go +++ b/internal/pkg/handler/upgrade.go @@ -328,7 +328,6 @@ func upgradeResource(clients kube.Clients, config util.Config, upgradeFuncs call } if strategyResult.Result == constants.Updated { var err error - if upgradeFuncs.SupportsPatch && strategyResult.Patch != nil { err = upgradeFuncs.PatchFunc(clients, config.Namespace, resource, strategyResult.Patch.Type, strategyResult.Patch.Bytes) } else { From 72d95ee12d2f4768edec9ef5dfc50788cdfacb8e Mon Sep 17 00:00:00 2001 From: tom1299 Date: Fri, 4 Jul 2025 11:23:09 +0200 Subject: [PATCH 6/8] Add requires --- .../pkg/callbacks/pause_deployment_test.go | 51 ++++++++----------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/internal/pkg/callbacks/pause_deployment_test.go b/internal/pkg/callbacks/pause_deployment_test.go index d43f5242a..757dedaff 100644 --- a/internal/pkg/callbacks/pause_deployment_test.go +++ b/internal/pkg/callbacks/pause_deployment_test.go @@ -9,6 +9,7 @@ import ( "github.com/stakater/Reloader/internal/pkg/options" "github.com/stakater/Reloader/pkg/kube" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" app "k8s.io/api/apps/v1" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -143,12 +144,12 @@ func TestGetPauseStartTime(t *testing.T) { t.Run(test.name, func(t *testing.T) { actualStartTime, err := GetPauseStartTime(test.deployment) - assert.NoError(t, err) + require.NoError(t, err) if !test.pausedByReloader { assert.Nil(t, actualStartTime) } else { - assert.NotNil(t, actualStartTime) + require.NotNil(t, actualStartTime) assert.WithinDuration(t, test.expectedStartTime, *actualStartTime, time.Second) } }) @@ -189,7 +190,7 @@ func TestParsePauseDuration(t *testing.T) { if test.invalidDuration { assert.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, test.expectedDuration, actualDuration) } }) @@ -257,7 +258,7 @@ func TestHandleMissingTimerSimple(t *testing.T) { context.TODO(), test.deployment, metav1.CreateOptions{}) - assert.NoError(t, err, "Expected no error when creating deployment") + require.NoError(t, err, "Expected no error when creating deployment") pauseDuration, _ := ParsePauseDuration(test.deployment.Annotations[options.PauseDeploymentAnnotation]) HandleMissingTimer(test.deployment, pauseDuration, clients) @@ -352,7 +353,7 @@ func TestPauseDeployment(t *testing.T) { hasBeenPaused := PauseDeployment(test.deployment, clients, false) - assert.Equal(t, test.shouldBePaused, hasBeenPaused, + require.Equal(t, test.shouldBePaused, hasBeenPaused, "PauseDeployment should have returned correct paused state") assert.Equal(t, test.expectedPaused, test.deployment.Spec.Paused, @@ -440,12 +441,12 @@ func TestPauseWithPatch(t *testing.T) { context.TODO(), test.deployment, metav1.CreateOptions{}) - assert.NoError(t, err, "Expected no error when creating deployment") + require.NoError(t, err, "Expected no error when creating deployment") result := PauseWithPatch(test.deployment, clients, test.pauseDurationString, test.pauseDuration) - assert.Equal(t, test.expectedResult, result, - "PauseWithPatch should return correct success/failure state") + require.Equal(t, test.expectedResult, result, + "PauseWithPatch should return correct state") // Check if timer was created timerKey := getTimerKey(test.deployment.Namespace, test.deployment.Name) @@ -459,7 +460,7 @@ func TestPauseWithPatch(t *testing.T) { test.deployment.Name, metav1.GetOptions{}) - assert.NoError(t, err, "Should be able to get updated deployment") + require.NoError(t, err, "Should be able to get updated deployment") assert.True(t, updatedDeployment.Spec.Paused, "Deployment should be paused after patch") pauseTimeAnnotation := updatedDeployment.Annotations[options.PauseDeploymentTimeAnnotation] @@ -520,11 +521,11 @@ func TestApplyPauseToDeployment(t *testing.T) { t.Run(test.name, func(t *testing.T) { ApplyPauseToDeployment(test.deployment, test.pauseDurationString, test.pauseDuration) - assert.Equal(t, test.expectPaused, test.deployment.Spec.Paused, + require.Equal(t, test.expectPaused, test.deployment.Spec.Paused, "Deployment should be paused after applying pause") if test.expectAnnotations { - assert.NotNil(t, test.deployment.Annotations, + require.NotNil(t, test.deployment.Annotations, "Annotations map should be created if it didn't exist") pauseTimeAnnotation := test.deployment.Annotations[options.PauseDeploymentTimeAnnotation] @@ -534,29 +535,19 @@ func TestApplyPauseToDeployment(t *testing.T) { pauseIntervalAnnotation := test.deployment.Annotations[options.PauseDeploymentAnnotation] assert.Equal(t, test.pauseDurationString, pauseIntervalAnnotation, "Pause interval annotation should match the input") - - // If there were existing annotations, make sure they are preserved - if test.deployment.Annotations != nil { - for key, value := range test.deployment.Annotations { - if key != options.PauseDeploymentTimeAnnotation && key != options.PauseDeploymentAnnotation { - assert.Equal(t, value, test.deployment.Annotations[key], - "Existing annotations should be preserved") - } - } - } } }) } } -func TestUpdateDeployment(t *testing.T) { +func TestPerformUpdate(t *testing.T) { tests := []struct { name string deployment *appsv1.Deployment pauseDurationString string pauseDuration time.Duration doPatch bool - expectedResult bool + updateSucceeded bool }{ { name: "update deployment with patch", @@ -572,7 +563,7 @@ func TestUpdateDeployment(t *testing.T) { pauseDurationString: "5m", pauseDuration: 5 * time.Minute, doPatch: true, - expectedResult: true, + updateSucceeded: true, }, { name: "update deployment without patch", @@ -588,7 +579,7 @@ func TestUpdateDeployment(t *testing.T) { pauseDurationString: "10m", pauseDuration: 10 * time.Minute, doPatch: false, - expectedResult: true, + updateSucceeded: true, }, } @@ -610,21 +601,21 @@ func TestUpdateDeployment(t *testing.T) { context.TODO(), test.deployment, metav1.CreateOptions{}) - assert.NoError(t, err, "Expected no error when creating deployment") + require.Equal(t, err, "Expected no error when creating deployment") result := PerformUpdate(test.deployment, clients, test.pauseDurationString, test.pauseDuration, test.doPatch) - assert.Equal(t, test.expectedResult, result, - "updateDeployment should return correct result") + require.Equal(t, test.updateSucceeded, result, + "PerformUpdate should return correct result") if test.doPatch { - // For patch, get deployment again + // For patch, get deployment updatedDeployment, err := fakeClient.AppsV1().Deployments(test.deployment.Namespace).Get( context.TODO(), test.deployment.Name, metav1.GetOptions{}) - assert.NoError(t, err, "Should be able to get updated deployment") + require.NoError(t, err, "Should be able to get updated deployment") assert.True(t, updatedDeployment.Spec.Paused, "Deployment should be paused") assert.NotEmpty(t, updatedDeployment.Annotations[options.PauseDeploymentTimeAnnotation], From b42324d2a49d23ecb5284b57b8109889c2a51032 Mon Sep 17 00:00:00 2001 From: tom1299 Date: Fri, 4 Jul 2025 14:35:22 +0200 Subject: [PATCH 7/8] Fix failing texts --- internal/pkg/callbacks/pause_deployment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/callbacks/pause_deployment_test.go b/internal/pkg/callbacks/pause_deployment_test.go index 757dedaff..938f6332f 100644 --- a/internal/pkg/callbacks/pause_deployment_test.go +++ b/internal/pkg/callbacks/pause_deployment_test.go @@ -601,7 +601,7 @@ func TestPerformUpdate(t *testing.T) { context.TODO(), test.deployment, metav1.CreateOptions{}) - require.Equal(t, err, "Expected no error when creating deployment") + require.Nil(t, err, "Expected no error when creating deployment") result := PerformUpdate(test.deployment, clients, test.pauseDurationString, test.pauseDuration, test.doPatch) From 9dfff2f043fd95106157c541ec4b80021712de9e Mon Sep 17 00:00:00 2001 From: tom1299 Date: Fri, 4 Jul 2025 15:27:25 +0200 Subject: [PATCH 8/8] Change documentation --- internal/pkg/callbacks/pause_deployment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/callbacks/pause_deployment_test.go b/internal/pkg/callbacks/pause_deployment_test.go index 938f6332f..b0aadaf36 100644 --- a/internal/pkg/callbacks/pause_deployment_test.go +++ b/internal/pkg/callbacks/pause_deployment_test.go @@ -634,7 +634,7 @@ func TestPerformUpdate(t *testing.T) { } } -// Simple helper function for test cases +// Simple helper function for the test cases func FindDeploymentByName(deployments []runtime.Object, deploymentName string) (*app.Deployment, error) { for _, deployment := range deployments { accessor, err := meta.Accessor(deployment)