fix: A bunch of issues surfaced up via e2e tests and remove old e2e ones

This commit is contained in:
TheiLLeniumStudios
2026-01-07 17:32:44 +01:00
parent 5548ce559a
commit bd030fe795
16 changed files with 171 additions and 1684 deletions
+1 -1
View File
@@ -121,7 +121,7 @@ func NewDefault() *Config {
LogFormat: "",
LogLevel: "info",
MetricsAddr: ":9090",
HealthAddr: ":8081",
HealthAddr: ":8080",
EnablePProf: false,
PProfAddr: ":6060",
Alerting: AlertingConfig{},
+2 -2
View File
@@ -48,8 +48,8 @@ func TestNewDefault(t *testing.T) {
t.Errorf("MetricsAddr = %q, want %q", cfg.MetricsAddr, ":9090")
}
if cfg.HealthAddr != ":8081" {
t.Errorf("HealthAddr = %q, want %q", cfg.HealthAddr, ":8081")
if cfg.HealthAddr != ":8080" {
t.Errorf("HealthAddr = %q, want %q", cfg.HealthAddr, ":8080")
}
if cfg.PProfAddr != ":6060" {
+35 -17
View File
@@ -1,11 +1,13 @@
package config
import (
"fmt"
"strings"
"time"
"github.com/spf13/pflag"
"github.com/spf13/viper"
"k8s.io/apimachinery/pkg/labels"
)
// v is the viper instance for configuration.
@@ -112,13 +114,13 @@ func BindFlags(fs *pflag.FlagSet, cfg *Config) {
)
// Filtering - selectors
fs.String(
"namespace-selector", "",
"Comma-separated list of namespace label selectors",
fs.StringSlice(
"namespace-selector", nil,
"Namespace label selectors (can be specified multiple times)",
)
fs.String(
"resource-label-selector", "",
"Comma-separated list of resource label selectors",
fs.StringSlice(
"resource-label-selector", nil,
"Resource label selectors (can be specified multiple times)",
)
// Logging
@@ -262,6 +264,9 @@ func ApplyFlags(cfg *Config) error {
cfg.HealthAddr = v.GetString("health-addr")
cfg.PProfAddr = v.GetString("pprof-addr")
cfg.WatchedNamespace = v.GetString("watch-namespace")
if cfg.WatchedNamespace == "" {
cfg.WatchedNamespace = v.GetString("KUBERNETES_NAMESPACE")
}
// Leader election
cfg.LeaderElection.LockName = v.GetString("leader-election-id")
@@ -300,19 +305,32 @@ func ApplyFlags(cfg *Config) error {
cfg.IgnoredWorkloads = splitAndTrim(v.GetString("ignored-workload-types"))
cfg.IgnoredNamespaces = splitAndTrim(v.GetString("namespaces-to-ignore"))
// Store raw selector strings
cfg.NamespaceSelectorStrings = splitAndTrim(v.GetString("namespace-selector"))
cfg.ResourceSelectorStrings = splitAndTrim(v.GetString("resource-label-selector"))
// Get selector slices and join with comma
nsSelectors := v.GetStringSlice("namespace-selector")
resSelectors := v.GetStringSlice("resource-label-selector")
// Parse selectors into labels.Selector
var err error
cfg.NamespaceSelectors, err = ParseSelectors(cfg.NamespaceSelectorStrings)
if err != nil {
return err
if len(nsSelectors) > 0 {
cfg.NamespaceSelectorStrings = nsSelectors
}
cfg.ResourceSelectors, err = ParseSelectors(cfg.ResourceSelectorStrings)
if err != nil {
return err
if len(resSelectors) > 0 {
cfg.ResourceSelectorStrings = resSelectors
}
if len(nsSelectors) > 0 {
joinedNS := strings.Join(nsSelectors, ",")
selector, err := labels.Parse(joinedNS)
if err != nil {
return fmt.Errorf("invalid selector %q: %w", joinedNS, err)
}
cfg.NamespaceSelectors = []labels.Selector{selector}
}
if len(resSelectors) > 0 {
joinedRes := strings.Join(resSelectors, ",")
selector, err := labels.Parse(joinedRes)
if err != nil {
return fmt.Errorf("invalid selector %q: %w", joinedRes, err)
}
cfg.ResourceSelectors = []labels.Selector{selector}
}
// Ensure duration defaults are preserved if not set
+2 -2
View File
@@ -249,8 +249,8 @@ func TestApplyFlags_Selectors(t *testing.T) {
t.Fatalf("ApplyFlags() error = %v", err)
}
if len(cfg.NamespaceSelectors) != 2 {
t.Errorf("NamespaceSelectors length = %d, want 2", len(cfg.NamespaceSelectors))
if len(cfg.NamespaceSelectors) != 1 {
t.Errorf("NamespaceSelectors length = %d, want 1", len(cfg.NamespaceSelectors))
}
if len(cfg.ResourceSelectors) != 1 {
+7 -1
View File
@@ -98,17 +98,23 @@ func (c *Config) Validate() error {
c.IgnoredResources = normalizeToLower(c.IgnoredResources)
// Normalize ignored workloads to canonical Kind values (e.g., "cronjobs" -> "CronJob")
c.IgnoredWorkloads = normalizeToLower(c.IgnoredWorkloads)
normalizedWorkloads := make([]string, 0, len(c.IgnoredWorkloads))
for _, w := range c.IgnoredWorkloads {
if _, err := workload.KindFromString(w); err != nil {
kind, err := workload.KindFromString(w)
if err != nil {
errs = append(
errs, ValidationError{
Field: "IgnoredWorkloads",
Message: fmt.Sprintf("unknown workload type %q", w),
},
)
} else {
normalizedWorkloads = append(normalizedWorkloads, string(kind))
}
}
c.IgnoredWorkloads = normalizedWorkloads
if len(errs) > 0 {
return errs
+2 -1
View File
@@ -166,7 +166,8 @@ func TestConfig_Validate_NormalizesIgnoredWorkloads(t *testing.T) {
t.Fatalf("Validate() error = %v", err)
}
expected := []string{"jobs", "cronjobs"}
// Should be normalized to canonical Kind values (e.g., "CronJob" not "cronjobs")
expected := []string{"Job", "CronJob"}
if len(cfg.IgnoredWorkloads) != len(expected) {
t.Fatalf("IgnoredWorkloads length = %d, want %d", len(cfg.IgnoredWorkloads), len(expected))
}
+12 -10
View File
@@ -31,19 +31,21 @@ func NewConfigMapReconciler(
webhookClient *webhook.Client,
alerter alerting.Alerter,
pauseHandler *reload.PauseHandler,
nsCache *NamespaceCache,
) *ConfigMapReconciler {
return NewResourceReconciler(
ResourceReconcilerDeps{
Client: c,
Log: log,
Config: cfg,
ReloadService: reloadService,
Registry: registry,
Collectors: collectors,
EventRecorder: eventRecorder,
WebhookClient: webhookClient,
Alerter: alerter,
PauseHandler: pauseHandler,
Client: c,
Log: log,
Config: cfg,
ReloadService: reloadService,
Registry: registry,
Collectors: collectors,
EventRecorder: eventRecorder,
WebhookClient: webhookClient,
Alerter: alerter,
PauseHandler: pauseHandler,
NamespaceCache: nsCache,
},
ResourceConfig[*corev1.ConfigMap]{
ResourceType: reload.ResourceTypeConfigMap,
+36 -14
View File
@@ -19,6 +19,7 @@ import (
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/healthz"
ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics/server"
)
@@ -73,6 +74,15 @@ func NewManager(opts ManagerOptions) (ctrl.Manager, error) {
RetryPeriod: &le.RetryPeriod,
}
if cfg.WatchedNamespace != "" {
mgrOpts.Cache = cache.Options{
DefaultNamespaces: map[string]cache.Config{
cfg.WatchedNamespace: {},
},
}
opts.Log.Info("namespace filtering enabled", "namespace", cfg.WatchedNamespace)
}
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOpts)
if err != nil {
return nil, fmt.Errorf("creating manager: %w", err)
@@ -115,6 +125,14 @@ func NewManagerWithRestConfig(opts ManagerOptions, restConfig *rest.Config) (ctr
RetryPeriod: &le.RetryPeriod,
}
if cfg.WatchedNamespace != "" {
mgrOpts.Cache = cache.Options{
DefaultNamespaces: map[string]cache.Config{
cfg.WatchedNamespace: {},
},
}
}
mgr, err := ctrl.NewManager(restConfig, mgrOpts)
if err != nil {
return nil, fmt.Errorf("creating manager: %w", err)
@@ -149,6 +167,22 @@ func SetupReconcilers(mgr ctrl.Manager, cfg *config.Config, log logr.Logger, col
log.Info("webhook mode enabled", "url", cfg.WebhookURL)
}
// Create namespace cache if namespace selectors are configured.
// This cache is shared between the namespace reconciler and resource reconcilers.
var nsCache *NamespaceCache
if len(cfg.NamespaceSelectors) > 0 {
nsCache = NewNamespaceCache(true)
if err := (&NamespaceReconciler{
Client: mgr.GetClient(),
Log: log.WithName("namespace-reconciler"),
Config: cfg,
Cache: nsCache,
}).SetupWithManager(mgr); err != nil {
return fmt.Errorf("setting up namespace reconciler: %w", err)
}
log.Info("namespace reconciler enabled for label selector filtering")
}
// Setup ConfigMap reconciler
if !cfg.IsResourceIgnored("configmaps") {
cmReconciler := NewConfigMapReconciler(
@@ -162,6 +196,7 @@ func SetupReconcilers(mgr ctrl.Manager, cfg *config.Config, log logr.Logger, col
webhookClient,
alerter,
pauseHandler,
nsCache,
)
if err := SetupConfigMapReconciler(mgr, cmReconciler); err != nil {
return fmt.Errorf("setting up configmap reconciler: %w", err)
@@ -181,26 +216,13 @@ func SetupReconcilers(mgr ctrl.Manager, cfg *config.Config, log logr.Logger, col
webhookClient,
alerter,
pauseHandler,
nsCache,
)
if err := SetupSecretReconciler(mgr, secretReconciler); err != nil {
return fmt.Errorf("setting up secret reconciler: %w", err)
}
}
// Setup Namespace reconciler if namespace selectors are configured
if len(cfg.NamespaceSelectors) > 0 {
nsCache := NewNamespaceCache(true)
if err := (&NamespaceReconciler{
Client: mgr.GetClient(),
Log: log.WithName("namespace-reconciler"),
Config: cfg,
Cache: nsCache,
}).SetupWithManager(mgr); err != nil {
return fmt.Errorf("setting up namespace reconciler: %w", err)
}
log.Info("namespace reconciler enabled for label selector filtering")
}
// Setup Deployment reconciler for pause handling
if err := (&DeploymentReconciler{
Client: mgr.GetClient(),
+38 -22
View File
@@ -21,16 +21,17 @@ import (
// 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
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
NamespaceCache *NamespaceCache
}
// ResourceConfig provides type-specific configuration for a resource reconciler.
@@ -75,10 +76,12 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req ctrl.Request)
resourceType := string(r.ResourceType)
log := r.Log.WithValues(resourceType, req.NamespacedName)
r.initOnce.Do(func() {
r.initialized = true
log.Info(resourceType + " controller initialized")
})
r.initOnce.Do(
func() {
r.initialized = true
log.Info(resourceType + " controller initialized")
},
)
r.Collectors.RecordEventReceived("reconcile", resourceType)
@@ -101,10 +104,19 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, nil
}
result, err := r.reloadHandler().Process(ctx, req.Namespace, req.Name, r.ResourceType,
if r.NamespaceCache != nil && r.NamespaceCache.IsEnabled() && !r.NamespaceCache.Contains(namespace) {
log.V(1).Info("skipping "+resourceType+" in namespace not matching selector", "namespace", namespace)
r.Collectors.RecordSkipped("namespace_selector")
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)
}, log,
)
r.recordReconcile(startTime, err)
return result, err
@@ -139,10 +151,12 @@ func (r *ResourceReconciler[T]) handleDelete(
resource.SetName(req.Name)
resource.SetNamespace(req.Namespace)
return r.reloadHandler().Process(ctx, req.Namespace, req.Name, r.ResourceType,
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)
}, log,
)
}
func (r *ResourceReconciler[T]) recordReconcile(startTime time.Time, err error) {
@@ -178,9 +192,11 @@ func (r *ResourceReconciler[T]) Initialized() *bool {
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(),
)).
WithEventFilter(
BuildEventFilter(
r.CreatePredicates(r.Config, r.ReloadService.Hasher()),
r.Config, r.Initialized(),
),
).
Complete(r)
}
+12 -10
View File
@@ -31,19 +31,21 @@ func NewSecretReconciler(
webhookClient *webhook.Client,
alerter alerting.Alerter,
pauseHandler *reload.PauseHandler,
nsCache *NamespaceCache,
) *SecretReconciler {
return NewResourceReconciler(
ResourceReconcilerDeps{
Client: c,
Log: log,
Config: cfg,
ReloadService: reloadService,
Registry: registry,
Collectors: collectors,
EventRecorder: eventRecorder,
WebhookClient: webhookClient,
Alerter: alerter,
PauseHandler: pauseHandler,
Client: c,
Log: log,
Config: cfg,
ReloadService: reloadService,
Registry: registry,
Collectors: collectors,
EventRecorder: eventRecorder,
WebhookClient: webhookClient,
Alerter: alerter,
PauseHandler: pauseHandler,
NamespaceCache: nsCache,
},
ResourceConfig[*corev1.Secret]{
ResourceType: reload.ResourceTypeSecret,
+9 -5
View File
@@ -47,11 +47,13 @@ func newTestDeps(t *testing.T, cfg *config.Config, objects ...runtime.Object) te
log: log,
cfg: cfg,
reloadService: reload.NewService(cfg, log),
registry: workload.NewRegistry(workload.RegistryOptions{
ArgoRolloutsEnabled: cfg.ArgoRolloutsEnabled,
DeploymentConfigEnabled: cfg.DeploymentConfigEnabled,
RolloutStrategyAnnotation: cfg.Annotations.RolloutStrategy,
}),
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),
@@ -74,6 +76,7 @@ func newConfigMapReconciler(t *testing.T, cfg *config.Config, objects ...runtime
deps.webhookClient,
deps.alerter,
nil,
nil,
)
}
@@ -92,6 +95,7 @@ func newSecretReconciler(t *testing.T, cfg *config.Config, objects ...runtime.Ob
deps.webhookClient,
deps.alerter,
nil,
nil,
)
}
+1 -1
View File
@@ -66,7 +66,7 @@ func (w *RolloutWorkload) Update(ctx context.Context, c client.Client) error {
restartAt := metav1.NewTime(time.Now())
w.Object().Spec.RestartAt = &restartAt
}
return c.Patch(ctx, w.Object(), client.StrategicMergeFrom(w.Original()), client.FieldOwner(FieldManager))
return c.Patch(ctx, w.Object(), client.MergeFrom(w.Original()), client.FieldOwner(FieldManager))
}
// getStrategy returns the rollout strategy from the annotation.