From 5548ce559a4600a15e43150dd0889f730351c836 Mon Sep 17 00:00:00 2001 From: TheiLLeniumStudios <104288623+TheiLLeniumStudios@users.noreply.github.com> Date: Mon, 5 Jan 2026 00:59:57 +0100 Subject: [PATCH] feat: Introduce a Generic ResourceReconciler and a generic BaseWorkload to de-duplicate a lot of code --- internal/pkg/alerting/http.go | 20 +- .../pkg/controller/configmap_reconciler.go | 160 +++-------- internal/pkg/controller/handler.go | 2 +- internal/pkg/controller/manager.go | 57 ++-- .../pkg/controller/resource_reconciler.go | 186 +++++++++++++ internal/pkg/controller/retry.go | 193 ++----------- internal/pkg/controller/retry_test.go | 29 +- internal/pkg/controller/secret_reconciler.go | 160 +++-------- internal/pkg/controller/test_helpers_test.go | 106 +++++--- internal/pkg/http/client.go | 69 +++++ internal/pkg/http/client_test.go | 142 ++++++++++ internal/pkg/metadata/metadata.go | 2 - internal/pkg/metadata/publisher.go | 5 +- internal/pkg/reload/decision.go | 2 +- internal/pkg/reload/pause.go | 6 +- internal/pkg/reload/pause_test.go | 4 +- internal/pkg/reload/service.go | 42 +-- internal/pkg/reload/service_test.go | 77 +++--- internal/pkg/webhook/webhook.go | 9 +- internal/pkg/workload/base.go | 188 +++++++++++++ internal/pkg/workload/cronjob.go | 138 ++++------ internal/pkg/workload/daemonset.go | 119 ++------ internal/pkg/workload/deployment.go | 120 ++------ internal/pkg/workload/deploymentconfig.go | 155 +++-------- internal/pkg/workload/interface.go | 125 +++++---- internal/pkg/workload/job.go | 146 +++++----- internal/pkg/workload/lister.go | 43 +-- internal/pkg/workload/registry.go | 39 ++- internal/pkg/workload/rollout.go | 128 ++------- internal/pkg/workload/statefulset.go | 119 ++------ internal/pkg/workload/workload_test.go | 256 +++++++++++++++++- 31 files changed, 1491 insertions(+), 1356 deletions(-) create mode 100644 internal/pkg/controller/resource_reconciler.go create mode 100644 internal/pkg/http/client.go create mode 100644 internal/pkg/http/client_test.go create mode 100644 internal/pkg/workload/base.go diff --git a/internal/pkg/alerting/http.go b/internal/pkg/alerting/http.go index e5bb3890..2501e695 100644 --- a/internal/pkg/alerting/http.go +++ b/internal/pkg/alerting/http.go @@ -6,8 +6,8 @@ import ( "fmt" "io" "net/http" - "net/url" - "time" + + httputil "github.com/stakater/Reloader/internal/pkg/http" ) // httpClient wraps http.Client with common configuration. @@ -17,20 +17,12 @@ type httpClient struct { // newHTTPClient creates a new httpClient with optional proxy support. func newHTTPClient(proxyURL string) *httpClient { - transport := &http.Transport{} - - if proxyURL != "" { - proxy, err := url.Parse(proxyURL) - if err == nil { - transport.Proxy = http.ProxyURL(proxy) - } - } + cfg := httputil.DefaultConfig() + cfg.Timeout = httputil.AlertingTimeout + cfg.ProxyURL = proxyURL return &httpClient{ - client: &http.Client{ - Transport: transport, - Timeout: 10 * time.Second, - }, + client: httputil.NewClient(cfg), } } diff --git a/internal/pkg/controller/configmap_reconciler.go b/internal/pkg/controller/configmap_reconciler.go index e14c14bf..bfa40622 100644 --- a/internal/pkg/controller/configmap_reconciler.go +++ b/internal/pkg/controller/configmap_reconciler.go @@ -1,10 +1,6 @@ package controller import ( - "context" - "sync" - "time" - "github.com/go-logr/logr" "github.com/stakater/Reloader/internal/pkg/alerting" "github.com/stakater/Reloader/internal/pkg/config" @@ -14,129 +10,57 @@ import ( "github.com/stakater/Reloader/internal/pkg/webhook" "github.com/stakater/Reloader/internal/pkg/workload" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) // ConfigMapReconciler watches ConfigMaps and triggers workload reloads. -type ConfigMapReconciler struct { - client.Client - Log logr.Logger - Config *config.Config - ReloadService *reload.Service - Registry *workload.Registry - Collectors *metrics.Collectors - EventRecorder *events.Recorder - WebhookClient *webhook.Client - Alerter alerting.Alerter - PauseHandler *reload.PauseHandler +type ConfigMapReconciler = ResourceReconciler[*corev1.ConfigMap] - handler *ReloadHandler - initialized bool - initOnce sync.Once +// NewConfigMapReconciler creates a new ConfigMapReconciler with the given dependencies. +func NewConfigMapReconciler( + c client.Client, + log logr.Logger, + cfg *config.Config, + reloadService *reload.Service, + registry *workload.Registry, + collectors *metrics.Collectors, + eventRecorder *events.Recorder, + webhookClient *webhook.Client, + alerter alerting.Alerter, + pauseHandler *reload.PauseHandler, +) *ConfigMapReconciler { + return NewResourceReconciler( + ResourceReconcilerDeps{ + Client: c, + Log: log, + Config: cfg, + ReloadService: reloadService, + Registry: registry, + Collectors: collectors, + EventRecorder: eventRecorder, + WebhookClient: webhookClient, + Alerter: alerter, + PauseHandler: pauseHandler, + }, + ResourceConfig[*corev1.ConfigMap]{ + ResourceType: reload.ResourceTypeConfigMap, + NewResource: func() *corev1.ConfigMap { return &corev1.ConfigMap{} }, + CreateChange: func(cm *corev1.ConfigMap, eventType reload.EventType) reload.ResourceChange { + return reload.ConfigMapChange{ConfigMap: cm, EventType: eventType} + }, + CreatePredicates: func(cfg *config.Config, hasher *reload.Hasher) predicate.Predicate { + return reload.ConfigMapPredicates(cfg, hasher) + }, + }, + ) } -// Reconcile handles ConfigMap events and triggers workload reloads as needed. -func (r *ConfigMapReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - startTime := time.Now() - log := r.Log.WithValues("configmap", req.NamespacedName) - - r.initOnce.Do(func() { - r.initialized = true - log.Info("ConfigMap controller initialized") - }) - - r.Collectors.RecordEventReceived("reconcile", "configmap") - - var cm corev1.ConfigMap - if err := r.Get(ctx, req.NamespacedName, &cm); err != nil { - if errors.IsNotFound(err) { - if r.Config.ReloadOnDelete { - r.Collectors.RecordEventReceived("delete", "configmap") - result, err := r.handleDelete(ctx, req, log) - if err != nil { - r.Collectors.RecordReconcile("error", time.Since(startTime)) - } else { - r.Collectors.RecordReconcile("success", time.Since(startTime)) - } - return result, err - } - r.Collectors.RecordSkipped("not_found") - r.Collectors.RecordReconcile("success", time.Since(startTime)) - return ctrl.Result{}, nil - } - log.Error(err, "failed to get ConfigMap") - r.Collectors.RecordError("get_configmap") - r.Collectors.RecordReconcile("error", time.Since(startTime)) - return ctrl.Result{}, err - } - - if r.Config.IsNamespaceIgnored(cm.Namespace) { - log.V(1).Info("skipping ConfigMap in ignored namespace") - r.Collectors.RecordSkipped("ignored_namespace") - r.Collectors.RecordReconcile("success", time.Since(startTime)) - return ctrl.Result{}, nil - } - - result, err := r.reloadHandler().Process(ctx, cm.Namespace, cm.Name, reload.ResourceTypeConfigMap, - func(workloads []workload.WorkloadAccessor) []reload.ReloadDecision { - return r.ReloadService.Process(reload.ConfigMapChange{ - ConfigMap: &cm, - EventType: reload.EventTypeUpdate, - }, workloads) - }, log) - - if err != nil { - r.Collectors.RecordReconcile("error", time.Since(startTime)) - } else { - r.Collectors.RecordReconcile("success", time.Since(startTime)) - } - return result, err -} - -func (r *ConfigMapReconciler) handleDelete(ctx context.Context, req ctrl.Request, log logr.Logger) (ctrl.Result, error) { - log.Info("handling ConfigMap deletion") - - cm := &corev1.ConfigMap{} - cm.Name = req.Name - cm.Namespace = req.Namespace - - return r.reloadHandler().Process(ctx, req.Namespace, req.Name, reload.ResourceTypeConfigMap, - func(workloads []workload.WorkloadAccessor) []reload.ReloadDecision { - return r.ReloadService.Process(reload.ConfigMapChange{ - ConfigMap: cm, - EventType: reload.EventTypeDelete, - }, workloads) - }, log) -} - -func (r *ConfigMapReconciler) reloadHandler() *ReloadHandler { - if r.handler == nil { - r.handler = &ReloadHandler{ - Client: r.Client, - Lister: workload.NewLister(r.Client, r.Registry, r.Config), - ReloadService: r.ReloadService, - WebhookClient: r.WebhookClient, - Collectors: r.Collectors, - EventRecorder: r.EventRecorder, - Alerter: r.Alerter, - PauseHandler: r.PauseHandler, - } - } - return r.handler -} - -// SetupWithManager sets up the controller with the Manager. -func (r *ConfigMapReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - For(&corev1.ConfigMap{}). - WithEventFilter(BuildEventFilter( - reload.ConfigMapPredicates(r.Config, r.ReloadService.Hasher()), - r.Config, &r.initialized, - )). - Complete(r) +// SetupConfigMapReconciler sets up a ConfigMap reconciler with the manager. +func SetupConfigMapReconciler(mgr ctrl.Manager, r *ConfigMapReconciler) error { + return r.SetupWithManager(mgr, &corev1.ConfigMap{}) } var _ reconcile.Reconciler = &ConfigMapReconciler{} diff --git a/internal/pkg/controller/handler.go b/internal/pkg/controller/handler.go index f4065ce8..46905e81 100644 --- a/internal/pkg/controller/handler.go +++ b/internal/pkg/controller/handler.go @@ -32,7 +32,7 @@ func (h *ReloadHandler) Process( ctx context.Context, namespace, resourceName string, resourceType reload.ResourceType, - getDecisions func([]workload.WorkloadAccessor) []reload.ReloadDecision, + getDecisions func([]workload.Workload) []reload.ReloadDecision, log logr.Logger, ) (ctrl.Result, error) { workloads, err := h.Lister.List(ctx, namespace) diff --git a/internal/pkg/controller/manager.go b/internal/pkg/controller/manager.go index aa1ab6cd..6994c881 100644 --- a/internal/pkg/controller/manager.go +++ b/internal/pkg/controller/manager.go @@ -127,11 +127,12 @@ func NewManagerWithRestConfig(opts ManagerOptions, restConfig *rest.Config) (ctr func SetupReconcilers(mgr ctrl.Manager, cfg *config.Config, log logr.Logger, collectors *metrics.Collectors) error { registry := workload.NewRegistry( workload.RegistryOptions{ - ArgoRolloutsEnabled: cfg.ArgoRolloutsEnabled, - DeploymentConfigEnabled: cfg.DeploymentConfigEnabled, + ArgoRolloutsEnabled: cfg.ArgoRolloutsEnabled, + DeploymentConfigEnabled: cfg.DeploymentConfigEnabled, + RolloutStrategyAnnotation: cfg.Annotations.RolloutStrategy, }, ) - reloadService := reload.NewService(cfg) + reloadService := reload.NewService(cfg, log.WithName("reload")) eventRecorder := events.NewRecorder(mgr.GetEventRecorderFor("reloader")) pauseHandler := reload.NewPauseHandler(cfg) @@ -150,36 +151,38 @@ func SetupReconcilers(mgr ctrl.Manager, cfg *config.Config, log logr.Logger, col // Setup ConfigMap reconciler if !cfg.IsResourceIgnored("configmaps") { - if err := (&ConfigMapReconciler{ - Client: mgr.GetClient(), - Log: log.WithName("configmap-reconciler"), - Config: cfg, - ReloadService: reloadService, - Registry: registry, - Collectors: collectors, - EventRecorder: eventRecorder, - WebhookClient: webhookClient, - Alerter: alerter, - PauseHandler: pauseHandler, - }).SetupWithManager(mgr); err != nil { + cmReconciler := NewConfigMapReconciler( + mgr.GetClient(), + log.WithName("configmap-reconciler"), + cfg, + reloadService, + registry, + collectors, + eventRecorder, + webhookClient, + alerter, + pauseHandler, + ) + if err := SetupConfigMapReconciler(mgr, cmReconciler); err != nil { return fmt.Errorf("setting up configmap reconciler: %w", err) } } // Setup Secret reconciler if !cfg.IsResourceIgnored("secrets") { - if err := (&SecretReconciler{ - Client: mgr.GetClient(), - Log: log.WithName("secret-reconciler"), - Config: cfg, - ReloadService: reloadService, - Registry: registry, - Collectors: collectors, - EventRecorder: eventRecorder, - WebhookClient: webhookClient, - Alerter: alerter, - PauseHandler: pauseHandler, - }).SetupWithManager(mgr); err != nil { + secretReconciler := NewSecretReconciler( + mgr.GetClient(), + log.WithName("secret-reconciler"), + cfg, + reloadService, + registry, + collectors, + eventRecorder, + webhookClient, + alerter, + pauseHandler, + ) + if err := SetupSecretReconciler(mgr, secretReconciler); err != nil { return fmt.Errorf("setting up secret reconciler: %w", err) } } diff --git a/internal/pkg/controller/resource_reconciler.go b/internal/pkg/controller/resource_reconciler.go new file mode 100644 index 00000000..0bd694dc --- /dev/null +++ b/internal/pkg/controller/resource_reconciler.go @@ -0,0 +1,186 @@ +package controller + +import ( + "context" + "sync" + "time" + + "github.com/go-logr/logr" + "github.com/stakater/Reloader/internal/pkg/alerting" + "github.com/stakater/Reloader/internal/pkg/config" + "github.com/stakater/Reloader/internal/pkg/events" + "github.com/stakater/Reloader/internal/pkg/metrics" + "github.com/stakater/Reloader/internal/pkg/reload" + "github.com/stakater/Reloader/internal/pkg/webhook" + "github.com/stakater/Reloader/internal/pkg/workload" + "k8s.io/apimachinery/pkg/api/errors" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +// ResourceReconcilerDeps holds shared dependencies for resource reconcilers. +type ResourceReconcilerDeps struct { + Client client.Client + Log logr.Logger + Config *config.Config + ReloadService *reload.Service + Registry *workload.Registry + Collectors *metrics.Collectors + EventRecorder *events.Recorder + WebhookClient *webhook.Client + Alerter alerting.Alerter + PauseHandler *reload.PauseHandler +} + +// ResourceConfig provides type-specific configuration for a resource reconciler. +type ResourceConfig[T client.Object] struct { + // ResourceType identifies the type of resource (configmap or secret). + ResourceType reload.ResourceType + + // NewResource creates a new instance of the resource type. + NewResource func() T + + // CreateChange creates a change event for the resource. + CreateChange func(resource T, eventType reload.EventType) reload.ResourceChange + + // CreatePredicates creates the predicates for this resource type. + CreatePredicates func(cfg *config.Config, hasher *reload.Hasher) predicate.Predicate +} + +// ResourceReconciler is a generic reconciler for ConfigMaps and Secrets. +type ResourceReconciler[T client.Object] struct { + ResourceReconcilerDeps + ResourceConfig[T] + + handler *ReloadHandler + initialized bool + initOnce sync.Once +} + +// NewResourceReconciler creates a new generic resource reconciler. +func NewResourceReconciler[T client.Object]( + deps ResourceReconcilerDeps, + cfg ResourceConfig[T], +) *ResourceReconciler[T] { + return &ResourceReconciler[T]{ + ResourceReconcilerDeps: deps, + ResourceConfig: cfg, + } +} + +// Reconcile handles resource events and triggers workload reloads as needed. +func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + startTime := time.Now() + resourceType := string(r.ResourceType) + log := r.Log.WithValues(resourceType, req.NamespacedName) + + r.initOnce.Do(func() { + r.initialized = true + log.Info(resourceType + " controller initialized") + }) + + r.Collectors.RecordEventReceived("reconcile", resourceType) + + resource := r.NewResource() + if err := r.Client.Get(ctx, req.NamespacedName, resource); err != nil { + if errors.IsNotFound(err) { + return r.handleNotFound(ctx, req, log, startTime) + } + log.Error(err, "failed to get "+resourceType) + r.Collectors.RecordError("get_" + resourceType) + r.Collectors.RecordReconcile("error", time.Since(startTime)) + return ctrl.Result{}, err + } + + namespace := resource.GetNamespace() + if r.Config.IsNamespaceIgnored(namespace) { + log.V(1).Info("skipping " + resourceType + " in ignored namespace") + r.Collectors.RecordSkipped("ignored_namespace") + r.Collectors.RecordReconcile("success", time.Since(startTime)) + return ctrl.Result{}, nil + } + + result, err := r.reloadHandler().Process(ctx, req.Namespace, req.Name, r.ResourceType, + func(workloads []workload.Workload) []reload.ReloadDecision { + return r.ReloadService.Process(r.CreateChange(resource, reload.EventTypeUpdate), workloads) + }, log) + + r.recordReconcile(startTime, err) + return result, err +} + +func (r *ResourceReconciler[T]) handleNotFound( + ctx context.Context, + req ctrl.Request, + log logr.Logger, + startTime time.Time, +) (ctrl.Result, error) { + if r.Config.ReloadOnDelete { + r.Collectors.RecordEventReceived("delete", string(r.ResourceType)) + result, err := r.handleDelete(ctx, req, log) + r.recordReconcile(startTime, err) + return result, err + } + r.Collectors.RecordSkipped("not_found") + r.Collectors.RecordReconcile("success", time.Since(startTime)) + return ctrl.Result{}, nil +} + +func (r *ResourceReconciler[T]) handleDelete( + ctx context.Context, + req ctrl.Request, + log logr.Logger, +) (ctrl.Result, error) { + log.Info("handling " + string(r.ResourceType) + " deletion") + + // Create a minimal resource with just name/namespace for the delete event + resource := r.NewResource() + resource.SetName(req.Name) + resource.SetNamespace(req.Namespace) + + return r.reloadHandler().Process(ctx, req.Namespace, req.Name, r.ResourceType, + func(workloads []workload.Workload) []reload.ReloadDecision { + return r.ReloadService.Process(r.CreateChange(resource, reload.EventTypeDelete), workloads) + }, log) +} + +func (r *ResourceReconciler[T]) recordReconcile(startTime time.Time, err error) { + if err != nil { + r.Collectors.RecordReconcile("error", time.Since(startTime)) + } else { + r.Collectors.RecordReconcile("success", time.Since(startTime)) + } +} + +func (r *ResourceReconciler[T]) reloadHandler() *ReloadHandler { + if r.handler == nil { + r.handler = &ReloadHandler{ + Client: r.Client, + Lister: workload.NewLister(r.Client, r.Registry, r.Config), + ReloadService: r.ReloadService, + WebhookClient: r.WebhookClient, + Collectors: r.Collectors, + EventRecorder: r.EventRecorder, + Alerter: r.Alerter, + PauseHandler: r.PauseHandler, + } + } + return r.handler +} + +// Initialized returns whether the reconciler has been initialized. +func (r *ResourceReconciler[T]) Initialized() *bool { + return &r.initialized +} + +// SetupWithManager sets up the controller with the Manager. +func (r *ResourceReconciler[T]) SetupWithManager(mgr ctrl.Manager, forObject T) error { + return ctrl.NewControllerManagedBy(mgr). + For(forObject). + WithEventFilter(BuildEventFilter( + r.CreatePredicates(r.Config, r.ReloadService.Hasher()), + r.Config, r.Initialized(), + )). + Complete(r) +} diff --git a/internal/pkg/controller/retry.go b/internal/pkg/controller/retry.go index fec0daac..e71dfa7f 100644 --- a/internal/pkg/controller/retry.go +++ b/internal/pkg/controller/retry.go @@ -2,13 +2,10 @@ package controller import ( "context" - "maps" "github.com/stakater/Reloader/internal/pkg/reload" "github.com/stakater/Reloader/internal/pkg/workload" - batchv1 "k8s.io/api/batch/v1" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -48,33 +45,31 @@ func UpdateObjectWithRetry( // UpdateWorkloadWithRetry updates a workload with exponential backoff on conflict. // On conflict, it re-fetches the object, re-applies the reload changes, and retries. -// For Jobs and CronJobs, special handling is applied: -// - Jobs are deleted and recreated with the same spec -// - CronJobs create a new Job from their template -// For Argo Rollouts, special handling is applied based on the rollout strategy annotation. +// Workloads use their UpdateStrategy to determine how they're updated: +// - UpdateStrategyPatch: uses strategic merge patch with retry (most workloads) +// - UpdateStrategyRecreate: deletes and recreates (Jobs) +// - UpdateStrategyCreateNew: creates a new resource from template (CronJobs) +// Deployments have additional pause handling for paused rollouts. func UpdateWorkloadWithRetry( ctx context.Context, c client.Client, reloadService *reload.Service, pauseHandler *reload.PauseHandler, - wl workload.WorkloadAccessor, + wl workload.Workload, resourceName string, resourceType reload.ResourceType, namespace string, hash string, autoReload bool, ) (bool, error) { - // Handle special workload types - switch wl.Kind() { - case workload.KindJob: - return updateJobWithRecreate(ctx, c, reloadService, wl, resourceName, resourceType, namespace, hash, autoReload) - case workload.KindCronJob: - return updateCronJobWithNewJob(ctx, c, reloadService, wl, resourceName, resourceType, namespace, hash, autoReload) - case workload.KindArgoRollout: - return updateArgoRollout(ctx, c, reloadService, wl, resourceName, resourceType, namespace, hash, autoReload) - case workload.KindDeployment: - return updateDeploymentWithPause(ctx, c, reloadService, pauseHandler, wl, resourceName, resourceType, namespace, hash, autoReload) + switch wl.UpdateStrategy() { + case workload.UpdateStrategyRecreate, workload.UpdateStrategyCreateNew: + return updateWithSpecialStrategy(ctx, c, reloadService, wl, resourceName, resourceType, namespace, hash, autoReload) default: + // UpdateStrategyPatch: use standard retry logic with special handling for Deployments + if wl.Kind() == workload.KindDeployment { + return updateDeploymentWithPause(ctx, c, reloadService, pauseHandler, wl, resourceName, resourceType, namespace, hash, autoReload) + } return updateStandardWorkload(ctx, c, reloadService, wl, resourceName, resourceType, namespace, hash, autoReload) } } @@ -85,7 +80,7 @@ func retryWithReload( ctx context.Context, c client.Client, reloadService *reload.Service, - wl workload.WorkloadAccessor, + wl workload.Workload, resourceName string, resourceType reload.ResourceType, namespace string, @@ -133,7 +128,7 @@ func updateStandardWorkload( ctx context.Context, c client.Client, reloadService *reload.Service, - wl workload.WorkloadAccessor, + wl workload.Workload, resourceName string, resourceType reload.ResourceType, namespace string, @@ -154,7 +149,7 @@ func updateDeploymentWithPause( c client.Client, reloadService *reload.Service, pauseHandler *reload.PauseHandler, - wl workload.WorkloadAccessor, + wl workload.Workload, resourceName string, resourceType reload.ResourceType, namespace string, @@ -176,25 +171,19 @@ func updateDeploymentWithPause( ) } -// updateJobWithRecreate deletes the Job and recreates it with the updated spec. -// Jobs are immutable after creation, so we must delete and recreate. -func updateJobWithRecreate( +// updateWithSpecialStrategy handles workloads that don't use standard patch. +// It applies reload changes, then delegates to the workload's PerformSpecialUpdate. +func updateWithSpecialStrategy( ctx context.Context, c client.Client, reloadService *reload.Service, - wl workload.WorkloadAccessor, + wl workload.Workload, resourceName string, resourceType reload.ResourceType, namespace string, hash string, autoReload bool, ) (bool, error) { - jobWl, ok := wl.(*workload.JobWorkload) - if !ok { - return false, nil - } - - // Apply reload changes to the workload updated, err := reloadService.ApplyReload( ctx, wl, @@ -212,145 +201,5 @@ func updateJobWithRecreate( return false, nil } - oldJob := jobWl.GetJob() - newJob := oldJob.DeepCopy() - - // Delete the old job with background propagation - policy := metav1.DeletePropagationBackground - if err := c.Delete( - ctx, oldJob, &client.DeleteOptions{ - PropagationPolicy: &policy, - }, - ); err != nil { - if !errors.IsNotFound(err) { - return false, err - } - } - - // Clear fields that should not be specified when creating a new Job - newJob.ResourceVersion = "" - newJob.UID = "" - newJob.CreationTimestamp = metav1.Time{} - newJob.Status = batchv1.JobStatus{} - - // Remove problematic labels that are auto-generated - delete(newJob.Spec.Template.Labels, "controller-uid") - delete(newJob.Spec.Template.Labels, batchv1.ControllerUidLabel) - delete(newJob.Spec.Template.Labels, batchv1.JobNameLabel) - delete(newJob.Spec.Template.Labels, "job-name") - - // Remove the selector to allow it to be auto-generated - newJob.Spec.Selector = nil - - // Create the new job with same spec - if err := c.Create(ctx, newJob, client.FieldOwner(workload.FieldManager)); err != nil { - return false, err - } - - return true, nil -} - -// updateCronJobWithNewJob creates a new Job from the CronJob's template. -// CronJobs don't get updated directly; instead, a new Job is triggered. -func updateCronJobWithNewJob( - ctx context.Context, - c client.Client, - reloadService *reload.Service, - wl workload.WorkloadAccessor, - resourceName string, - resourceType reload.ResourceType, - namespace string, - hash string, - autoReload bool, -) (bool, error) { - cronJobWl, ok := wl.(*workload.CronJobWorkload) - if !ok { - return false, nil - } - - // Apply reload changes to get the updated spec - updated, err := reloadService.ApplyReload( - ctx, - wl, - resourceName, - resourceType, - namespace, - hash, - autoReload, - ) - if err != nil { - return false, err - } - - if !updated { - return false, nil - } - - cronJob := cronJobWl.GetCronJob() - - annotations := make(map[string]string) - annotations["cronjob.kubernetes.io/instantiate"] = "manual" - maps.Copy(annotations, cronJob.Spec.JobTemplate.Annotations) - - job := &batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: cronJob.Name + "-", - Namespace: cronJob.Namespace, - Annotations: annotations, - Labels: cronJob.Spec.JobTemplate.Labels, - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(cronJob, batchv1.SchemeGroupVersion.WithKind("CronJob")), - }, - }, - Spec: cronJob.Spec.JobTemplate.Spec, - } - - if err := c.Create(ctx, job, client.FieldOwner(workload.FieldManager)); err != nil { - return false, err - } - - savedAnnotations := maps.Clone(cronJob.Spec.JobTemplate.Spec.Template.Annotations) - - err = UpdateObjectWithRetry( - ctx, c, cronJob, func() (bool, error) { - if cronJob.Spec.JobTemplate.Spec.Template.Annotations == nil { - cronJob.Spec.JobTemplate.Spec.Template.Annotations = make(map[string]string) - } - maps.Copy(cronJob.Spec.JobTemplate.Spec.Template.Annotations, savedAnnotations) - return true, nil - }, - ) - - if err != nil { - return false, err - } - - return true, nil -} - -// updateArgoRollout updates an Argo Rollout using its custom Update method. -// This handles the rollout strategy annotation to determine whether to do -// a standard rollout or set the restartAt field. -func updateArgoRollout( - ctx context.Context, - c client.Client, - reloadService *reload.Service, - wl workload.WorkloadAccessor, - resourceName string, - resourceType reload.ResourceType, - namespace string, - hash string, - autoReload bool, -) (bool, error) { - rolloutWl, ok := wl.(*workload.RolloutWorkload) - if !ok { - return false, nil - } - - return retryWithReload( - ctx, c, reloadService, wl, resourceName, resourceType, namespace, hash, autoReload, - func() error { - return rolloutWl.Update(ctx, c) - }, - ) + return wl.PerformSpecialUpdate(ctx, c) } diff --git a/internal/pkg/controller/retry_test.go b/internal/pkg/controller/retry_test.go index 97271f85..8e43e0e2 100644 --- a/internal/pkg/controller/retry_test.go +++ b/internal/pkg/controller/retry_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/go-logr/logr/testr" "github.com/stakater/Reloader/internal/pkg/config" "github.com/stakater/Reloader/internal/pkg/controller" "github.com/stakater/Reloader/internal/pkg/reload" @@ -22,14 +23,14 @@ func TestUpdateWorkloadWithRetry_WorkloadTypes(t *testing.T) { tests := []struct { name string object runtime.Object - workload func(runtime.Object) workload.WorkloadAccessor + workload func(runtime.Object) workload.Workload resourceType reload.ResourceType verify func(t *testing.T, c client.Client) }{ { name: "Deployment", object: testutil.NewDeployment("test-deployment", "default", nil), - workload: func(o runtime.Object) workload.WorkloadAccessor { + workload: func(o runtime.Object) workload.Workload { return workload.NewDeploymentWorkload(o.(*appsv1.Deployment)) }, resourceType: reload.ResourceTypeConfigMap, @@ -46,7 +47,7 @@ func TestUpdateWorkloadWithRetry_WorkloadTypes(t *testing.T) { { name: "DaemonSet", object: testutil.NewDaemonSet("test-daemonset", "default", nil), - workload: func(o runtime.Object) workload.WorkloadAccessor { + workload: func(o runtime.Object) workload.Workload { return workload.NewDaemonSetWorkload(o.(*appsv1.DaemonSet)) }, resourceType: reload.ResourceTypeSecret, @@ -63,7 +64,7 @@ func TestUpdateWorkloadWithRetry_WorkloadTypes(t *testing.T) { { name: "StatefulSet", object: testutil.NewStatefulSet("test-statefulset", "default", nil), - workload: func(o runtime.Object) workload.WorkloadAccessor { + workload: func(o runtime.Object) workload.Workload { return workload.NewStatefulSetWorkload(o.(*appsv1.StatefulSet)) }, resourceType: reload.ResourceTypeConfigMap, @@ -80,7 +81,7 @@ func TestUpdateWorkloadWithRetry_WorkloadTypes(t *testing.T) { { name: "Job", object: testutil.NewJob("test-job", "default"), - workload: func(o runtime.Object) workload.WorkloadAccessor { + workload: func(o runtime.Object) workload.Workload { return workload.NewJobWorkload(o.(*batchv1.Job)) }, resourceType: reload.ResourceTypeConfigMap, @@ -97,7 +98,7 @@ func TestUpdateWorkloadWithRetry_WorkloadTypes(t *testing.T) { { name: "CronJob", object: testutil.NewCronJob("test-cronjob", "default"), - workload: func(o runtime.Object) workload.WorkloadAccessor { + workload: func(o runtime.Object) workload.Workload { return workload.NewCronJobWorkload(o.(*batchv1.CronJob)) }, resourceType: reload.ResourceTypeSecret, @@ -120,7 +121,7 @@ func TestUpdateWorkloadWithRetry_WorkloadTypes(t *testing.T) { t.Run( tt.name, func(t *testing.T) { cfg := config.NewDefault() - reloadService := reload.NewService(cfg) + reloadService := reload.NewService(cfg, testr.New(t)) fakeClient := fake.NewClientBuilder(). WithScheme(testutil.NewScheme()). @@ -201,7 +202,7 @@ func TestUpdateWorkloadWithRetry_Strategies(t *testing.T) { tt.name, func(t *testing.T) { cfg := config.NewDefault() cfg.ReloadStrategy = tt.strategy - reloadService := reload.NewService(cfg) + reloadService := reload.NewService(cfg, testr.New(t)) deployment := testutil.NewDeployment("test-deployment", "default", nil) fakeClient := fake.NewClientBuilder(). @@ -246,7 +247,7 @@ func TestUpdateWorkloadWithRetry_Strategies(t *testing.T) { func TestUpdateWorkloadWithRetry_NoUpdate(t *testing.T) { cfg := config.NewDefault() - reloadService := reload.NewService(cfg) + reloadService := reload.NewService(cfg, testr.New(t)) deployment := testutil.NewDeployment("test-deployment", "default", nil) deployment.Spec.Template.Spec.Containers[0].Env = []corev1.EnvVar{ @@ -306,7 +307,7 @@ func TestResourceTypeKind(t *testing.T) { func TestUpdateWorkloadWithRetry_PauseDeployment(t *testing.T) { cfg := config.NewDefault() - reloadService := reload.NewService(cfg) + reloadService := reload.NewService(cfg, testr.New(t)) pauseHandler := reload.NewPauseHandler(cfg) deployment := testutil.NewDeployment( @@ -367,7 +368,7 @@ func TestUpdateWorkloadWithRetry_PauseDeployment(t *testing.T) { // TestUpdateWorkloadWithRetry_PauseWithExplicitAnnotation tests pause with explicit configmap annotation (no auto). func TestUpdateWorkloadWithRetry_PauseWithExplicitAnnotation(t *testing.T) { cfg := config.NewDefault() - reloadService := reload.NewService(cfg) + reloadService := reload.NewService(cfg, testr.New(t)) pauseHandler := reload.NewPauseHandler(cfg) deployment := testutil.NewDeployment( @@ -428,7 +429,7 @@ func TestUpdateWorkloadWithRetry_PauseWithExplicitAnnotation(t *testing.T) { // TestUpdateWorkloadWithRetry_PauseWithSecretReload tests pause with Secret-triggered reload. func TestUpdateWorkloadWithRetry_PauseWithSecretReload(t *testing.T) { cfg := config.NewDefault() - reloadService := reload.NewService(cfg) + reloadService := reload.NewService(cfg, testr.New(t)) pauseHandler := reload.NewPauseHandler(cfg) deployment := testutil.NewDeployment( @@ -485,7 +486,7 @@ func TestUpdateWorkloadWithRetry_PauseWithSecretReload(t *testing.T) { // TestUpdateWorkloadWithRetry_PauseWithAutoSecret tests pause with auto annotation + Secret change. func TestUpdateWorkloadWithRetry_PauseWithAutoSecret(t *testing.T) { cfg := config.NewDefault() - reloadService := reload.NewService(cfg) + reloadService := reload.NewService(cfg, testr.New(t)) pauseHandler := reload.NewPauseHandler(cfg) deployment := testutil.NewDeployment( @@ -536,7 +537,7 @@ func TestUpdateWorkloadWithRetry_PauseWithAutoSecret(t *testing.T) { func TestUpdateWorkloadWithRetry_NoPauseWithoutAnnotation(t *testing.T) { cfg := config.NewDefault() - reloadService := reload.NewService(cfg) + reloadService := reload.NewService(cfg, testr.New(t)) pauseHandler := reload.NewPauseHandler(cfg) deployment := testutil.NewDeployment( diff --git a/internal/pkg/controller/secret_reconciler.go b/internal/pkg/controller/secret_reconciler.go index e7b2481b..f20f25a4 100644 --- a/internal/pkg/controller/secret_reconciler.go +++ b/internal/pkg/controller/secret_reconciler.go @@ -1,10 +1,6 @@ package controller import ( - "context" - "sync" - "time" - "github.com/go-logr/logr" "github.com/stakater/Reloader/internal/pkg/alerting" "github.com/stakater/Reloader/internal/pkg/config" @@ -14,129 +10,57 @@ import ( "github.com/stakater/Reloader/internal/pkg/webhook" "github.com/stakater/Reloader/internal/pkg/workload" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) // SecretReconciler watches Secrets and triggers workload reloads. -type SecretReconciler struct { - client.Client - Log logr.Logger - Config *config.Config - ReloadService *reload.Service - Registry *workload.Registry - Collectors *metrics.Collectors - EventRecorder *events.Recorder - WebhookClient *webhook.Client - Alerter alerting.Alerter - PauseHandler *reload.PauseHandler +type SecretReconciler = ResourceReconciler[*corev1.Secret] - handler *ReloadHandler - initialized bool - initOnce sync.Once +// NewSecretReconciler creates a new SecretReconciler with the given dependencies. +func NewSecretReconciler( + c client.Client, + log logr.Logger, + cfg *config.Config, + reloadService *reload.Service, + registry *workload.Registry, + collectors *metrics.Collectors, + eventRecorder *events.Recorder, + webhookClient *webhook.Client, + alerter alerting.Alerter, + pauseHandler *reload.PauseHandler, +) *SecretReconciler { + return NewResourceReconciler( + ResourceReconcilerDeps{ + Client: c, + Log: log, + Config: cfg, + ReloadService: reloadService, + Registry: registry, + Collectors: collectors, + EventRecorder: eventRecorder, + WebhookClient: webhookClient, + Alerter: alerter, + PauseHandler: pauseHandler, + }, + ResourceConfig[*corev1.Secret]{ + ResourceType: reload.ResourceTypeSecret, + NewResource: func() *corev1.Secret { return &corev1.Secret{} }, + CreateChange: func(s *corev1.Secret, eventType reload.EventType) reload.ResourceChange { + return reload.SecretChange{Secret: s, EventType: eventType} + }, + CreatePredicates: func(cfg *config.Config, hasher *reload.Hasher) predicate.Predicate { + return reload.SecretPredicates(cfg, hasher) + }, + }, + ) } -// Reconcile handles Secret events and triggers workload reloads as needed. -func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - startTime := time.Now() - log := r.Log.WithValues("secret", req.NamespacedName) - - r.initOnce.Do(func() { - r.initialized = true - log.Info("Secret controller initialized") - }) - - r.Collectors.RecordEventReceived("reconcile", "secret") - - var secret corev1.Secret - if err := r.Get(ctx, req.NamespacedName, &secret); err != nil { - if errors.IsNotFound(err) { - if r.Config.ReloadOnDelete { - r.Collectors.RecordEventReceived("delete", "secret") - result, err := r.handleDelete(ctx, req, log) - if err != nil { - r.Collectors.RecordReconcile("error", time.Since(startTime)) - } else { - r.Collectors.RecordReconcile("success", time.Since(startTime)) - } - return result, err - } - r.Collectors.RecordSkipped("not_found") - r.Collectors.RecordReconcile("success", time.Since(startTime)) - return ctrl.Result{}, nil - } - log.Error(err, "failed to get Secret") - r.Collectors.RecordError("get_secret") - r.Collectors.RecordReconcile("error", time.Since(startTime)) - return ctrl.Result{}, err - } - - if r.Config.IsNamespaceIgnored(secret.Namespace) { - log.V(1).Info("skipping Secret in ignored namespace") - r.Collectors.RecordSkipped("ignored_namespace") - r.Collectors.RecordReconcile("success", time.Since(startTime)) - return ctrl.Result{}, nil - } - - result, err := r.reloadHandler().Process(ctx, secret.Namespace, secret.Name, reload.ResourceTypeSecret, - func(workloads []workload.WorkloadAccessor) []reload.ReloadDecision { - return r.ReloadService.Process(reload.SecretChange{ - Secret: &secret, - EventType: reload.EventTypeUpdate, - }, workloads) - }, log) - - if err != nil { - r.Collectors.RecordReconcile("error", time.Since(startTime)) - } else { - r.Collectors.RecordReconcile("success", time.Since(startTime)) - } - return result, err -} - -func (r *SecretReconciler) handleDelete(ctx context.Context, req ctrl.Request, log logr.Logger) (ctrl.Result, error) { - log.Info("handling Secret deletion") - - secret := &corev1.Secret{} - secret.Name = req.Name - secret.Namespace = req.Namespace - - return r.reloadHandler().Process(ctx, req.Namespace, req.Name, reload.ResourceTypeSecret, - func(workloads []workload.WorkloadAccessor) []reload.ReloadDecision { - return r.ReloadService.Process(reload.SecretChange{ - Secret: secret, - EventType: reload.EventTypeDelete, - }, workloads) - }, log) -} - -func (r *SecretReconciler) reloadHandler() *ReloadHandler { - if r.handler == nil { - r.handler = &ReloadHandler{ - Client: r.Client, - Lister: workload.NewLister(r.Client, r.Registry, r.Config), - ReloadService: r.ReloadService, - WebhookClient: r.WebhookClient, - Collectors: r.Collectors, - EventRecorder: r.EventRecorder, - Alerter: r.Alerter, - PauseHandler: r.PauseHandler, - } - } - return r.handler -} - -// SetupWithManager sets up the controller with the Manager. -func (r *SecretReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - For(&corev1.Secret{}). - WithEventFilter(BuildEventFilter( - reload.SecretPredicates(r.Config, r.ReloadService.Hasher()), - r.Config, &r.initialized, - )). - Complete(r) +// SetupSecretReconciler sets up a Secret reconciler with the manager. +func SetupSecretReconciler(mgr ctrl.Manager, r *SecretReconciler) error { + return r.SetupWithManager(mgr, &corev1.Secret{}) } var _ reconcile.Reconciler = &SecretReconciler{} diff --git a/internal/pkg/controller/test_helpers_test.go b/internal/pkg/controller/test_helpers_test.go index 916696ab..019be789 100644 --- a/internal/pkg/controller/test_helpers_test.go +++ b/internal/pkg/controller/test_helpers_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/go-logr/logr" "github.com/go-logr/logr/testr" "github.com/stakater/Reloader/internal/pkg/alerting" "github.com/stakater/Reloader/internal/pkg/config" @@ -21,56 +22,77 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +// testDeps holds shared test dependencies. +type testDeps struct { + client *fake.ClientBuilder + log logr.Logger + cfg *config.Config + reloadService *reload.Service + registry *workload.Registry + collectors *metrics.Collectors + eventRecorder *events.Recorder + webhookClient *webhook.Client + alerter alerting.Alerter +} + +// newTestDeps creates shared test dependencies for reconciler tests. +func newTestDeps(t *testing.T, cfg *config.Config, objects ...runtime.Object) testDeps { + t.Helper() + log := testr.New(t) + collectors := metrics.NewCollectors() + return testDeps{ + client: fake.NewClientBuilder(). + WithScheme(testutil.NewScheme()). + WithRuntimeObjects(objects...), + log: log, + cfg: cfg, + reloadService: reload.NewService(cfg, log), + registry: workload.NewRegistry(workload.RegistryOptions{ + ArgoRolloutsEnabled: cfg.ArgoRolloutsEnabled, + DeploymentConfigEnabled: cfg.DeploymentConfigEnabled, + RolloutStrategyAnnotation: cfg.Annotations.RolloutStrategy, + }), + collectors: &collectors, + eventRecorder: events.NewRecorder(nil), + webhookClient: webhook.NewClient("", log), + alerter: &alerting.NoOpAlerter{}, + } +} + // newConfigMapReconciler creates a ConfigMapReconciler for testing. func newConfigMapReconciler(t *testing.T, cfg *config.Config, objects ...runtime.Object) *controller.ConfigMapReconciler { t.Helper() - fakeClient := fake.NewClientBuilder(). - WithScheme(testutil.NewScheme()). - WithRuntimeObjects(objects...). - Build() - - collectors := metrics.NewCollectors() - - return &controller.ConfigMapReconciler{ - Client: fakeClient, - Log: testr.New(t), - Config: cfg, - ReloadService: reload.NewService(cfg), - Registry: workload.NewRegistry(workload.RegistryOptions{ - ArgoRolloutsEnabled: cfg.ArgoRolloutsEnabled, - DeploymentConfigEnabled: cfg.DeploymentConfigEnabled, - }), - Collectors: &collectors, - EventRecorder: events.NewRecorder(nil), - WebhookClient: webhook.NewClient("", testr.New(t)), - Alerter: &alerting.NoOpAlerter{}, - } + deps := newTestDeps(t, cfg, objects...) + return controller.NewConfigMapReconciler( + deps.client.Build(), + deps.log, + deps.cfg, + deps.reloadService, + deps.registry, + deps.collectors, + deps.eventRecorder, + deps.webhookClient, + deps.alerter, + nil, + ) } // newSecretReconciler creates a SecretReconciler for testing. func newSecretReconciler(t *testing.T, cfg *config.Config, objects ...runtime.Object) *controller.SecretReconciler { t.Helper() - fakeClient := fake.NewClientBuilder(). - WithScheme(testutil.NewScheme()). - WithRuntimeObjects(objects...). - Build() - - collectors := metrics.NewCollectors() - - return &controller.SecretReconciler{ - Client: fakeClient, - Log: testr.New(t), - Config: cfg, - ReloadService: reload.NewService(cfg), - Registry: workload.NewRegistry(workload.RegistryOptions{ - ArgoRolloutsEnabled: cfg.ArgoRolloutsEnabled, - DeploymentConfigEnabled: cfg.DeploymentConfigEnabled, - }), - Collectors: &collectors, - EventRecorder: events.NewRecorder(nil), - WebhookClient: webhook.NewClient("", testr.New(t)), - Alerter: &alerting.NoOpAlerter{}, - } + deps := newTestDeps(t, cfg, objects...) + return controller.NewSecretReconciler( + deps.client.Build(), + deps.log, + deps.cfg, + deps.reloadService, + deps.registry, + deps.collectors, + deps.eventRecorder, + deps.webhookClient, + deps.alerter, + nil, + ) } // newNamespaceReconciler creates a NamespaceReconciler for testing. diff --git a/internal/pkg/http/client.go b/internal/pkg/http/client.go new file mode 100644 index 00000000..c1ca613d --- /dev/null +++ b/internal/pkg/http/client.go @@ -0,0 +1,69 @@ +// Package http provides shared HTTP client functionality. +package http + +import ( + "net/http" + "net/url" + "time" +) + +const ( + // DefaultTimeout is the default HTTP client timeout. + DefaultTimeout = 30 * time.Second + + // AlertingTimeout is the shorter timeout used for alerting. + AlertingTimeout = 10 * time.Second +) + +// ClientConfig configures an HTTP client. +type ClientConfig struct { + // Timeout for HTTP requests. + Timeout time.Duration + + // ProxyURL is an optional proxy URL. + ProxyURL string + + // MaxIdleConns controls the maximum number of idle connections. + MaxIdleConns int + + // MaxIdleConnsPerHost controls the maximum idle connections per host. + MaxIdleConnsPerHost int + + // IdleConnTimeout is the maximum time an idle connection remains open. + IdleConnTimeout time.Duration +} + +// DefaultConfig returns the default HTTP client configuration. +func DefaultConfig() ClientConfig { + return ClientConfig{ + Timeout: DefaultTimeout, + MaxIdleConns: 100, + MaxIdleConnsPerHost: 10, + IdleConnTimeout: 90 * time.Second, + } +} + +// NewClient creates a new HTTP client with the given configuration. +func NewClient(cfg ClientConfig) *http.Client { + transport := &http.Transport{ + MaxIdleConns: cfg.MaxIdleConns, + MaxIdleConnsPerHost: cfg.MaxIdleConnsPerHost, + IdleConnTimeout: cfg.IdleConnTimeout, + } + + if cfg.ProxyURL != "" { + if proxy, err := url.Parse(cfg.ProxyURL); err == nil { + transport.Proxy = http.ProxyURL(proxy) + } + } + + return &http.Client{ + Transport: transport, + Timeout: cfg.Timeout, + } +} + +// NewDefaultClient creates an HTTP client with default configuration. +func NewDefaultClient() *http.Client { + return NewClient(DefaultConfig()) +} diff --git a/internal/pkg/http/client_test.go b/internal/pkg/http/client_test.go new file mode 100644 index 00000000..2b937b19 --- /dev/null +++ b/internal/pkg/http/client_test.go @@ -0,0 +1,142 @@ +package http + +import ( + "net/http" + "testing" + "time" +) + +func TestDefaultConfig(t *testing.T) { + cfg := DefaultConfig() + + if cfg.Timeout != DefaultTimeout { + t.Errorf("expected timeout %v, got %v", DefaultTimeout, cfg.Timeout) + } + if cfg.MaxIdleConns != 100 { + t.Errorf("expected MaxIdleConns 100, got %d", cfg.MaxIdleConns) + } + if cfg.MaxIdleConnsPerHost != 10 { + t.Errorf("expected MaxIdleConnsPerHost 10, got %d", cfg.MaxIdleConnsPerHost) + } + if cfg.IdleConnTimeout != 90*time.Second { + t.Errorf("expected IdleConnTimeout 90s, got %v", cfg.IdleConnTimeout) + } +} + +func TestNewClient(t *testing.T) { + tests := []struct { + name string + cfg ClientConfig + wantNil bool + }{ + { + name: "default config", + cfg: DefaultConfig(), + wantNil: false, + }, + { + name: "custom timeout", + cfg: ClientConfig{ + Timeout: 5 * time.Second, + MaxIdleConns: 50, + MaxIdleConnsPerHost: 5, + IdleConnTimeout: 30 * time.Second, + }, + wantNil: false, + }, + { + name: "with proxy", + cfg: ClientConfig{ + Timeout: DefaultTimeout, + ProxyURL: "http://proxy.example.com:8080", + MaxIdleConns: 100, + MaxIdleConnsPerHost: 10, + IdleConnTimeout: 90 * time.Second, + }, + wantNil: false, + }, + { + name: "with invalid proxy URL", + cfg: ClientConfig{ + Timeout: DefaultTimeout, + ProxyURL: "://invalid", + MaxIdleConns: 100, + MaxIdleConnsPerHost: 10, + IdleConnTimeout: 90 * time.Second, + }, + wantNil: false, + }, + { + name: "zero values", + cfg: ClientConfig{ + Timeout: 0, + }, + wantNil: false, + }, + } + + for _, tt := range tests { + t.Run( + tt.name, func(t *testing.T) { + client := NewClient(tt.cfg) + + if tt.wantNil && client != nil { + t.Error("expected nil client") + } + if !tt.wantNil && client == nil { + t.Error("expected non-nil client") + } + + if client != nil { + if client.Timeout != tt.cfg.Timeout { + t.Errorf("expected timeout %v, got %v", tt.cfg.Timeout, client.Timeout) + } + + transport, ok := client.Transport.(*http.Transport) + if !ok { + t.Fatal("expected *http.Transport") + } + if transport.MaxIdleConns != tt.cfg.MaxIdleConns { + t.Errorf("expected MaxIdleConns %d, got %d", tt.cfg.MaxIdleConns, transport.MaxIdleConns) + } + if transport.MaxIdleConnsPerHost != tt.cfg.MaxIdleConnsPerHost { + t.Errorf("expected MaxIdleConnsPerHost %d, got %d", tt.cfg.MaxIdleConnsPerHost, transport.MaxIdleConnsPerHost) + } + } + }, + ) + } +} + +func TestNewDefaultClient(t *testing.T) { + client := NewDefaultClient() + + if client == nil { + t.Fatal("expected non-nil client") + } + + if client.Timeout != DefaultTimeout { + t.Errorf("expected timeout %v, got %v", DefaultTimeout, client.Timeout) + } + + transport, ok := client.Transport.(*http.Transport) + if !ok { + t.Fatal("expected *http.Transport") + } + + if transport.MaxIdleConns != 100 { + t.Errorf("expected MaxIdleConns 100, got %d", transport.MaxIdleConns) + } + if transport.MaxIdleConnsPerHost != 10 { + t.Errorf("expected MaxIdleConnsPerHost 10, got %d", transport.MaxIdleConnsPerHost) + } +} + +func TestConstants(t *testing.T) { + if DefaultTimeout != 30*time.Second { + t.Errorf("expected DefaultTimeout 30s, got %v", DefaultTimeout) + } + if AlertingTimeout != 10*time.Second { + t.Errorf("expected AlertingTimeout 10s, got %v", AlertingTimeout) + } +} diff --git a/internal/pkg/metadata/metadata.go b/internal/pkg/metadata/metadata.go index 616d987a..0e132761 100644 --- a/internal/pkg/metadata/metadata.go +++ b/internal/pkg/metadata/metadata.go @@ -20,8 +20,6 @@ const ( ConfigMapLabelKey = "reloader.stakater.com/meta-info" // ConfigMapLabelValue is the label value for the metadata ConfigMap. ConfigMapLabelValue = "reloader-oss" - // FieldManager is the field manager name for server-side apply. - FieldManager = "reloader" // Environment variables for deployment info. EnvReloaderNamespace = "RELOADER_NAMESPACE" diff --git a/internal/pkg/metadata/publisher.go b/internal/pkg/metadata/publisher.go index b92cc8c7..385dd270 100644 --- a/internal/pkg/metadata/publisher.go +++ b/internal/pkg/metadata/publisher.go @@ -7,6 +7,7 @@ import ( "github.com/go-logr/logr" "github.com/stakater/Reloader/internal/pkg/config" + "github.com/stakater/Reloader/internal/pkg/workload" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" @@ -52,7 +53,7 @@ func (p *Publisher) Publish(ctx context.Context) error { return fmt.Errorf("failed to get existing meta info configmap: %w", err) } p.log.Info("Creating meta info configmap") - if err := p.client.Create(ctx, configMap, client.FieldOwner(FieldManager)); err != nil { + if err := p.client.Create(ctx, configMap, client.FieldOwner(workload.FieldManager)); err != nil { return fmt.Errorf("failed to create meta info configmap: %w", err) } p.log.Info("Meta info configmap created successfully") @@ -62,7 +63,7 @@ func (p *Publisher) Publish(ctx context.Context) error { p.log.Info("Meta info configmap already exists, updating it") existing.Data = configMap.Data existing.Labels = configMap.Labels - if err := p.client.Update(ctx, existing, client.FieldOwner(FieldManager)); err != nil { + if err := p.client.Update(ctx, existing, client.FieldOwner(workload.FieldManager)); err != nil { return fmt.Errorf("failed to update meta info configmap: %w", err) } p.log.Info("Meta info configmap updated successfully") diff --git a/internal/pkg/reload/decision.go b/internal/pkg/reload/decision.go index 6002b3b2..62592582 100644 --- a/internal/pkg/reload/decision.go +++ b/internal/pkg/reload/decision.go @@ -7,7 +7,7 @@ import ( // ReloadDecision contains the result of evaluating whether to reload a workload. type ReloadDecision struct { // Workload is the workload accessor. - Workload workload.WorkloadAccessor + Workload workload.Workload // ShouldReload indicates whether the workload should be reloaded. ShouldReload bool // AutoReload indicates if this is an auto-reload. diff --git a/internal/pkg/reload/pause.go b/internal/pkg/reload/pause.go index e6e33366..78194e78 100644 --- a/internal/pkg/reload/pause.go +++ b/internal/pkg/reload/pause.go @@ -20,7 +20,7 @@ func NewPauseHandler(cfg *config.Config) *PauseHandler { } // ShouldPause checks if a deployment should be paused after reload. -func (h *PauseHandler) ShouldPause(wl workload.WorkloadAccessor) bool { +func (h *PauseHandler) ShouldPause(wl workload.Workload) bool { if wl.Kind() != workload.KindDeployment { return false } @@ -35,7 +35,7 @@ func (h *PauseHandler) ShouldPause(wl workload.WorkloadAccessor) bool { } // GetPausePeriod returns the configured pause period for a workload. -func (h *PauseHandler) GetPausePeriod(wl workload.WorkloadAccessor) (time.Duration, error) { +func (h *PauseHandler) GetPausePeriod(wl workload.Workload) (time.Duration, error) { annotations := wl.GetAnnotations() if annotations == nil { return 0, fmt.Errorf("no annotations on workload") @@ -50,7 +50,7 @@ func (h *PauseHandler) GetPausePeriod(wl workload.WorkloadAccessor) (time.Durati } // ApplyPause pauses a deployment and sets the paused-at annotation. -func (h *PauseHandler) ApplyPause(wl workload.WorkloadAccessor) error { +func (h *PauseHandler) ApplyPause(wl workload.Workload) error { deployWl, ok := wl.(*workload.DeploymentWorkload) if !ok { return fmt.Errorf("workload is not a deployment") diff --git a/internal/pkg/reload/pause_test.go b/internal/pkg/reload/pause_test.go index 74e8162b..49fea4a5 100644 --- a/internal/pkg/reload/pause_test.go +++ b/internal/pkg/reload/pause_test.go @@ -16,7 +16,7 @@ func TestPauseHandler_ShouldPause(t *testing.T) { tests := []struct { name string - workload workload.WorkloadAccessor + workload workload.Workload want bool }{ { @@ -66,7 +66,7 @@ func TestPauseHandler_GetPausePeriod(t *testing.T) { tests := []struct { name string - workload workload.WorkloadAccessor + workload workload.Workload wantPeriod time.Duration wantErr bool }{ diff --git a/internal/pkg/reload/service.go b/internal/pkg/reload/service.go index 96460897..ae2e85f2 100644 --- a/internal/pkg/reload/service.go +++ b/internal/pkg/reload/service.go @@ -3,8 +3,10 @@ package reload import ( "context" "encoding/json" + "fmt" "time" + "github.com/go-logr/logr" "github.com/stakater/Reloader/internal/pkg/config" "github.com/stakater/Reloader/internal/pkg/workload" corev1 "k8s.io/api/core/v1" @@ -13,15 +15,17 @@ import ( // Service orchestrates the reload logic for ConfigMaps and Secrets. type Service struct { cfg *config.Config + log logr.Logger hasher *Hasher matcher *Matcher strategy Strategy } // NewService creates a new reload Service with the given configuration. -func NewService(cfg *config.Config) *Service { +func NewService(cfg *config.Config, log logr.Logger) *Service { return &Service{ cfg: cfg, + log: log, hasher: NewHasher(), matcher: NewMatcher(cfg), strategy: NewStrategy(cfg), @@ -29,7 +33,7 @@ func NewService(cfg *config.Config) *Service { } // Process evaluates all workloads to determine which should be reloaded. -func (s *Service) Process(change ResourceChange, workloads []workload.WorkloadAccessor) []ReloadDecision { +func (s *Service) Process(change ResourceChange, workloads []workload.Workload) []ReloadDecision { if change.IsNil() { return nil } @@ -59,7 +63,7 @@ func (s *Service) processResource( resourceAnnotations map[string]string, resourceType ResourceType, hash string, - workloads []workload.WorkloadAccessor, + workloads []workload.Workload, ) []ReloadDecision { var decisions []ReloadDecision @@ -96,13 +100,15 @@ func (s *Service) processResource( shouldReload = false } - decisions = append(decisions, ReloadDecision{ - Workload: wl, - ShouldReload: shouldReload, - AutoReload: matchResult.AutoReload, - Reason: matchResult.Reason, - Hash: hash, - }) + decisions = append( + decisions, ReloadDecision{ + Workload: wl, + ShouldReload: shouldReload, + AutoReload: matchResult.AutoReload, + Reason: matchResult.Reason, + Hash: hash, + }, + ) } return decisions @@ -124,7 +130,7 @@ func (s *Service) shouldProcessEvent(eventType EventType) bool { // ApplyReload applies the reload strategy to a workload. func (s *Service) ApplyReload( ctx context.Context, - wl workload.WorkloadAccessor, + wl workload.Workload, resourceName string, resourceType ResourceType, namespace string, @@ -149,20 +155,23 @@ func (s *Service) ApplyReload( } if updated { - s.setAttributionAnnotation(wl, resourceName, resourceType, namespace, hash, container) + // Attribution annotation is informational; log errors but don't fail reloads + if err := s.setAttributionAnnotation(wl, resourceName, resourceType, namespace, hash, container); err != nil { + s.log.V(1).Info("failed to set attribution annotation", "error", err, "workload", wl.GetName()) + } } return updated, nil } func (s *Service) setAttributionAnnotation( - wl workload.WorkloadAccessor, + wl workload.Workload, resourceName string, resourceType ResourceType, namespace string, hash string, container *corev1.Container, -) { +) error { containerName := "" if container != nil { containerName = container.Name @@ -179,14 +188,15 @@ func (s *Service) setAttributionAnnotation( sourceJSON, err := json.Marshal(source) if err != nil { - return + return fmt.Errorf("failed to marshal reload source: %w", err) } wl.SetPodTemplateAnnotation(s.cfg.Annotations.LastReloadedFrom, string(sourceJSON)) + return nil } func (s *Service) findTargetContainer( - wl workload.WorkloadAccessor, + wl workload.Workload, resourceName string, resourceType ResourceType, autoReload bool, diff --git a/internal/pkg/reload/service_test.go b/internal/pkg/reload/service_test.go index dae653f7..5a13f025 100644 --- a/internal/pkg/reload/service_test.go +++ b/internal/pkg/reload/service_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/go-logr/logr/testr" "github.com/stakater/Reloader/internal/pkg/config" "github.com/stakater/Reloader/internal/pkg/testutil" "github.com/stakater/Reloader/internal/pkg/workload" @@ -13,7 +14,7 @@ import ( func TestService_ProcessConfigMap_AutoReload(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Create a deployment with auto annotation that uses the configmap deploy := testutil.NewDeployment( @@ -34,7 +35,7 @@ func TestService_ProcessConfigMap_AutoReload(t *testing.T) { }, } - workloads := []workload.WorkloadAccessor{ + workloads := []workload.Workload{ workload.NewDeploymentWorkload(deploy), } @@ -74,7 +75,7 @@ func TestService_ProcessConfigMap_AutoReload(t *testing.T) { func TestService_ProcessConfigMap_ExplicitAnnotation(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment( "test-deploy", "default", map[string]string{ @@ -82,7 +83,7 @@ func TestService_ProcessConfigMap_ExplicitAnnotation(t *testing.T) { }, ) - workloads := []workload.WorkloadAccessor{ + workloads := []workload.Workload{ workload.NewDeploymentWorkload(deploy), } @@ -118,7 +119,7 @@ func TestService_ProcessConfigMap_ExplicitAnnotation(t *testing.T) { func TestService_ProcessConfigMap_IgnoredResource(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Create a deployment with auto annotation deploy := testutil.NewDeployment( @@ -139,7 +140,7 @@ func TestService_ProcessConfigMap_IgnoredResource(t *testing.T) { }, } - workloads := []workload.WorkloadAccessor{ + workloads := []workload.Workload{ workload.NewDeploymentWorkload(deploy), } @@ -174,7 +175,7 @@ func TestService_ProcessConfigMap_IgnoredResource(t *testing.T) { func TestService_ProcessSecret_AutoReload(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Create a deployment with auto annotation that uses the secret deploy := testutil.NewDeployment( @@ -193,7 +194,7 @@ func TestService_ProcessSecret_AutoReload(t *testing.T) { }, } - workloads := []workload.WorkloadAccessor{ + workloads := []workload.Workload{ workload.NewDeploymentWorkload(deploy), } @@ -230,7 +231,7 @@ func TestService_ProcessSecret_AutoReload(t *testing.T) { func TestService_ProcessConfigMap_DeleteEvent(t *testing.T) { cfg := config.NewDefault() cfg.ReloadOnDelete = true - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Create a deployment with explicit configmap annotation deploy := testutil.NewDeployment( @@ -239,7 +240,7 @@ func TestService_ProcessConfigMap_DeleteEvent(t *testing.T) { }, ) - workloads := []workload.WorkloadAccessor{ + workloads := []workload.Workload{ workload.NewDeploymentWorkload(deploy), } @@ -274,7 +275,7 @@ func TestService_ProcessConfigMap_DeleteEvent(t *testing.T) { func TestService_ProcessConfigMap_DeleteEventDisabled(t *testing.T) { cfg := config.NewDefault() cfg.ReloadOnDelete = false // Disabled by default - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment( "test-deploy", "default", map[string]string{ @@ -282,7 +283,7 @@ func TestService_ProcessConfigMap_DeleteEventDisabled(t *testing.T) { }, ) - workloads := []workload.WorkloadAccessor{ + workloads := []workload.Workload{ workload.NewDeploymentWorkload(deploy), } @@ -309,7 +310,7 @@ func TestService_ProcessConfigMap_DeleteEventDisabled(t *testing.T) { func TestService_ApplyReload_EnvVarStrategy(t *testing.T) { cfg := config.NewDefault() cfg.ReloadStrategy = config.ReloadStrategyEnvVars - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment("test-deploy", "default", nil) accessor := workload.NewDeploymentWorkload(deploy) @@ -353,7 +354,7 @@ func TestService_ApplyReload_EnvVarStrategy(t *testing.T) { func TestService_ApplyReload_AnnotationStrategy(t *testing.T) { cfg := config.NewDefault() cfg.ReloadStrategy = config.ReloadStrategyAnnotations - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment("test-deploy", "default", nil) accessor := workload.NewDeploymentWorkload(deploy) @@ -379,7 +380,7 @@ func TestService_ApplyReload_AnnotationStrategy(t *testing.T) { func TestService_ApplyReload_EnvVarDeletion(t *testing.T) { cfg := config.NewDefault() cfg.ReloadStrategy = config.ReloadStrategyEnvVars - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment("test-deploy", "default", nil) // Pre-add an env var @@ -425,7 +426,7 @@ func TestService_ApplyReload_EnvVarDeletion(t *testing.T) { func TestService_ApplyReload_NoChangeIfSameHash(t *testing.T) { cfg := config.NewDefault() cfg.ReloadStrategy = config.ReloadStrategyEnvVars - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment("test-deploy", "default", nil) // Pre-add env var with same hash @@ -448,7 +449,7 @@ func TestService_ApplyReload_NoChangeIfSameHash(t *testing.T) { func TestService_ProcessConfigMap_MultipleWorkloads(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Create multiple workloads deploy1 := testutil.NewDeployment( @@ -494,7 +495,7 @@ func TestService_ProcessConfigMap_MultipleWorkloads(t *testing.T) { }, ) - workloads := []workload.WorkloadAccessor{ + workloads := []workload.Workload{ workload.NewDeploymentWorkload(deploy1), workload.NewDeploymentWorkload(deploy2), workload.NewDeploymentWorkload(deploy3), @@ -535,7 +536,7 @@ func TestService_ProcessConfigMap_MultipleWorkloads(t *testing.T) { func TestService_ProcessConfigMap_DifferentNamespaces(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Create deployments in different namespaces deploy1 := testutil.NewDeployment( @@ -574,7 +575,7 @@ func TestService_ProcessConfigMap_DifferentNamespaces(t *testing.T) { }, } - workloads := []workload.WorkloadAccessor{ + workloads := []workload.Workload{ workload.NewDeploymentWorkload(deploy1), workload.NewDeploymentWorkload(deploy2), } @@ -610,7 +611,7 @@ func TestService_ProcessConfigMap_DifferentNamespaces(t *testing.T) { func TestService_Hasher(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) hasher := svc.Hasher() if hasher == nil { @@ -650,7 +651,7 @@ func TestService_shouldProcessEvent(t *testing.T) { cfg := config.NewDefault() cfg.ReloadOnCreate = tt.reloadOnCreate cfg.ReloadOnDelete = tt.reloadOnDelete - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) result := svc.shouldProcessEvent(tt.eventType) if result != tt.expected { @@ -663,7 +664,7 @@ func TestService_shouldProcessEvent(t *testing.T) { func TestService_findVolumeUsingResource_ConfigMap(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) tests := []struct { name string @@ -749,7 +750,7 @@ func TestService_findVolumeUsingResource_ConfigMap(t *testing.T) { func TestService_findVolumeUsingResource_Secret(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) tests := []struct { name string @@ -824,7 +825,7 @@ func TestService_findVolumeUsingResource_Secret(t *testing.T) { func TestService_findContainerWithVolumeMount(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) tests := []struct { name string @@ -908,7 +909,7 @@ func TestService_findContainerWithVolumeMount(t *testing.T) { func TestService_findContainerWithEnvRef_ConfigMap(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) tests := []struct { name string @@ -1010,7 +1011,7 @@ func TestService_findContainerWithEnvRef_ConfigMap(t *testing.T) { func TestService_findContainerWithEnvRef_Secret(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) tests := []struct { name string @@ -1099,7 +1100,7 @@ func TestService_findContainerWithEnvRef_Secret(t *testing.T) { func TestService_findTargetContainer_AutoReload(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Test with autoReload=true and volume mount deploy := testutil.NewDeployment("test", "default", nil) @@ -1135,7 +1136,7 @@ func TestService_findTargetContainer_AutoReload(t *testing.T) { func TestService_findTargetContainer_AutoReload_EnvRef(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Test with autoReload=true and env ref (no volume) deploy := testutil.NewDeployment("test", "default", nil) @@ -1173,7 +1174,7 @@ func TestService_findTargetContainer_AutoReload_EnvRef(t *testing.T) { func TestService_findTargetContainer_AutoReload_InitContainer(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Test with autoReload=true where init container uses the volume deploy := testutil.NewDeployment("test", "default", nil) @@ -1216,7 +1217,7 @@ func TestService_findTargetContainer_AutoReload_InitContainer(t *testing.T) { func TestService_findTargetContainer_AutoReload_InitContainerEnvRef(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // Test with autoReload=true where init container has env ref deploy := testutil.NewDeployment("test", "default", nil) @@ -1257,7 +1258,7 @@ func TestService_findTargetContainer_AutoReload_InitContainerEnvRef(t *testing.T func TestService_findTargetContainer_NoContainers(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment("test", "default", nil) deploy.Spec.Template.Spec.Containers = []corev1.Container{} @@ -1271,7 +1272,7 @@ func TestService_findTargetContainer_NoContainers(t *testing.T) { func TestService_findTargetContainer_NonAutoReload(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment("test", "default", nil) deploy.Spec.Template.Spec.Containers = []corev1.Container{ @@ -1292,7 +1293,7 @@ func TestService_findTargetContainer_NonAutoReload(t *testing.T) { func TestService_findTargetContainer_AutoReload_FallbackToFirst(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) // autoReload=true but no matching volume or env ref - should fallback to first container deploy := testutil.NewDeployment("test", "default", nil) @@ -1313,10 +1314,10 @@ func TestService_findTargetContainer_AutoReload_FallbackToFirst(t *testing.T) { func TestService_ProcessNilChange(t *testing.T) { cfg := config.NewDefault() - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment("test", "default", nil) - workloads := []workload.WorkloadAccessor{workload.NewDeploymentWorkload(deploy)} + workloads := []workload.Workload{workload.NewDeploymentWorkload(deploy)} // Test with nil ConfigMap change := ConfigMapChange{ @@ -1333,14 +1334,14 @@ func TestService_ProcessNilChange(t *testing.T) { func TestService_ProcessCreateEventDisabled(t *testing.T) { cfg := config.NewDefault() cfg.ReloadOnCreate = false - svc := NewService(cfg) + svc := NewService(cfg, testr.New(t)) deploy := testutil.NewDeployment( "test", "default", map[string]string{ "reloader.stakater.com/auto": "true", }, ) - workloads := []workload.WorkloadAccessor{workload.NewDeploymentWorkload(deploy)} + workloads := []workload.Workload{workload.NewDeploymentWorkload(deploy)} cm := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "test-cm", Namespace: "default"}, diff --git a/internal/pkg/webhook/webhook.go b/internal/pkg/webhook/webhook.go index d5b3c4cd..ea250732 100644 --- a/internal/pkg/webhook/webhook.go +++ b/internal/pkg/webhook/webhook.go @@ -11,6 +11,7 @@ import ( "time" "github.com/go-logr/logr" + httputil "github.com/stakater/Reloader/internal/pkg/http" ) // Payload represents the data sent to the webhook endpoint. @@ -43,11 +44,9 @@ type Client struct { // NewClient creates a new webhook client. func NewClient(url string, log logr.Logger) *Client { return &Client{ - httpClient: &http.Client{ - Timeout: 30 * time.Second, - }, - url: url, - log: log, + httpClient: httputil.NewDefaultClient(), + url: url, + log: log, } } diff --git a/internal/pkg/workload/base.go b/internal/pkg/workload/base.go new file mode 100644 index 00000000..71576b0c --- /dev/null +++ b/internal/pkg/workload/base.go @@ -0,0 +1,188 @@ +package workload + +import ( + "context" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// PodTemplateAccessor provides access to a workload's pod template. +// Each workload type implements this to provide access to its specific template location. +type PodTemplateAccessor interface { + // GetPodTemplateSpec returns a pointer to the pod template spec. + // Returns nil if the workload doesn't have a pod template + GetPodTemplateSpec() *corev1.PodTemplateSpec + + // GetObjectMeta returns the workload's object metadata. + GetObjectMeta() *metav1.ObjectMeta +} + +// BaseWorkload provides common functionality for all workload types. +// It uses composition with a PodTemplateAccessor to access type-specific fields. +type BaseWorkload[T client.Object] struct { + object T + original T + accessor PodTemplateAccessor + kind Kind +} + +// NewBaseWorkload creates a new BaseWorkload with the given object and accessor. +func NewBaseWorkload[T client.Object](obj T, original T, accessor PodTemplateAccessor, kind Kind) *BaseWorkload[T] { + return &BaseWorkload[T]{ + object: obj, + original: original, + accessor: accessor, + kind: kind, + } +} + +func (b *BaseWorkload[T]) Kind() Kind { + return b.kind +} + +func (b *BaseWorkload[T]) GetObject() client.Object { + return b.object +} + +func (b *BaseWorkload[T]) GetName() string { + return b.accessor.GetObjectMeta().Name +} + +func (b *BaseWorkload[T]) GetNamespace() string { + return b.accessor.GetObjectMeta().Namespace +} + +func (b *BaseWorkload[T]) GetAnnotations() map[string]string { + return b.accessor.GetObjectMeta().Annotations +} + +func (b *BaseWorkload[T]) GetPodTemplateAnnotations() map[string]string { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return nil + } + if template.Annotations == nil { + template.Annotations = make(map[string]string) + } + return template.Annotations +} + +func (b *BaseWorkload[T]) SetPodTemplateAnnotation(key, value string) { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return + } + if template.Annotations == nil { + template.Annotations = make(map[string]string) + } + template.Annotations[key] = value +} + +func (b *BaseWorkload[T]) GetContainers() []corev1.Container { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return nil + } + return template.Spec.Containers +} + +func (b *BaseWorkload[T]) SetContainers(containers []corev1.Container) { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return + } + template.Spec.Containers = containers +} + +func (b *BaseWorkload[T]) GetInitContainers() []corev1.Container { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return nil + } + return template.Spec.InitContainers +} + +func (b *BaseWorkload[T]) SetInitContainers(containers []corev1.Container) { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return + } + template.Spec.InitContainers = containers +} + +func (b *BaseWorkload[T]) GetVolumes() []corev1.Volume { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return nil + } + return template.Spec.Volumes +} + +func (b *BaseWorkload[T]) GetEnvFromSources() []corev1.EnvFromSource { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return nil + } + var sources []corev1.EnvFromSource + for _, container := range template.Spec.Containers { + sources = append(sources, container.EnvFrom...) + } + for _, container := range template.Spec.InitContainers { + sources = append(sources, container.EnvFrom...) + } + return sources +} + +func (b *BaseWorkload[T]) UsesConfigMap(name string) bool { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return false + } + return SpecUsesConfigMap(&template.Spec, name) +} + +func (b *BaseWorkload[T]) UsesSecret(name string) bool { + template := b.accessor.GetPodTemplateSpec() + if template == nil { + return false + } + return SpecUsesSecret(&template.Spec, name) +} + +func (b *BaseWorkload[T]) GetOwnerReferences() []metav1.OwnerReference { + return b.accessor.GetObjectMeta().OwnerReferences +} + +// Update performs a strategic merge patch update. +func (b *BaseWorkload[T]) Update(ctx context.Context, c client.Client) error { + return c.Patch(ctx, b.object, client.StrategicMergeFrom(b.original), client.FieldOwner(FieldManager)) +} + +// ResetOriginal resets the original state to the current object state. +func (b *BaseWorkload[T]) ResetOriginal() { + b.original = b.object.DeepCopyObject().(T) +} + +// UpdateStrategy returns the default patch strategy. +// Workloads with special update logic should override this. +func (b *BaseWorkload[T]) UpdateStrategy() UpdateStrategy { + return UpdateStrategyPatch +} + +// PerformSpecialUpdate returns false for standard workloads. +// Workloads with special update logic should override this. +func (b *BaseWorkload[T]) PerformSpecialUpdate(ctx context.Context, c client.Client) (bool, error) { + return false, nil +} + +// Object returns the underlying Kubernetes object. +func (b *BaseWorkload[T]) Object() T { + return b.object +} + +// Original returns the original state of the object. +func (b *BaseWorkload[T]) Original() T { + return b.original +} diff --git a/internal/pkg/workload/cronjob.go b/internal/pkg/workload/cronjob.go index 9f61b019..222d4c61 100644 --- a/internal/pkg/workload/cronjob.go +++ b/internal/pkg/workload/cronjob.go @@ -2,6 +2,7 @@ package workload import ( "context" + "maps" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -9,116 +10,89 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +// cronJobAccessor implements PodTemplateAccessor for CronJob. +type cronJobAccessor struct { + cronjob *batchv1.CronJob +} + +func (a *cronJobAccessor) GetPodTemplateSpec() *corev1.PodTemplateSpec { + // CronJob has the pod template nested under JobTemplate.Spec.Template + return &a.cronjob.Spec.JobTemplate.Spec.Template +} + +func (a *cronJobAccessor) GetObjectMeta() *metav1.ObjectMeta { + return &a.cronjob.ObjectMeta +} + // CronJobWorkload wraps a Kubernetes CronJob. // Note: CronJobs have a special update mechanism - instead of updating the CronJob itself, // Reloader creates a new Job from the CronJob's template. type CronJobWorkload struct { - cronjob *batchv1.CronJob + *BaseWorkload[*batchv1.CronJob] } // NewCronJobWorkload creates a new CronJobWorkload. func NewCronJobWorkload(c *batchv1.CronJob) *CronJobWorkload { - return &CronJobWorkload{cronjob: c} -} - -// Ensure CronJobWorkload implements WorkloadAccessor. -var _ WorkloadAccessor = (*CronJobWorkload)(nil) - -func (w *CronJobWorkload) Kind() Kind { - return KindCronJob -} - -func (w *CronJobWorkload) GetObject() client.Object { - return w.cronjob -} - -func (w *CronJobWorkload) GetName() string { - return w.cronjob.Name -} - -func (w *CronJobWorkload) GetNamespace() string { - return w.cronjob.Namespace -} - -func (w *CronJobWorkload) GetAnnotations() map[string]string { - return w.cronjob.Annotations -} - -// GetPodTemplateAnnotations returns annotations from the JobTemplate's pod template. -func (w *CronJobWorkload) GetPodTemplateAnnotations() map[string]string { - if w.cronjob.Spec.JobTemplate.Spec.Template.Annotations == nil { - w.cronjob.Spec.JobTemplate.Spec.Template.Annotations = make(map[string]string) + original := c.DeepCopy() + accessor := &cronJobAccessor{cronjob: c} + return &CronJobWorkload{ + BaseWorkload: NewBaseWorkload(c, original, accessor, KindCronJob), } - return w.cronjob.Spec.JobTemplate.Spec.Template.Annotations } -func (w *CronJobWorkload) SetPodTemplateAnnotation(key, value string) { - if w.cronjob.Spec.JobTemplate.Spec.Template.Annotations == nil { - w.cronjob.Spec.JobTemplate.Spec.Template.Annotations = make(map[string]string) - } - w.cronjob.Spec.JobTemplate.Spec.Template.Annotations[key] = value -} +// Ensure CronJobWorkload implements Workload. +var _ Workload = (*CronJobWorkload)(nil) -func (w *CronJobWorkload) GetContainers() []corev1.Container { - return w.cronjob.Spec.JobTemplate.Spec.Template.Spec.Containers -} - -func (w *CronJobWorkload) SetContainers(containers []corev1.Container) { - w.cronjob.Spec.JobTemplate.Spec.Template.Spec.Containers = containers -} - -func (w *CronJobWorkload) GetInitContainers() []corev1.Container { - return w.cronjob.Spec.JobTemplate.Spec.Template.Spec.InitContainers -} - -func (w *CronJobWorkload) SetInitContainers(containers []corev1.Container) { - w.cronjob.Spec.JobTemplate.Spec.Template.Spec.InitContainers = containers -} - -func (w *CronJobWorkload) GetVolumes() []corev1.Volume { - return w.cronjob.Spec.JobTemplate.Spec.Template.Spec.Volumes -} - -// Update for CronJob is a no-op - use CreateJobFromCronJob instead. +// Update for CronJob is a no-op - use PerformSpecialUpdate instead. // CronJobs trigger reloads by creating a new Job from their template. func (w *CronJobWorkload) Update(ctx context.Context, c client.Client) error { // CronJobs don't get updated directly - a new Job is created instead - // This is handled by the reload package's special CronJob logic + // This is handled by PerformSpecialUpdate return nil } -func (w *CronJobWorkload) DeepCopy() Workload { - return &CronJobWorkload{cronjob: w.cronjob.DeepCopy()} -} - // ResetOriginal is a no-op for CronJobs since they don't use strategic merge patch. // CronJobs create new Jobs instead of being patched. func (w *CronJobWorkload) ResetOriginal() {} -func (w *CronJobWorkload) GetEnvFromSources() []corev1.EnvFromSource { - var sources []corev1.EnvFromSource - for _, container := range w.cronjob.Spec.JobTemplate.Spec.Template.Spec.Containers { - sources = append(sources, container.EnvFrom...) +func (w *CronJobWorkload) UpdateStrategy() UpdateStrategy { + return UpdateStrategyCreateNew +} + +// PerformSpecialUpdate creates a new Job from the CronJob's template. +// This triggers an immediate execution of the CronJob with updated config. +func (w *CronJobWorkload) PerformSpecialUpdate(ctx context.Context, c client.Client) (bool, error) { + cronJob := w.Object() + + annotations := make(map[string]string) + annotations["cronjob.kubernetes.io/instantiate"] = "manual" + maps.Copy(annotations, cronJob.Spec.JobTemplate.Annotations) + + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: cronJob.Name + "-", + Namespace: cronJob.Namespace, + Annotations: annotations, + Labels: cronJob.Spec.JobTemplate.Labels, + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(cronJob, batchv1.SchemeGroupVersion.WithKind("CronJob")), + }, + }, + Spec: cronJob.Spec.JobTemplate.Spec, } - for _, container := range w.cronjob.Spec.JobTemplate.Spec.Template.Spec.InitContainers { - sources = append(sources, container.EnvFrom...) + + if err := c.Create(ctx, job, client.FieldOwner(FieldManager)); err != nil { + return false, err } - return sources + + return true, nil } -func (w *CronJobWorkload) UsesConfigMap(name string) bool { - return SpecUsesConfigMap(&w.cronjob.Spec.JobTemplate.Spec.Template.Spec, name) -} - -func (w *CronJobWorkload) UsesSecret(name string) bool { - return SpecUsesSecret(&w.cronjob.Spec.JobTemplate.Spec.Template.Spec, name) -} - -func (w *CronJobWorkload) GetOwnerReferences() []metav1.OwnerReference { - return w.cronjob.OwnerReferences +func (w *CronJobWorkload) DeepCopy() Workload { + return NewCronJobWorkload(w.Object().DeepCopy()) } // GetCronJob returns the underlying CronJob for special handling. func (w *CronJobWorkload) GetCronJob() *batchv1.CronJob { - return w.cronjob + return w.Object() } diff --git a/internal/pkg/workload/daemonset.go b/internal/pkg/workload/daemonset.go index c2294a4c..ee6b121e 100644 --- a/internal/pkg/workload/daemonset.go +++ b/internal/pkg/workload/daemonset.go @@ -1,119 +1,46 @@ package workload import ( - "context" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) +// daemonSetAccessor implements PodTemplateAccessor for DaemonSet. +type daemonSetAccessor struct { + daemonset *appsv1.DaemonSet +} + +func (a *daemonSetAccessor) GetPodTemplateSpec() *corev1.PodTemplateSpec { + return &a.daemonset.Spec.Template +} + +func (a *daemonSetAccessor) GetObjectMeta() *metav1.ObjectMeta { + return &a.daemonset.ObjectMeta +} + // DaemonSetWorkload wraps a Kubernetes DaemonSet. type DaemonSetWorkload struct { - daemonset *appsv1.DaemonSet - original *appsv1.DaemonSet + *BaseWorkload[*appsv1.DaemonSet] } // NewDaemonSetWorkload creates a new DaemonSetWorkload. func NewDaemonSetWorkload(d *appsv1.DaemonSet) *DaemonSetWorkload { + original := d.DeepCopy() + accessor := &daemonSetAccessor{daemonset: d} return &DaemonSetWorkload{ - daemonset: d, - original: d.DeepCopy(), + BaseWorkload: NewBaseWorkload(d, original, accessor, KindDaemonSet), } } -// Ensure DaemonSetWorkload implements WorkloadAccessor. -var _ WorkloadAccessor = (*DaemonSetWorkload)(nil) - -func (w *DaemonSetWorkload) Kind() Kind { - return KindDaemonSet -} - -func (w *DaemonSetWorkload) GetObject() client.Object { - return w.daemonset -} - -func (w *DaemonSetWorkload) GetName() string { - return w.daemonset.Name -} - -func (w *DaemonSetWorkload) GetNamespace() string { - return w.daemonset.Namespace -} - -func (w *DaemonSetWorkload) GetAnnotations() map[string]string { - return w.daemonset.Annotations -} - -func (w *DaemonSetWorkload) GetPodTemplateAnnotations() map[string]string { - if w.daemonset.Spec.Template.Annotations == nil { - w.daemonset.Spec.Template.Annotations = make(map[string]string) - } - return w.daemonset.Spec.Template.Annotations -} - -func (w *DaemonSetWorkload) SetPodTemplateAnnotation(key, value string) { - if w.daemonset.Spec.Template.Annotations == nil { - w.daemonset.Spec.Template.Annotations = make(map[string]string) - } - w.daemonset.Spec.Template.Annotations[key] = value -} - -func (w *DaemonSetWorkload) GetContainers() []corev1.Container { - return w.daemonset.Spec.Template.Spec.Containers -} - -func (w *DaemonSetWorkload) SetContainers(containers []corev1.Container) { - w.daemonset.Spec.Template.Spec.Containers = containers -} - -func (w *DaemonSetWorkload) GetInitContainers() []corev1.Container { - return w.daemonset.Spec.Template.Spec.InitContainers -} - -func (w *DaemonSetWorkload) SetInitContainers(containers []corev1.Container) { - w.daemonset.Spec.Template.Spec.InitContainers = containers -} - -func (w *DaemonSetWorkload) GetVolumes() []corev1.Volume { - return w.daemonset.Spec.Template.Spec.Volumes -} - -func (w *DaemonSetWorkload) Update(ctx context.Context, c client.Client) error { - return c.Patch(ctx, w.daemonset, client.StrategicMergeFrom(w.original), client.FieldOwner(FieldManager)) -} +// Ensure DaemonSetWorkload implements Workload. +var _ Workload = (*DaemonSetWorkload)(nil) func (w *DaemonSetWorkload) DeepCopy() Workload { - return &DaemonSetWorkload{ - daemonset: w.daemonset.DeepCopy(), - original: w.original.DeepCopy(), - } + return NewDaemonSetWorkload(w.Object().DeepCopy()) } -func (w *DaemonSetWorkload) ResetOriginal() { - w.original = w.daemonset.DeepCopy() -} - -func (w *DaemonSetWorkload) GetEnvFromSources() []corev1.EnvFromSource { - var sources []corev1.EnvFromSource - for _, container := range w.daemonset.Spec.Template.Spec.Containers { - sources = append(sources, container.EnvFrom...) - } - for _, container := range w.daemonset.Spec.Template.Spec.InitContainers { - sources = append(sources, container.EnvFrom...) - } - return sources -} - -func (w *DaemonSetWorkload) UsesConfigMap(name string) bool { - return SpecUsesConfigMap(&w.daemonset.Spec.Template.Spec, name) -} - -func (w *DaemonSetWorkload) UsesSecret(name string) bool { - return SpecUsesSecret(&w.daemonset.Spec.Template.Spec, name) -} - -func (w *DaemonSetWorkload) GetOwnerReferences() []metav1.OwnerReference { - return w.daemonset.OwnerReferences +// GetDaemonSet returns the underlying DaemonSet for special handling. +func (w *DaemonSetWorkload) GetDaemonSet() *appsv1.DaemonSet { + return w.Object() } diff --git a/internal/pkg/workload/deployment.go b/internal/pkg/workload/deployment.go index 747e9945..ddb621cf 100644 --- a/internal/pkg/workload/deployment.go +++ b/internal/pkg/workload/deployment.go @@ -1,124 +1,46 @@ package workload import ( - "context" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) +// deploymentAccessor implements PodTemplateAccessor for Deployment. +type deploymentAccessor struct { + deployment *appsv1.Deployment +} + +func (a *deploymentAccessor) GetPodTemplateSpec() *corev1.PodTemplateSpec { + return &a.deployment.Spec.Template +} + +func (a *deploymentAccessor) GetObjectMeta() *metav1.ObjectMeta { + return &a.deployment.ObjectMeta +} + // DeploymentWorkload wraps a Kubernetes Deployment. type DeploymentWorkload struct { - deployment *appsv1.Deployment - original *appsv1.Deployment + *BaseWorkload[*appsv1.Deployment] } // NewDeploymentWorkload creates a new DeploymentWorkload. func NewDeploymentWorkload(d *appsv1.Deployment) *DeploymentWorkload { + original := d.DeepCopy() + accessor := &deploymentAccessor{deployment: d} return &DeploymentWorkload{ - deployment: d, - original: d.DeepCopy(), + BaseWorkload: NewBaseWorkload(d, original, accessor, KindDeployment), } } -// Ensure DeploymentWorkload implements WorkloadAccessor. -var _ WorkloadAccessor = (*DeploymentWorkload)(nil) - -func (w *DeploymentWorkload) Kind() Kind { - return KindDeployment -} - -func (w *DeploymentWorkload) GetObject() client.Object { - return w.deployment -} - -func (w *DeploymentWorkload) GetName() string { - return w.deployment.Name -} - -func (w *DeploymentWorkload) GetNamespace() string { - return w.deployment.Namespace -} - -func (w *DeploymentWorkload) GetAnnotations() map[string]string { - return w.deployment.Annotations -} - -func (w *DeploymentWorkload) GetPodTemplateAnnotations() map[string]string { - if w.deployment.Spec.Template.Annotations == nil { - w.deployment.Spec.Template.Annotations = make(map[string]string) - } - return w.deployment.Spec.Template.Annotations -} - -func (w *DeploymentWorkload) SetPodTemplateAnnotation(key, value string) { - if w.deployment.Spec.Template.Annotations == nil { - w.deployment.Spec.Template.Annotations = make(map[string]string) - } - w.deployment.Spec.Template.Annotations[key] = value -} - -func (w *DeploymentWorkload) GetContainers() []corev1.Container { - return w.deployment.Spec.Template.Spec.Containers -} - -func (w *DeploymentWorkload) SetContainers(containers []corev1.Container) { - w.deployment.Spec.Template.Spec.Containers = containers -} - -func (w *DeploymentWorkload) GetInitContainers() []corev1.Container { - return w.deployment.Spec.Template.Spec.InitContainers -} - -func (w *DeploymentWorkload) SetInitContainers(containers []corev1.Container) { - w.deployment.Spec.Template.Spec.InitContainers = containers -} - -func (w *DeploymentWorkload) GetVolumes() []corev1.Volume { - return w.deployment.Spec.Template.Spec.Volumes -} - -func (w *DeploymentWorkload) Update(ctx context.Context, c client.Client) error { - return c.Patch(ctx, w.deployment, client.StrategicMergeFrom(w.original), client.FieldOwner(FieldManager)) -} +// Ensure DeploymentWorkload implements Workload. +var _ Workload = (*DeploymentWorkload)(nil) func (w *DeploymentWorkload) DeepCopy() Workload { - return &DeploymentWorkload{ - deployment: w.deployment.DeepCopy(), - original: w.original.DeepCopy(), - } -} - -func (w *DeploymentWorkload) ResetOriginal() { - w.original = w.deployment.DeepCopy() -} - -func (w *DeploymentWorkload) GetEnvFromSources() []corev1.EnvFromSource { - var sources []corev1.EnvFromSource - for _, container := range w.deployment.Spec.Template.Spec.Containers { - sources = append(sources, container.EnvFrom...) - } - for _, container := range w.deployment.Spec.Template.Spec.InitContainers { - sources = append(sources, container.EnvFrom...) - } - return sources -} - -func (w *DeploymentWorkload) UsesConfigMap(name string) bool { - return SpecUsesConfigMap(&w.deployment.Spec.Template.Spec, name) -} - -func (w *DeploymentWorkload) UsesSecret(name string) bool { - return SpecUsesSecret(&w.deployment.Spec.Template.Spec, name) -} - -func (w *DeploymentWorkload) GetOwnerReferences() []metav1.OwnerReference { - return w.deployment.OwnerReferences + return NewDeploymentWorkload(w.Object().DeepCopy()) } // GetDeployment returns the underlying Deployment for special handling. func (w *DeploymentWorkload) GetDeployment() *appsv1.Deployment { - return w.deployment + return w.Object() } diff --git a/internal/pkg/workload/deploymentconfig.go b/internal/pkg/workload/deploymentconfig.go index 680a78b6..736a486e 100644 --- a/internal/pkg/workload/deploymentconfig.go +++ b/internal/pkg/workload/deploymentconfig.go @@ -1,154 +1,77 @@ package workload import ( - "context" - openshiftv1 "github.com/openshift/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) +// deploymentConfigAccessor implements PodTemplateAccessor for DeploymentConfig. +type deploymentConfigAccessor struct { + dc *openshiftv1.DeploymentConfig +} + +func (a *deploymentConfigAccessor) GetPodTemplateSpec() *corev1.PodTemplateSpec { + // DeploymentConfig has a pointer to PodTemplateSpec which may be nil + return a.dc.Spec.Template +} + +func (a *deploymentConfigAccessor) GetObjectMeta() *metav1.ObjectMeta { + return &a.dc.ObjectMeta +} + // DeploymentConfigWorkload wraps an OpenShift DeploymentConfig. type DeploymentConfigWorkload struct { - dc *openshiftv1.DeploymentConfig - original *openshiftv1.DeploymentConfig + *BaseWorkload[*openshiftv1.DeploymentConfig] } // NewDeploymentConfigWorkload creates a new DeploymentConfigWorkload. func NewDeploymentConfigWorkload(dc *openshiftv1.DeploymentConfig) *DeploymentConfigWorkload { + original := dc.DeepCopy() + accessor := &deploymentConfigAccessor{dc: dc} return &DeploymentConfigWorkload{ - dc: dc, - original: dc.DeepCopy(), + BaseWorkload: NewBaseWorkload(dc, original, accessor, KindDeploymentConfig), } } -// Ensure DeploymentConfigWorkload implements WorkloadAccessor. -var _ WorkloadAccessor = (*DeploymentConfigWorkload)(nil) - -func (w *DeploymentConfigWorkload) Kind() Kind { - return KindDeploymentConfig -} - -func (w *DeploymentConfigWorkload) GetObject() client.Object { - return w.dc -} - -func (w *DeploymentConfigWorkload) GetName() string { - return w.dc.Name -} - -func (w *DeploymentConfigWorkload) GetNamespace() string { - return w.dc.Namespace -} - -func (w *DeploymentConfigWorkload) GetAnnotations() map[string]string { - return w.dc.Annotations -} - -func (w *DeploymentConfigWorkload) GetPodTemplateAnnotations() map[string]string { - if w.dc.Spec.Template == nil { - return nil - } - if w.dc.Spec.Template.Annotations == nil { - w.dc.Spec.Template.Annotations = make(map[string]string) - } - return w.dc.Spec.Template.Annotations -} +// Ensure DeploymentConfigWorkload implements Workload. +var _ Workload = (*DeploymentConfigWorkload)(nil) +// SetPodTemplateAnnotation overrides the base to ensure Template is initialized. func (w *DeploymentConfigWorkload) SetPodTemplateAnnotation(key, value string) { - if w.dc.Spec.Template == nil { - w.dc.Spec.Template = &corev1.PodTemplateSpec{} + dc := w.Object() + if dc.Spec.Template == nil { + dc.Spec.Template = &corev1.PodTemplateSpec{} } - if w.dc.Spec.Template.Annotations == nil { - w.dc.Spec.Template.Annotations = make(map[string]string) + if dc.Spec.Template.Annotations == nil { + dc.Spec.Template.Annotations = make(map[string]string) } - w.dc.Spec.Template.Annotations[key] = value -} - -func (w *DeploymentConfigWorkload) GetContainers() []corev1.Container { - if w.dc.Spec.Template == nil { - return nil - } - return w.dc.Spec.Template.Spec.Containers + dc.Spec.Template.Annotations[key] = value } +// SetContainers overrides the base to ensure Template is initialized. func (w *DeploymentConfigWorkload) SetContainers(containers []corev1.Container) { - if w.dc.Spec.Template == nil { - w.dc.Spec.Template = &corev1.PodTemplateSpec{} + dc := w.Object() + if dc.Spec.Template == nil { + dc.Spec.Template = &corev1.PodTemplateSpec{} } - w.dc.Spec.Template.Spec.Containers = containers -} - -func (w *DeploymentConfigWorkload) GetInitContainers() []corev1.Container { - if w.dc.Spec.Template == nil { - return nil - } - return w.dc.Spec.Template.Spec.InitContainers + dc.Spec.Template.Spec.Containers = containers } +// SetInitContainers overrides the base to ensure Template is initialized. func (w *DeploymentConfigWorkload) SetInitContainers(containers []corev1.Container) { - if w.dc.Spec.Template == nil { - w.dc.Spec.Template = &corev1.PodTemplateSpec{} + dc := w.Object() + if dc.Spec.Template == nil { + dc.Spec.Template = &corev1.PodTemplateSpec{} } - w.dc.Spec.Template.Spec.InitContainers = containers -} - -func (w *DeploymentConfigWorkload) GetVolumes() []corev1.Volume { - if w.dc.Spec.Template == nil { - return nil - } - return w.dc.Spec.Template.Spec.Volumes -} - -func (w *DeploymentConfigWorkload) Update(ctx context.Context, c client.Client) error { - return c.Patch(ctx, w.dc, client.StrategicMergeFrom(w.original), client.FieldOwner(FieldManager)) + dc.Spec.Template.Spec.InitContainers = containers } func (w *DeploymentConfigWorkload) DeepCopy() Workload { - return &DeploymentConfigWorkload{ - dc: w.dc.DeepCopy(), - original: w.original.DeepCopy(), - } -} - -func (w *DeploymentConfigWorkload) ResetOriginal() { - w.original = w.dc.DeepCopy() -} - -func (w *DeploymentConfigWorkload) GetEnvFromSources() []corev1.EnvFromSource { - if w.dc.Spec.Template == nil { - return nil - } - var sources []corev1.EnvFromSource - for _, container := range w.dc.Spec.Template.Spec.Containers { - sources = append(sources, container.EnvFrom...) - } - for _, container := range w.dc.Spec.Template.Spec.InitContainers { - sources = append(sources, container.EnvFrom...) - } - return sources -} - -func (w *DeploymentConfigWorkload) UsesConfigMap(name string) bool { - if w.dc.Spec.Template == nil { - return false - } - return SpecUsesConfigMap(&w.dc.Spec.Template.Spec, name) -} - -func (w *DeploymentConfigWorkload) UsesSecret(name string) bool { - if w.dc.Spec.Template == nil { - return false - } - return SpecUsesSecret(&w.dc.Spec.Template.Spec, name) -} - -func (w *DeploymentConfigWorkload) GetOwnerReferences() []metav1.OwnerReference { - return w.dc.OwnerReferences + return NewDeploymentConfigWorkload(w.Object().DeepCopy()) } // GetDeploymentConfig returns the underlying DeploymentConfig for special handling. func (w *DeploymentConfigWorkload) GetDeploymentConfig() *openshiftv1.DeploymentConfig { - return w.dc + return w.Object() } diff --git a/internal/pkg/workload/interface.go b/internal/pkg/workload/interface.go index 40249edb..6b4ccf7d 100644 --- a/internal/pkg/workload/interface.go +++ b/internal/pkg/workload/interface.go @@ -31,9 +31,20 @@ const ( KindDeploymentConfig Kind = "DeploymentConfig" ) -// Workload provides a uniform interface for managing Kubernetes workloads. -// All implementations must be safe for concurrent use. -type Workload interface { +// UpdateStrategy defines how a workload should be updated. +type UpdateStrategy int + +const ( + // UpdateStrategyPatch uses strategic merge patch (default for most workloads). + UpdateStrategyPatch UpdateStrategy = iota + // UpdateStrategyRecreate deletes and recreates the workload (Jobs). + UpdateStrategyRecreate + // UpdateStrategyCreateNew creates a new resource from template (CronJobs). + UpdateStrategyCreateNew +) + +// WorkloadIdentity provides basic identification for a workload. +type WorkloadIdentity interface { // Kind returns the workload type. Kind() Kind @@ -45,54 +56,11 @@ type Workload interface { // GetNamespace returns the workload namespace. GetNamespace() string - - // GetAnnotations returns the workload's annotations. - GetAnnotations() map[string]string - - // GetPodTemplateAnnotations returns annotations from the pod template spec. - GetPodTemplateAnnotations() map[string]string - - // SetPodTemplateAnnotation sets an annotation on the pod template. - SetPodTemplateAnnotation(key, value string) - - // GetContainers returns all containers (including init containers). - GetContainers() []corev1.Container - - // SetContainers updates the containers. - SetContainers(containers []corev1.Container) - - // GetInitContainers returns all init containers. - GetInitContainers() []corev1.Container - - // SetInitContainers updates the init containers. - SetInitContainers(containers []corev1.Container) - - // GetVolumes returns the pod template volumes. - GetVolumes() []corev1.Volume - - // Update persists changes to the workload. - Update(ctx context.Context, c client.Client) error - - // ResetOriginal resets the original state to the current object state. - // This should be called after re-fetching the object (e.g., after a conflict) - // to ensure strategic merge patch diffs are calculated correctly. - ResetOriginal() - - // DeepCopy returns a deep copy of the workload. - DeepCopy() Workload } -// Accessor provides read-only access to workload configuration. -// Use this interface when you only need to inspect workload state. -type Accessor interface { - // Kind returns the workload type. - Kind() Kind - - // GetName returns the workload name. - GetName() string - - // GetNamespace returns the workload namespace. - GetNamespace() string +// WorkloadReader provides read-only access to workload state. +type WorkloadReader interface { + WorkloadIdentity // GetAnnotations returns the workload's annotations. GetAnnotations() map[string]string @@ -112,19 +80,62 @@ type Accessor interface { // GetEnvFromSources returns all envFrom sources from all containers. GetEnvFromSources() []corev1.EnvFromSource + // GetOwnerReferences returns the owner references of the workload. + GetOwnerReferences() []metav1.OwnerReference +} + +// WorkloadMatcher provides methods for checking resource usage. +type WorkloadMatcher interface { // UsesConfigMap checks if the workload uses a specific ConfigMap. UsesConfigMap(name string) bool // UsesSecret checks if the workload uses a specific Secret. UsesSecret(name string) bool - - // GetOwnerReferences returns the owner references of the workload. - GetOwnerReferences() []metav1.OwnerReference } -// WorkloadAccessor provides both Workload and Accessor interfaces. -// This is the primary type returned by the registry. -type WorkloadAccessor interface { - Workload - Accessor +// WorkloadMutator provides methods for modifying workload state. +type WorkloadMutator interface { + // SetPodTemplateAnnotation sets an annotation on the pod template. + SetPodTemplateAnnotation(key, value string) + + // SetContainers updates the containers. + SetContainers(containers []corev1.Container) + + // SetInitContainers updates the init containers. + SetInitContainers(containers []corev1.Container) +} + +// WorkloadUpdater provides methods for persisting workload changes. +type WorkloadUpdater interface { + // Update persists changes to the workload. + Update(ctx context.Context, c client.Client) error + + // UpdateStrategy returns how this workload should be updated. + // Most workloads use UpdateStrategyPatch (strategic merge patch). + // Jobs use UpdateStrategyRecreate (delete and recreate). + // CronJobs use UpdateStrategyCreateNew (create a new Job from template). + UpdateStrategy() UpdateStrategy + + // PerformSpecialUpdate handles non-standard update logic. + // This is called when UpdateStrategy() != UpdateStrategyPatch. + // For UpdateStrategyPatch workloads, this returns (false, nil). + PerformSpecialUpdate(ctx context.Context, c client.Client) (updated bool, err error) + + // ResetOriginal resets the original state to the current object state. + // This should be called after re-fetching the object (e.g., after a conflict) + // to ensure strategic merge patch diffs are calculated correctly. + ResetOriginal() + + // DeepCopy returns a deep copy of the workload. + DeepCopy() Workload +} + +// Workload combines all workload interfaces for full workload access. +// Use specific interfaces (WorkloadReader, WorkloadMatcher, etc.) when possible +// to limit scope and improve testability. +type Workload interface { + WorkloadReader + WorkloadMatcher + WorkloadMutator + WorkloadUpdater } diff --git a/internal/pkg/workload/job.go b/internal/pkg/workload/job.go index 291249f8..557c8f6c 100644 --- a/internal/pkg/workload/job.go +++ b/internal/pkg/workload/job.go @@ -5,119 +5,103 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) +// jobAccessor implements PodTemplateAccessor for Job. +type jobAccessor struct { + job *batchv1.Job +} + +func (a *jobAccessor) GetPodTemplateSpec() *corev1.PodTemplateSpec { + return &a.job.Spec.Template +} + +func (a *jobAccessor) GetObjectMeta() *metav1.ObjectMeta { + return &a.job.ObjectMeta +} + // JobWorkload wraps a Kubernetes Job. // Note: Jobs have a special update mechanism - instead of updating the Job, // Reloader deletes and recreates it with the same spec. type JobWorkload struct { - job *batchv1.Job + *BaseWorkload[*batchv1.Job] } // NewJobWorkload creates a new JobWorkload. func NewJobWorkload(j *batchv1.Job) *JobWorkload { - return &JobWorkload{job: j} -} - -// Ensure JobWorkload implements WorkloadAccessor. -var _ WorkloadAccessor = (*JobWorkload)(nil) - -func (w *JobWorkload) Kind() Kind { - return KindJob -} - -func (w *JobWorkload) GetObject() client.Object { - return w.job -} - -func (w *JobWorkload) GetName() string { - return w.job.Name -} - -func (w *JobWorkload) GetNamespace() string { - return w.job.Namespace -} - -func (w *JobWorkload) GetAnnotations() map[string]string { - return w.job.Annotations -} - -func (w *JobWorkload) GetPodTemplateAnnotations() map[string]string { - if w.job.Spec.Template.Annotations == nil { - w.job.Spec.Template.Annotations = make(map[string]string) + original := j.DeepCopy() + accessor := &jobAccessor{job: j} + return &JobWorkload{ + BaseWorkload: NewBaseWorkload(j, original, accessor, KindJob), } - return w.job.Spec.Template.Annotations } -func (w *JobWorkload) SetPodTemplateAnnotation(key, value string) { - if w.job.Spec.Template.Annotations == nil { - w.job.Spec.Template.Annotations = make(map[string]string) - } - w.job.Spec.Template.Annotations[key] = value -} +// Ensure JobWorkload implements Workload. +var _ Workload = (*JobWorkload)(nil) -func (w *JobWorkload) GetContainers() []corev1.Container { - return w.job.Spec.Template.Spec.Containers -} - -func (w *JobWorkload) SetContainers(containers []corev1.Container) { - w.job.Spec.Template.Spec.Containers = containers -} - -func (w *JobWorkload) GetInitContainers() []corev1.Container { - return w.job.Spec.Template.Spec.InitContainers -} - -func (w *JobWorkload) SetInitContainers(containers []corev1.Container) { - w.job.Spec.Template.Spec.InitContainers = containers -} - -func (w *JobWorkload) GetVolumes() []corev1.Volume { - return w.job.Spec.Template.Spec.Volumes -} - -// Update for Job is a no-op - use RecreateJob instead. +// Update for Job is a no-op - use PerformSpecialUpdate instead. // Jobs trigger reloads by being deleted and recreated. func (w *JobWorkload) Update(ctx context.Context, c client.Client) error { // Jobs don't get updated directly - they are deleted and recreated - // This is handled by the reload package's special Job logic + // This is handled by PerformSpecialUpdate return nil } -func (w *JobWorkload) DeepCopy() Workload { - return &JobWorkload{job: w.job.DeepCopy()} -} - // ResetOriginal is a no-op for Jobs since they don't use strategic merge patch. // Jobs are deleted and recreated instead of being patched. func (w *JobWorkload) ResetOriginal() {} -func (w *JobWorkload) GetEnvFromSources() []corev1.EnvFromSource { - var sources []corev1.EnvFromSource - for _, container := range w.job.Spec.Template.Spec.Containers { - sources = append(sources, container.EnvFrom...) +func (w *JobWorkload) UpdateStrategy() UpdateStrategy { + return UpdateStrategyRecreate +} + +// PerformSpecialUpdate deletes the Job and recreates it with the updated spec. +// This is necessary because Jobs are immutable after creation. +func (w *JobWorkload) PerformSpecialUpdate(ctx context.Context, c client.Client) (bool, error) { + oldJob := w.Object() + newJob := oldJob.DeepCopy() + + // Delete the old job with background propagation + policy := metav1.DeletePropagationBackground + if err := c.Delete(ctx, oldJob, &client.DeleteOptions{ + PropagationPolicy: &policy, + }); err != nil { + if !errors.IsNotFound(err) { + return false, err + } } - for _, container := range w.job.Spec.Template.Spec.InitContainers { - sources = append(sources, container.EnvFrom...) + + // Clear fields that should not be specified when creating a new Job + newJob.ResourceVersion = "" + newJob.UID = "" + newJob.CreationTimestamp = metav1.Time{} + newJob.Status = batchv1.JobStatus{} + + // Remove problematic labels that are auto-generated + delete(newJob.Spec.Template.Labels, "controller-uid") + delete(newJob.Spec.Template.Labels, batchv1.ControllerUidLabel) + delete(newJob.Spec.Template.Labels, batchv1.JobNameLabel) + delete(newJob.Spec.Template.Labels, "job-name") + + // Remove the selector to allow it to be auto-generated + newJob.Spec.Selector = nil + + // Create the new job with same spec + if err := c.Create(ctx, newJob, client.FieldOwner(FieldManager)); err != nil { + return false, err } - return sources + + return true, nil } -func (w *JobWorkload) UsesConfigMap(name string) bool { - return SpecUsesConfigMap(&w.job.Spec.Template.Spec, name) -} - -func (w *JobWorkload) UsesSecret(name string) bool { - return SpecUsesSecret(&w.job.Spec.Template.Spec, name) -} - -func (w *JobWorkload) GetOwnerReferences() []metav1.OwnerReference { - return w.job.OwnerReferences +func (w *JobWorkload) DeepCopy() Workload { + return NewJobWorkload(w.Object().DeepCopy()) } // GetJob returns the underlying Job for special handling. func (w *JobWorkload) GetJob() *batchv1.Job { - return w.job + return w.Object() } diff --git a/internal/pkg/workload/lister.go b/internal/pkg/workload/lister.go index 07cde615..1b982fea 100644 --- a/internal/pkg/workload/lister.go +++ b/internal/pkg/workload/lister.go @@ -3,7 +3,6 @@ package workload import ( "context" - argorolloutv1alpha1 "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" openshiftv1 "github.com/openshift/api/apps/v1" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" @@ -32,8 +31,8 @@ func NewLister(c client.Client, registry *Registry, checker IgnoreChecker) *List } // List returns all workloads in the given namespace. -func (l *Lister) List(ctx context.Context, namespace string) ([]WorkloadAccessor, error) { - var result []WorkloadAccessor +func (l *Lister) List(ctx context.Context, namespace string) ([]Workload, error) { + var result []Workload for _, kind := range l.Registry.SupportedKinds() { if l.Checker != nil && l.Checker.IsWorkloadIgnored(string(kind)) { @@ -50,7 +49,7 @@ func (l *Lister) List(ctx context.Context, namespace string) ([]WorkloadAccessor return result, nil } -func (l *Lister) listByKind(ctx context.Context, namespace string, kind Kind) ([]WorkloadAccessor, error) { +func (l *Lister) listByKind(ctx context.Context, namespace string, kind Kind) ([]Workload, error) { lister := l.Registry.ListerFor(kind) if lister == nil { return nil, nil @@ -58,84 +57,72 @@ func (l *Lister) listByKind(ctx context.Context, namespace string, kind Kind) ([ return lister(ctx, l.Client, namespace) } -func listDeployments(ctx context.Context, c client.Client, namespace string) ([]WorkloadAccessor, error) { +func listDeployments(ctx context.Context, c client.Client, namespace string) ([]Workload, error) { var list appsv1.DeploymentList if err := c.List(ctx, &list, client.InNamespace(namespace)); err != nil { return nil, err } - result := make([]WorkloadAccessor, len(list.Items)) + result := make([]Workload, len(list.Items)) for i := range list.Items { result[i] = NewDeploymentWorkload(&list.Items[i]) } return result, nil } -func listDaemonSets(ctx context.Context, c client.Client, namespace string) ([]WorkloadAccessor, error) { +func listDaemonSets(ctx context.Context, c client.Client, namespace string) ([]Workload, error) { var list appsv1.DaemonSetList if err := c.List(ctx, &list, client.InNamespace(namespace)); err != nil { return nil, err } - result := make([]WorkloadAccessor, len(list.Items)) + result := make([]Workload, len(list.Items)) for i := range list.Items { result[i] = NewDaemonSetWorkload(&list.Items[i]) } return result, nil } -func listStatefulSets(ctx context.Context, c client.Client, namespace string) ([]WorkloadAccessor, error) { +func listStatefulSets(ctx context.Context, c client.Client, namespace string) ([]Workload, error) { var list appsv1.StatefulSetList if err := c.List(ctx, &list, client.InNamespace(namespace)); err != nil { return nil, err } - result := make([]WorkloadAccessor, len(list.Items)) + result := make([]Workload, len(list.Items)) for i := range list.Items { result[i] = NewStatefulSetWorkload(&list.Items[i]) } return result, nil } -func listJobs(ctx context.Context, c client.Client, namespace string) ([]WorkloadAccessor, error) { +func listJobs(ctx context.Context, c client.Client, namespace string) ([]Workload, error) { var list batchv1.JobList if err := c.List(ctx, &list, client.InNamespace(namespace)); err != nil { return nil, err } - result := make([]WorkloadAccessor, len(list.Items)) + result := make([]Workload, len(list.Items)) for i := range list.Items { result[i] = NewJobWorkload(&list.Items[i]) } return result, nil } -func listCronJobs(ctx context.Context, c client.Client, namespace string) ([]WorkloadAccessor, error) { +func listCronJobs(ctx context.Context, c client.Client, namespace string) ([]Workload, error) { var list batchv1.CronJobList if err := c.List(ctx, &list, client.InNamespace(namespace)); err != nil { return nil, err } - result := make([]WorkloadAccessor, len(list.Items)) + result := make([]Workload, len(list.Items)) for i := range list.Items { result[i] = NewCronJobWorkload(&list.Items[i]) } return result, nil } -func listRollouts(ctx context.Context, c client.Client, namespace string) ([]WorkloadAccessor, error) { - var list argorolloutv1alpha1.RolloutList - if err := c.List(ctx, &list, client.InNamespace(namespace)); err != nil { - return nil, err - } - result := make([]WorkloadAccessor, len(list.Items)) - for i := range list.Items { - result[i] = NewRolloutWorkload(&list.Items[i]) - } - return result, nil -} - -func listDeploymentConfigs(ctx context.Context, c client.Client, namespace string) ([]WorkloadAccessor, error) { +func listDeploymentConfigs(ctx context.Context, c client.Client, namespace string) ([]Workload, error) { var list openshiftv1.DeploymentConfigList if err := c.List(ctx, &list, client.InNamespace(namespace)); err != nil { return nil, err } - result := make([]WorkloadAccessor, len(list.Items)) + result := make([]Workload, len(list.Items)) for i := range list.Items { result[i] = NewDeploymentConfigWorkload(&list.Items[i]) } diff --git a/internal/pkg/workload/registry.go b/internal/pkg/workload/registry.go index 3516338d..5392eca1 100644 --- a/internal/pkg/workload/registry.go +++ b/internal/pkg/workload/registry.go @@ -13,26 +13,29 @@ import ( ) // WorkloadLister is a function that lists workloads of a specific kind. -type WorkloadLister func(ctx context.Context, c client.Client, namespace string) ([]WorkloadAccessor, error) +type WorkloadLister func(ctx context.Context, c client.Client, namespace string) ([]Workload, error) // RegistryOptions configures the workload registry. type RegistryOptions struct { - ArgoRolloutsEnabled bool - DeploymentConfigEnabled bool + ArgoRolloutsEnabled bool + DeploymentConfigEnabled bool + RolloutStrategyAnnotation string } // Registry provides factory methods for creating Workload instances. type Registry struct { - argoRolloutsEnabled bool - deploymentConfigEnabled bool - listers map[Kind]WorkloadLister + argoRolloutsEnabled bool + deploymentConfigEnabled bool + rolloutStrategyAnnotation string + listers map[Kind]WorkloadLister } // NewRegistry creates a new workload registry. func NewRegistry(opts RegistryOptions) *Registry { r := &Registry{ - argoRolloutsEnabled: opts.ArgoRolloutsEnabled, - deploymentConfigEnabled: opts.DeploymentConfigEnabled, + argoRolloutsEnabled: opts.ArgoRolloutsEnabled, + deploymentConfigEnabled: opts.DeploymentConfigEnabled, + rolloutStrategyAnnotation: opts.RolloutStrategyAnnotation, listers: map[Kind]WorkloadLister{ KindDeployment: listDeployments, KindDaemonSet: listDaemonSets, @@ -42,7 +45,19 @@ func NewRegistry(opts RegistryOptions) *Registry { }, } if opts.ArgoRolloutsEnabled { - r.listers[KindArgoRollout] = listRollouts + // Use closure to capture the strategy annotation + strategyAnnotation := opts.RolloutStrategyAnnotation + r.listers[KindArgoRollout] = func(ctx context.Context, c client.Client, namespace string) ([]Workload, error) { + var list argorolloutv1alpha1.RolloutList + if err := c.List(ctx, &list, client.InNamespace(namespace)); err != nil { + return nil, err + } + result := make([]Workload, len(list.Items)) + for i := range list.Items { + result[i] = NewRolloutWorkload(&list.Items[i], strategyAnnotation) + } + return result, nil + } } if opts.DeploymentConfigEnabled { r.listers[KindDeploymentConfig] = listDeploymentConfigs @@ -73,8 +88,8 @@ func (r *Registry) SupportedKinds() []Kind { return kinds } -// FromObject creates a WorkloadAccessor from a Kubernetes object. -func (r *Registry) FromObject(obj client.Object) (WorkloadAccessor, error) { +// FromObject creates a Workload from a Kubernetes object. +func (r *Registry) FromObject(obj client.Object) (Workload, error) { switch o := obj.(type) { case *appsv1.Deployment: return NewDeploymentWorkload(o), nil @@ -90,7 +105,7 @@ func (r *Registry) FromObject(obj client.Object) (WorkloadAccessor, error) { if !r.argoRolloutsEnabled { return nil, fmt.Errorf("argo Rollouts support is not enabled") } - return NewRolloutWorkload(o), nil + return NewRolloutWorkload(o, r.rolloutStrategyAnnotation), nil case *openshiftv1.DeploymentConfig: if !r.deploymentConfigEnabled { return nil, fmt.Errorf("openShift DeploymentConfig support is not enabled") diff --git a/internal/pkg/workload/rollout.go b/internal/pkg/workload/rollout.go index 8e78d3e9..39d70fc7 100644 --- a/internal/pkg/workload/rollout.go +++ b/internal/pkg/workload/rollout.go @@ -22,79 +22,39 @@ const ( RolloutStrategyRestart RolloutStrategy = "restart" ) -// RolloutStrategyAnnotation is the annotation key for specifying the rollout strategy. -const RolloutStrategyAnnotation = "reloader.stakater.com/rollout-strategy" +// rolloutAccessor implements PodTemplateAccessor for Rollout. +type rolloutAccessor struct { + rollout *argorolloutv1alpha1.Rollout +} + +func (a *rolloutAccessor) GetPodTemplateSpec() *corev1.PodTemplateSpec { + return &a.rollout.Spec.Template +} + +func (a *rolloutAccessor) GetObjectMeta() *metav1.ObjectMeta { + return &a.rollout.ObjectMeta +} // RolloutWorkload wraps an Argo Rollout. type RolloutWorkload struct { - rollout *argorolloutv1alpha1.Rollout - original *argorolloutv1alpha1.Rollout + *BaseWorkload[*argorolloutv1alpha1.Rollout] + strategyAnnotation string } // NewRolloutWorkload creates a new RolloutWorkload. -func NewRolloutWorkload(r *argorolloutv1alpha1.Rollout) *RolloutWorkload { +// The strategyAnnotation parameter specifies the annotation key used to determine +// the rollout strategy (from config.Annotations.RolloutStrategy). +func NewRolloutWorkload(r *argorolloutv1alpha1.Rollout, strategyAnnotation string) *RolloutWorkload { + original := r.DeepCopy() + accessor := &rolloutAccessor{rollout: r} return &RolloutWorkload{ - rollout: r, - original: r.DeepCopy(), + BaseWorkload: NewBaseWorkload(r, original, accessor, KindArgoRollout), + strategyAnnotation: strategyAnnotation, } } -// Ensure RolloutWorkload implements WorkloadAccessor. -var _ WorkloadAccessor = (*RolloutWorkload)(nil) - -func (w *RolloutWorkload) Kind() Kind { - return KindArgoRollout -} - -func (w *RolloutWorkload) GetObject() client.Object { - return w.rollout -} - -func (w *RolloutWorkload) GetName() string { - return w.rollout.Name -} - -func (w *RolloutWorkload) GetNamespace() string { - return w.rollout.Namespace -} - -func (w *RolloutWorkload) GetAnnotations() map[string]string { - return w.rollout.Annotations -} - -func (w *RolloutWorkload) GetPodTemplateAnnotations() map[string]string { - if w.rollout.Spec.Template.Annotations == nil { - w.rollout.Spec.Template.Annotations = make(map[string]string) - } - return w.rollout.Spec.Template.Annotations -} - -func (w *RolloutWorkload) SetPodTemplateAnnotation(key, value string) { - if w.rollout.Spec.Template.Annotations == nil { - w.rollout.Spec.Template.Annotations = make(map[string]string) - } - w.rollout.Spec.Template.Annotations[key] = value -} - -func (w *RolloutWorkload) GetContainers() []corev1.Container { - return w.rollout.Spec.Template.Spec.Containers -} - -func (w *RolloutWorkload) SetContainers(containers []corev1.Container) { - w.rollout.Spec.Template.Spec.Containers = containers -} - -func (w *RolloutWorkload) GetInitContainers() []corev1.Container { - return w.rollout.Spec.Template.Spec.InitContainers -} - -func (w *RolloutWorkload) SetInitContainers(containers []corev1.Container) { - w.rollout.Spec.Template.Spec.InitContainers = containers -} - -func (w *RolloutWorkload) GetVolumes() []corev1.Volume { - return w.rollout.Spec.Template.Spec.Volumes -} +// Ensure RolloutWorkload implements Workload. +var _ Workload = (*RolloutWorkload)(nil) // Update updates the Rollout. It uses the rollout strategy annotation to determine // whether to do a standard rollout or set the restartAt field. @@ -104,18 +64,18 @@ func (w *RolloutWorkload) Update(ctx context.Context, c client.Client) error { case RolloutStrategyRestart: // Set restartAt field to trigger a restart restartAt := metav1.NewTime(time.Now()) - w.rollout.Spec.RestartAt = &restartAt + w.Object().Spec.RestartAt = &restartAt } - return c.Patch(ctx, w.rollout, client.StrategicMergeFrom(w.original), client.FieldOwner(FieldManager)) + return c.Patch(ctx, w.Object(), client.StrategicMergeFrom(w.Original()), client.FieldOwner(FieldManager)) } // getStrategy returns the rollout strategy from the annotation. func (w *RolloutWorkload) getStrategy() RolloutStrategy { - annotations := w.rollout.GetAnnotations() + annotations := w.Object().GetAnnotations() if annotations == nil { return RolloutStrategyRollout } - strategy := annotations[RolloutStrategyAnnotation] + strategy := annotations[w.strategyAnnotation] switch RolloutStrategy(strategy) { case RolloutStrategyRestart: return RolloutStrategyRestart @@ -125,42 +85,12 @@ func (w *RolloutWorkload) getStrategy() RolloutStrategy { } func (w *RolloutWorkload) DeepCopy() Workload { - return &RolloutWorkload{ - rollout: w.rollout.DeepCopy(), - original: w.original.DeepCopy(), - } -} - -func (w *RolloutWorkload) ResetOriginal() { - w.original = w.rollout.DeepCopy() -} - -func (w *RolloutWorkload) GetEnvFromSources() []corev1.EnvFromSource { - var sources []corev1.EnvFromSource - for _, container := range w.rollout.Spec.Template.Spec.Containers { - sources = append(sources, container.EnvFrom...) - } - for _, container := range w.rollout.Spec.Template.Spec.InitContainers { - sources = append(sources, container.EnvFrom...) - } - return sources -} - -func (w *RolloutWorkload) UsesConfigMap(name string) bool { - return SpecUsesConfigMap(&w.rollout.Spec.Template.Spec, name) -} - -func (w *RolloutWorkload) UsesSecret(name string) bool { - return SpecUsesSecret(&w.rollout.Spec.Template.Spec, name) -} - -func (w *RolloutWorkload) GetOwnerReferences() []metav1.OwnerReference { - return w.rollout.OwnerReferences + return NewRolloutWorkload(w.Object().DeepCopy(), w.strategyAnnotation) } // GetRollout returns the underlying Rollout for special handling. func (w *RolloutWorkload) GetRollout() *argorolloutv1alpha1.Rollout { - return w.rollout + return w.Object() } // GetStrategy returns the configured rollout strategy. diff --git a/internal/pkg/workload/statefulset.go b/internal/pkg/workload/statefulset.go index ebec4a00..8e9d1e48 100644 --- a/internal/pkg/workload/statefulset.go +++ b/internal/pkg/workload/statefulset.go @@ -1,119 +1,46 @@ package workload import ( - "context" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) +// statefulSetAccessor implements PodTemplateAccessor for StatefulSet. +type statefulSetAccessor struct { + statefulset *appsv1.StatefulSet +} + +func (a *statefulSetAccessor) GetPodTemplateSpec() *corev1.PodTemplateSpec { + return &a.statefulset.Spec.Template +} + +func (a *statefulSetAccessor) GetObjectMeta() *metav1.ObjectMeta { + return &a.statefulset.ObjectMeta +} + // StatefulSetWorkload wraps a Kubernetes StatefulSet. type StatefulSetWorkload struct { - statefulset *appsv1.StatefulSet - original *appsv1.StatefulSet + *BaseWorkload[*appsv1.StatefulSet] } // NewStatefulSetWorkload creates a new StatefulSetWorkload. func NewStatefulSetWorkload(s *appsv1.StatefulSet) *StatefulSetWorkload { + original := s.DeepCopy() + accessor := &statefulSetAccessor{statefulset: s} return &StatefulSetWorkload{ - statefulset: s, - original: s.DeepCopy(), + BaseWorkload: NewBaseWorkload(s, original, accessor, KindStatefulSet), } } -// Ensure StatefulSetWorkload implements WorkloadAccessor. -var _ WorkloadAccessor = (*StatefulSetWorkload)(nil) - -func (w *StatefulSetWorkload) Kind() Kind { - return KindStatefulSet -} - -func (w *StatefulSetWorkload) GetObject() client.Object { - return w.statefulset -} - -func (w *StatefulSetWorkload) GetName() string { - return w.statefulset.Name -} - -func (w *StatefulSetWorkload) GetNamespace() string { - return w.statefulset.Namespace -} - -func (w *StatefulSetWorkload) GetAnnotations() map[string]string { - return w.statefulset.Annotations -} - -func (w *StatefulSetWorkload) GetPodTemplateAnnotations() map[string]string { - if w.statefulset.Spec.Template.Annotations == nil { - w.statefulset.Spec.Template.Annotations = make(map[string]string) - } - return w.statefulset.Spec.Template.Annotations -} - -func (w *StatefulSetWorkload) SetPodTemplateAnnotation(key, value string) { - if w.statefulset.Spec.Template.Annotations == nil { - w.statefulset.Spec.Template.Annotations = make(map[string]string) - } - w.statefulset.Spec.Template.Annotations[key] = value -} - -func (w *StatefulSetWorkload) GetContainers() []corev1.Container { - return w.statefulset.Spec.Template.Spec.Containers -} - -func (w *StatefulSetWorkload) SetContainers(containers []corev1.Container) { - w.statefulset.Spec.Template.Spec.Containers = containers -} - -func (w *StatefulSetWorkload) GetInitContainers() []corev1.Container { - return w.statefulset.Spec.Template.Spec.InitContainers -} - -func (w *StatefulSetWorkload) SetInitContainers(containers []corev1.Container) { - w.statefulset.Spec.Template.Spec.InitContainers = containers -} - -func (w *StatefulSetWorkload) GetVolumes() []corev1.Volume { - return w.statefulset.Spec.Template.Spec.Volumes -} - -func (w *StatefulSetWorkload) Update(ctx context.Context, c client.Client) error { - return c.Patch(ctx, w.statefulset, client.StrategicMergeFrom(w.original), client.FieldOwner(FieldManager)) -} +// Ensure StatefulSetWorkload implements Workload. +var _ Workload = (*StatefulSetWorkload)(nil) func (w *StatefulSetWorkload) DeepCopy() Workload { - return &StatefulSetWorkload{ - statefulset: w.statefulset.DeepCopy(), - original: w.original.DeepCopy(), - } + return NewStatefulSetWorkload(w.Object().DeepCopy()) } -func (w *StatefulSetWorkload) ResetOriginal() { - w.original = w.statefulset.DeepCopy() -} - -func (w *StatefulSetWorkload) GetEnvFromSources() []corev1.EnvFromSource { - var sources []corev1.EnvFromSource - for _, container := range w.statefulset.Spec.Template.Spec.Containers { - sources = append(sources, container.EnvFrom...) - } - for _, container := range w.statefulset.Spec.Template.Spec.InitContainers { - sources = append(sources, container.EnvFrom...) - } - return sources -} - -func (w *StatefulSetWorkload) UsesConfigMap(name string) bool { - return SpecUsesConfigMap(&w.statefulset.Spec.Template.Spec, name) -} - -func (w *StatefulSetWorkload) UsesSecret(name string) bool { - return SpecUsesSecret(&w.statefulset.Spec.Template.Spec, name) -} - -func (w *StatefulSetWorkload) GetOwnerReferences() []metav1.OwnerReference { - return w.statefulset.OwnerReferences +// GetStatefulSet returns the underlying StatefulSet for special handling. +func (w *StatefulSetWorkload) GetStatefulSet() *appsv1.StatefulSet { + return w.Object() } diff --git a/internal/pkg/workload/workload_test.go b/internal/pkg/workload/workload_test.go index 91139d85..084eb1e5 100644 --- a/internal/pkg/workload/workload_test.go +++ b/internal/pkg/workload/workload_test.go @@ -10,6 +10,9 @@ import ( "github.com/stakater/Reloader/internal/pkg/testutil" ) +// testRolloutStrategyAnnotation is the annotation key used in tests for rollout strategy. +const testRolloutStrategyAnnotation = "reloader.stakater.com/rollout-strategy" + // addEnvVar adds an environment variable with a ConfigMapKeyRef or SecretKeyRef to a container. func addEnvVarConfigMapRef(containers []corev1.Container, envName, configMapName, key string) { if len(containers) > 0 { @@ -763,10 +766,10 @@ func TestStatefulSetWorkload_GetOwnerReferences(t *testing.T) { // Test that workloads implement the interface func TestWorkloadInterface(t *testing.T) { - var _ WorkloadAccessor = (*DeploymentWorkload)(nil) - var _ WorkloadAccessor = (*DaemonSetWorkload)(nil) - var _ WorkloadAccessor = (*StatefulSetWorkload)(nil) - var _ WorkloadAccessor = (*RolloutWorkload)(nil) + var _ Workload = (*DeploymentWorkload)(nil) + var _ Workload = (*DaemonSetWorkload)(nil) + var _ Workload = (*StatefulSetWorkload)(nil) + var _ Workload = (*RolloutWorkload)(nil) } // RolloutWorkload tests @@ -781,7 +784,7 @@ func TestRolloutWorkload_BasicGetters(t *testing.T) { }, } - w := NewRolloutWorkload(rollout) + w := NewRolloutWorkload(rollout, testRolloutStrategyAnnotation) if w.Kind() != KindArgoRollout { t.Errorf("Kind() = %v, want %v", w.Kind(), KindArgoRollout) @@ -814,7 +817,7 @@ func TestRolloutWorkload_PodTemplateAnnotations(t *testing.T) { }, } - w := NewRolloutWorkload(rollout) + w := NewRolloutWorkload(rollout, testRolloutStrategyAnnotation) // Test get annotations := w.GetPodTemplateAnnotations() @@ -834,7 +837,7 @@ func TestRolloutWorkload_GetStrategy_Default(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "test"}, } - w := NewRolloutWorkload(rollout) + w := NewRolloutWorkload(rollout, testRolloutStrategyAnnotation) if w.GetStrategy() != RolloutStrategyRollout { t.Errorf("GetStrategy() = %v, want %v (default)", w.GetStrategy(), RolloutStrategyRollout) @@ -846,12 +849,12 @@ func TestRolloutWorkload_GetStrategy_Restart(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "test", Annotations: map[string]string{ - RolloutStrategyAnnotation: "restart", + testRolloutStrategyAnnotation: "restart", }, }, } - w := NewRolloutWorkload(rollout) + w := NewRolloutWorkload(rollout, testRolloutStrategyAnnotation) if w.GetStrategy() != RolloutStrategyRestart { t.Errorf("GetStrategy() = %v, want %v", w.GetStrategy(), RolloutStrategyRestart) @@ -881,7 +884,7 @@ func TestRolloutWorkload_UsesConfigMap_Volume(t *testing.T) { }, } - w := NewRolloutWorkload(rollout) + w := NewRolloutWorkload(rollout, testRolloutStrategyAnnotation) if !w.UsesConfigMap("rollout-config") { t.Error("Rollout UsesConfigMap should return true for ConfigMap volume") @@ -916,7 +919,7 @@ func TestRolloutWorkload_UsesSecret_EnvFrom(t *testing.T) { }, } - w := NewRolloutWorkload(rollout) + w := NewRolloutWorkload(rollout, testRolloutStrategyAnnotation) if !w.UsesSecret("rollout-secret") { t.Error("Rollout UsesSecret should return true for Secret envFrom") @@ -940,7 +943,7 @@ func TestRolloutWorkload_DeepCopy(t *testing.T) { }, } - w := NewRolloutWorkload(rollout) + w := NewRolloutWorkload(rollout, testRolloutStrategyAnnotation) copy := w.DeepCopy() // Verify copy is independent @@ -1176,8 +1179,8 @@ func TestCronJobWorkload_DeepCopy(t *testing.T) { // Test that Job and CronJob implement the interface func TestJobCronJobWorkloadInterface(t *testing.T) { - var _ WorkloadAccessor = (*JobWorkload)(nil) - var _ WorkloadAccessor = (*CronJobWorkload)(nil) + var _ Workload = (*JobWorkload)(nil) + var _ Workload = (*CronJobWorkload)(nil) } // DeploymentConfig tests @@ -1538,5 +1541,228 @@ func TestDeploymentConfigWorkload_GetDeploymentConfig(t *testing.T) { // Test that DeploymentConfig implements the interface func TestDeploymentConfigWorkloadInterface(t *testing.T) { - var _ WorkloadAccessor = (*DeploymentConfigWorkload)(nil) + var _ Workload = (*DeploymentConfigWorkload)(nil) +} + +// Tests for UpdateStrategy +func TestWorkload_UpdateStrategy(t *testing.T) { + tests := []struct { + name string + workload Workload + expected UpdateStrategy + }{ + { + name: "Deployment uses Patch strategy", + workload: NewDeploymentWorkload(testutil.NewDeployment("test", "default", nil)), + expected: UpdateStrategyPatch, + }, + { + name: "DaemonSet uses Patch strategy", + workload: NewDaemonSetWorkload(testutil.NewDaemonSet("test", "default", nil)), + expected: UpdateStrategyPatch, + }, + { + name: "StatefulSet uses Patch strategy", + workload: NewStatefulSetWorkload(testutil.NewStatefulSet("test", "default", nil)), + expected: UpdateStrategyPatch, + }, + { + name: "Job uses Recreate strategy", + workload: NewJobWorkload(testutil.NewJob("test", "default")), + expected: UpdateStrategyRecreate, + }, + { + name: "CronJob uses CreateNew strategy", + workload: NewCronJobWorkload(testutil.NewCronJob("test", "default")), + expected: UpdateStrategyCreateNew, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.workload.UpdateStrategy(); got != tt.expected { + t.Errorf("UpdateStrategy() = %v, want %v", got, tt.expected) + } + }) + } +} + +// Tests for ResetOriginal +func TestDeploymentWorkload_ResetOriginal(t *testing.T) { + deploy := testutil.NewDeployment("test", "default", nil) + w := NewDeploymentWorkload(deploy) + + // Modify the workload + w.SetPodTemplateAnnotation("modified", "true") + + // Original should still not have the annotation + originalAnnotations := w.Original().Spec.Template.Annotations + if originalAnnotations != nil && originalAnnotations["modified"] == "true" { + t.Error("Original should not be modified yet") + } + + // Reset original + w.ResetOriginal() + + // Now original should have the annotation + if w.Original().Spec.Template.Annotations["modified"] != "true" { + t.Error("ResetOriginal should update original to match current state") + } +} + +func TestJobWorkload_ResetOriginal(t *testing.T) { + job := testutil.NewJob("test", "default") + w := NewJobWorkload(job) + + // ResetOriginal should be a no-op for Jobs (they don't use strategic merge patch) + w.SetPodTemplateAnnotation("modified", "true") + w.ResetOriginal() // Should not panic or error +} + +func TestCronJobWorkload_ResetOriginal(t *testing.T) { + cj := testutil.NewCronJob("test", "default") + w := NewCronJobWorkload(cj) + + // ResetOriginal should be a no-op for CronJobs + w.SetPodTemplateAnnotation("modified", "true") + w.ResetOriginal() // Should not panic or error +} + +// Tests for BaseWorkload.Original() +func TestDeploymentWorkload_Original(t *testing.T) { + deploy := testutil.NewDeployment("test", "default", nil) + deploy.Spec.Template.Annotations = map[string]string{"initial": "value"} + + w := NewDeploymentWorkload(deploy) + + // Modify the current object + w.SetPodTemplateAnnotation("new", "annotation") + + // Original should still have only the initial annotation + original := w.Original() + if original.Spec.Template.Annotations["new"] == "annotation" { + t.Error("Original should not reflect changes to current object") + } + if original.Spec.Template.Annotations["initial"] != "value" { + t.Error("Original should retain initial state") + } +} + +// Tests for PerformSpecialUpdate returning false for standard workloads +func TestDeploymentWorkload_PerformSpecialUpdate(t *testing.T) { + deploy := testutil.NewDeployment("test", "default", nil) + w := NewDeploymentWorkload(deploy) + + updated, err := w.PerformSpecialUpdate(t.Context(), nil) + if err != nil { + t.Errorf("PerformSpecialUpdate() error = %v", err) + } + if updated { + t.Error("PerformSpecialUpdate() should return false for Deployment") + } +} + +func TestDaemonSetWorkload_PerformSpecialUpdate(t *testing.T) { + ds := testutil.NewDaemonSet("test", "default", nil) + w := NewDaemonSetWorkload(ds) + + updated, err := w.PerformSpecialUpdate(t.Context(), nil) + if err != nil { + t.Errorf("PerformSpecialUpdate() error = %v", err) + } + if updated { + t.Error("PerformSpecialUpdate() should return false for DaemonSet") + } +} + +func TestStatefulSetWorkload_PerformSpecialUpdate(t *testing.T) { + ss := testutil.NewStatefulSet("test", "default", nil) + w := NewStatefulSetWorkload(ss) + + updated, err := w.PerformSpecialUpdate(t.Context(), nil) + if err != nil { + t.Errorf("PerformSpecialUpdate() error = %v", err) + } + if updated { + t.Error("PerformSpecialUpdate() should return false for StatefulSet") + } +} + +// Test Update returns nil for Job (no-op, uses PerformSpecialUpdate instead) +func TestJobWorkload_Update(t *testing.T) { + job := testutil.NewJob("test", "default") + w := NewJobWorkload(job) + + err := w.Update(t.Context(), nil) + if err != nil { + t.Errorf("Update() should return nil for Job, got %v", err) + } +} + +// Test Update returns nil for CronJob (no-op, uses PerformSpecialUpdate instead) +func TestCronJobWorkload_Update(t *testing.T) { + cj := testutil.NewCronJob("test", "default") + w := NewCronJobWorkload(cj) + + err := w.Update(t.Context(), nil) + if err != nil { + t.Errorf("Update() should return nil for CronJob, got %v", err) + } +} + +// Test GetJob and GetCronJob accessors +func TestJobWorkload_GetJob(t *testing.T) { + job := testutil.NewJob("test", "default") + w := NewJobWorkload(job) + + if w.GetJob() != job { + t.Error("GetJob should return the underlying Job") + } +} + +func TestCronJobWorkload_GetCronJob(t *testing.T) { + cj := testutil.NewCronJob("test", "default") + w := NewCronJobWorkload(cj) + + if w.GetCronJob() != cj { + t.Error("GetCronJob should return the underlying CronJob") + } +} + +func TestDeploymentWorkload_GetDeployment(t *testing.T) { + deploy := testutil.NewDeployment("test", "default", nil) + w := NewDeploymentWorkload(deploy) + + if w.GetDeployment() != deploy { + t.Error("GetDeployment should return the underlying Deployment") + } +} + +func TestDaemonSetWorkload_GetDaemonSet(t *testing.T) { + ds := testutil.NewDaemonSet("test", "default", nil) + w := NewDaemonSetWorkload(ds) + + if w.GetDaemonSet() != ds { + t.Error("GetDaemonSet should return the underlying DaemonSet") + } +} + +func TestStatefulSetWorkload_GetStatefulSet(t *testing.T) { + ss := testutil.NewStatefulSet("test", "default", nil) + w := NewStatefulSetWorkload(ss) + + if w.GetStatefulSet() != ss { + t.Error("GetStatefulSet should return the underlying StatefulSet") + } +} + +func TestRolloutWorkload_GetRollout(t *testing.T) { + rollout := &argorolloutv1alpha1.Rollout{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + } + w := NewRolloutWorkload(rollout, testRolloutStrategyAnnotation) + + if w.GetRollout() != rollout { + t.Error("GetRollout should return the underlying Rollout") + } }