From 7f9f32ca58afa80e358311b9579fcc5d438f0dc7 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Wed, 14 Sep 2022 15:01:57 +0100 Subject: [PATCH 01/16] Add leadership election --- internal/pkg/cmd/reloader.go | 116 ++++++++++++++++++++++++-- internal/pkg/controller/controller.go | 11 ++- internal/pkg/handler/create.go | 9 +- internal/pkg/handler/handler.go | 2 +- internal/pkg/handler/update.go | 9 +- internal/pkg/options/flags.go | 2 + 6 files changed, 137 insertions(+), 12 deletions(-) diff --git a/internal/pkg/cmd/reloader.go b/internal/pkg/cmd/reloader.go index cbce531b..de4f1190 100644 --- a/internal/pkg/cmd/reloader.go +++ b/internal/pkg/cmd/reloader.go @@ -1,11 +1,14 @@ package cmd import ( + "context" "errors" "fmt" - "github.com/stakater/Reloader/internal/pkg/constants" "os" "strings" + "time" + + "github.com/stakater/Reloader/internal/pkg/constants" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -15,6 +18,15 @@ import ( "github.com/stakater/Reloader/internal/pkg/util" "github.com/stakater/Reloader/pkg/kube" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/leaderelection" + "k8s.io/client-go/tools/leaderelection/resourcelock" +) + +const ( + lockName string = "stakaer-reloader-lock" + podNameEnv string = "POD_NAME" + podNamespaceEnv string = "POD_NAMESPACE" ) // NewReloaderCommand starts the reloader controller @@ -38,21 +50,34 @@ func NewReloaderCommand() *cobra.Command { cmd.PersistentFlags().StringVar(&options.IsArgoRollouts, "is-Argo-Rollouts", "false", "Add support for argo rollouts") cmd.PersistentFlags().StringVar(&options.ReloadStrategy, constants.ReloadStrategyFlag, constants.EnvVarsReloadStrategy, "Specifies the desired reload strategy") cmd.PersistentFlags().StringVar(&options.ReloadOnCreate, "reload-on-create", "false", "Add support to watch create events") + cmd.PersistentFlags().BoolVar(&options.EnableHA, "enable-ha", false, "Adds support for running multiple replicas via leadership election") return cmd } func validateFlags(*cobra.Command, []string) error { // Ensure the reload strategy is one of the following... + var validReloadStrategy bool valid := []string{constants.EnvVarsReloadStrategy, constants.AnnotationsReloadStrategy} for _, s := range valid { if s == options.ReloadStrategy { - return nil + validReloadStrategy = true } } - err := fmt.Sprintf("%s must be one of: %s", constants.ReloadStrategyFlag, strings.Join(valid, ", ")) - return errors.New(err) + if !validReloadStrategy { + err := fmt.Sprintf("%s must be one of: %s", constants.ReloadStrategyFlag, strings.Join(valid, ", ")) + return errors.New(err) + } + + // Validate that HA options are correct + if options.EnableHA { + if _, _, err := validateHAEnvs(); err != nil { + return err + } + } + + return nil } func configureLogging(logFormat string) error { @@ -68,6 +93,25 @@ func configureLogging(logFormat string) error { return nil } +func validateHAEnvs() (string, string, error) { + podName, podNamespace := getHAEnvs() + + if podName == "" { + return podName, podNamespace, fmt.Errorf("%s not set, cannot run in HA mode without %s set", podNameEnv, podNameEnv) + } + if podNamespace == "" { + return podName, podNamespace, fmt.Errorf("%s not set, cannot run in HA mode without %s set", podNamespaceEnv, podNamespaceEnv) + } + return podName, podNamespace, nil +} + +func getHAEnvs() (string, string) { + podName := os.Getenv(podNameEnv) + podNamespace := os.Getenv(podNamespaceEnv) + + return podName, podNamespace +} + func startReloader(cmd *cobra.Command, args []string) { err := configureLogging(options.LogFormat) if err != nil { @@ -99,6 +143,7 @@ func startReloader(cmd *cobra.Command, args []string) { collectors := metrics.SetupPrometheusEndpoint() + var controllers []*controller.Controller for k := range kube.ResourceMap { if ignoredResourcesList.Contains(k) { continue @@ -109,6 +154,13 @@ func startReloader(cmd *cobra.Command, args []string) { logrus.Fatalf("%s", err) } + // If HA is enabled then we need to run leadership election + if options.EnableHA { + c.SetLeader(false) + } + + controllers = append(controllers, c) + // Now let's start the controller stop := make(chan struct{}) defer close(stop) @@ -116,10 +168,64 @@ func startReloader(cmd *cobra.Command, args []string) { go c.Run(1, stop) } - // Wait forever + // Run the leadership election + if options.EnableHA { + podName, podNamespace := getHAEnvs() + lock := getNewLock(clientset, lockName, podName, podNamespace) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + runLeaderElection(lock, ctx, podName, controllers) + } + select {} } +func getNewLock(clientset *kubernetes.Clientset, lockName, podname, namespace string) *resourcelock.LeaseLock { + return &resourcelock.LeaseLock{ + LeaseMeta: v1.ObjectMeta{ + Name: lockName, + Namespace: namespace, + }, + Client: clientset.CoordinationV1(), + LockConfig: resourcelock.ResourceLockConfig{ + Identity: podname, + }, + } +} + +func runLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, id string, controllers []*controller.Controller) { + leaderelection.RunOrDie(ctx, leaderelection.LeaderElectionConfig{ + Lock: lock, + ReleaseOnCancel: true, + // TODO Validate that keys persist in the cache for at least one leadership election cycle + LeaseDuration: 15 * time.Second, + RenewDeadline: 10 * time.Second, + RetryPeriod: 2 * time.Second, + Callbacks: leaderelection.LeaderCallbacks{ + OnStartedLeading: func(c context.Context) { + setLeader(controllers, true) + }, + OnStoppedLeading: func() { + setLeader(controllers, false) + }, + OnNewLeader: func(current_id string) { + if current_id == id { + //klog.Info("still the leader!") + return + } + //klog.Info("new leader is %s", current_id) + }, + }, + }) +} + +func setLeader(controllers []*controller.Controller, isLeader bool) { + for _, c := range controllers { + c := c + c.SetLeader(isLeader) + } +} + func getIgnoredNamespacesList(cmd *cobra.Command) (util.List, error) { return getStringSliceFromFlags(cmd, "namespaces-to-ignore") } diff --git a/internal/pkg/controller/controller.go b/internal/pkg/controller/controller.go index 1b2e13b2..bd21b604 100644 --- a/internal/pkg/controller/controller.go +++ b/internal/pkg/controller/controller.go @@ -2,9 +2,10 @@ package controller import ( "fmt" - "github.com/stakater/Reloader/internal/pkg/options" "time" + "github.com/stakater/Reloader/internal/pkg/options" + "github.com/sirupsen/logrus" "github.com/stakater/Reloader/internal/pkg/handler" "github.com/stakater/Reloader/internal/pkg/metrics" @@ -29,6 +30,7 @@ type Controller struct { namespace string ignoredNamespaces util.List collectors metrics.Collectors + isLeader bool } // controllerInitialized flag determines whether controlled is being initialized @@ -56,6 +58,7 @@ func NewController( c.informer = informer c.queue = queue c.collectors = collectors + c.isLeader = true return &c, nil } @@ -140,7 +143,7 @@ func (c *Controller) processNextItem() bool { defer c.queue.Done(resourceHandler) // Invoke the method containing the business logic - err := resourceHandler.(handler.ResourceHandler).Handle() + err := resourceHandler.(handler.ResourceHandler).Handle(c.isLeader) // Handle the error if something went wrong during the execution of the business logic c.handleErr(err, resourceHandler) return true @@ -171,3 +174,7 @@ func (c *Controller) handleErr(err error, key interface{}) { runtime.HandleError(err) logrus.Infof("Dropping the key %q out of the queue: %v", key, err) } + +func (c *Controller) SetLeader(isLeader bool) { + c.isLeader = isLeader +} diff --git a/internal/pkg/handler/create.go b/internal/pkg/handler/create.go index f6364c5c..da3c75e0 100644 --- a/internal/pkg/handler/create.go +++ b/internal/pkg/handler/create.go @@ -1,6 +1,8 @@ package handler import ( + "fmt" + "github.com/sirupsen/logrus" "github.com/stakater/Reloader/internal/pkg/metrics" "github.com/stakater/Reloader/internal/pkg/util" @@ -14,13 +16,16 @@ type ResourceCreatedHandler struct { } // Handle processes the newly created resource -func (r ResourceCreatedHandler) Handle() error { +func (r ResourceCreatedHandler) Handle(isLeader bool) error { if r.Resource == nil { logrus.Errorf("Resource creation handler received nil resource") } else { config, _ := r.GetConfig() // process resource based on its type - return doRollingUpgrade(config, r.Collectors) + if isLeader { + return doRollingUpgrade(config, r.Collectors) + } + return fmt.Errorf("instance is not leader, will not perform rolling upgrade on %s %s/%s", config.Type, config.ResourceName, config.Namespace) } return nil } diff --git a/internal/pkg/handler/handler.go b/internal/pkg/handler/handler.go index 634e080b..4dbf10e2 100644 --- a/internal/pkg/handler/handler.go +++ b/internal/pkg/handler/handler.go @@ -6,6 +6,6 @@ import ( // ResourceHandler handles the creation and update of resources type ResourceHandler interface { - Handle() error + Handle(isLeader bool) error GetConfig() (util.Config, string) } diff --git a/internal/pkg/handler/update.go b/internal/pkg/handler/update.go index 4854151c..77da76e0 100644 --- a/internal/pkg/handler/update.go +++ b/internal/pkg/handler/update.go @@ -1,6 +1,8 @@ package handler import ( + "fmt" + "github.com/sirupsen/logrus" "github.com/stakater/Reloader/internal/pkg/metrics" "github.com/stakater/Reloader/internal/pkg/util" @@ -15,14 +17,17 @@ type ResourceUpdatedHandler struct { } // Handle processes the updated resource -func (r ResourceUpdatedHandler) Handle() error { +func (r ResourceUpdatedHandler) Handle(isLeader bool) error { if r.Resource == nil || r.OldResource == nil { logrus.Errorf("Resource update handler received nil resource") } else { config, oldSHAData := r.GetConfig() if config.SHAValue != oldSHAData { // process resource based on its type - return doRollingUpgrade(config, r.Collectors) + if isLeader { + return doRollingUpgrade(config, r.Collectors) + } + return fmt.Errorf("instance is not leader, will not perform rolling upgrade on %s %s/%s", config.Type, config.ResourceName, config.Namespace) } } return nil diff --git a/internal/pkg/options/flags.go b/internal/pkg/options/flags.go index 097e3c06..d8d51643 100644 --- a/internal/pkg/options/flags.go +++ b/internal/pkg/options/flags.go @@ -25,4 +25,6 @@ var ( ReloadStrategy = constants.EnvVarsReloadStrategy // ReloadOnCreate Adds support to watch create events ReloadOnCreate = "false" + // EnableHA adds support for running multiple replicas via leadership election + EnableHA = false ) From 401d4227d129b4cd1ae28ac12aabb30d265e8f46 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Wed, 14 Sep 2022 16:37:21 +0100 Subject: [PATCH 02/16] Move consts to const pkg Should move leadership bits to own pkg? --- internal/pkg/cmd/reloader.go | 28 ++++++++++++--------------- internal/pkg/constants/constants.go | 7 +++++++ internal/pkg/controller/controller.go | 1 + 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/internal/pkg/cmd/reloader.go b/internal/pkg/cmd/reloader.go index de4f1190..15c8d3c0 100644 --- a/internal/pkg/cmd/reloader.go +++ b/internal/pkg/cmd/reloader.go @@ -23,12 +23,6 @@ import ( "k8s.io/client-go/tools/leaderelection/resourcelock" ) -const ( - lockName string = "stakaer-reloader-lock" - podNameEnv string = "POD_NAME" - podNamespaceEnv string = "POD_NAMESPACE" -) - // NewReloaderCommand starts the reloader controller func NewReloaderCommand() *cobra.Command { cmd := &cobra.Command{ @@ -72,7 +66,7 @@ func validateFlags(*cobra.Command, []string) error { // Validate that HA options are correct if options.EnableHA { - if _, _, err := validateHAEnvs(); err != nil { + if err := validateHAEnvs(); err != nil { return err } } @@ -93,21 +87,21 @@ func configureLogging(logFormat string) error { return nil } -func validateHAEnvs() (string, string, error) { +func validateHAEnvs() error { podName, podNamespace := getHAEnvs() if podName == "" { - return podName, podNamespace, fmt.Errorf("%s not set, cannot run in HA mode without %s set", podNameEnv, podNameEnv) + return fmt.Errorf("%s not set, cannot run in HA mode without %s set", constants.PodNameEnv, constants.PodNameEnv) } if podNamespace == "" { - return podName, podNamespace, fmt.Errorf("%s not set, cannot run in HA mode without %s set", podNamespaceEnv, podNamespaceEnv) + return fmt.Errorf("%s not set, cannot run in HA mode without %s set", constants.PodNamespaceEnv, constants.PodNamespaceEnv) } - return podName, podNamespace, nil + return nil } func getHAEnvs() (string, string) { - podName := os.Getenv(podNameEnv) - podNamespace := os.Getenv(podNamespaceEnv) + podName := os.Getenv(constants.PodNameEnv) + podNamespace := os.Getenv(constants.PodNamespaceEnv) return podName, podNamespace } @@ -171,7 +165,7 @@ func startReloader(cmd *cobra.Command, args []string) { // Run the leadership election if options.EnableHA { podName, podNamespace := getHAEnvs() - lock := getNewLock(clientset, lockName, podName, podNamespace) + lock := getNewLock(clientset, constants.LockName, podName, podNamespace) ctx, cancel := context.WithCancel(context.Background()) defer cancel() runLeaderElection(lock, ctx, podName, controllers) @@ -204,16 +198,18 @@ func runLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, id str Callbacks: leaderelection.LeaderCallbacks{ OnStartedLeading: func(c context.Context) { setLeader(controllers, true) + logrus.Info("became leader") }, OnStoppedLeading: func() { setLeader(controllers, false) + logrus.Info("no longer leader") }, OnNewLeader: func(current_id string) { if current_id == id { - //klog.Info("still the leader!") + logrus.Info("still the leader!") return } - //klog.Info("new leader is %s", current_id) + logrus.Infof("new leader is %s", current_id) }, }, }) diff --git a/internal/pkg/constants/constants.go b/internal/pkg/constants/constants.go index a5748c6a..0020b9cd 100644 --- a/internal/pkg/constants/constants.go +++ b/internal/pkg/constants/constants.go @@ -20,3 +20,10 @@ const ( // AnnotationsReloadStrategy instructs Reloader to add pod template annotations to facilitate a restart AnnotationsReloadStrategy = "annotations" ) + +// Leadership election related consts +const ( + LockName string = "stakaer-reloader-lock" + PodNameEnv string = "POD_NAME" + PodNamespaceEnv string = "POD_NAMESPACE" +) diff --git a/internal/pkg/controller/controller.go b/internal/pkg/controller/controller.go index bd21b604..29b8c08e 100644 --- a/internal/pkg/controller/controller.go +++ b/internal/pkg/controller/controller.go @@ -177,4 +177,5 @@ func (c *Controller) handleErr(err error, key interface{}) { func (c *Controller) SetLeader(isLeader bool) { c.isLeader = isLeader + logrus.Info("controller active") } From 16079bd1d4451e5cb16b98ab5c4f90f776d18e17 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Wed, 14 Sep 2022 16:37:40 +0100 Subject: [PATCH 03/16] Update helm chart for HA in global mode --- .../chart/reloader/templates/clusterrole.yaml | 9 +++++++++ .../chart/reloader/templates/deployment.yaml | 17 +++++++++++++++-- .../kubernetes/chart/reloader/values.yaml | 5 ++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml index c88f8bb5..f7a8aea6 100644 --- a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml +++ b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml @@ -78,3 +78,12 @@ rules: - update - patch {{- end }} +{{- if .Values.reloader.enableHA }} + - apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - get + - update +{{- end}} diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index 48f7790e..7f313e7d 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -60,7 +60,7 @@ spec: - image: "{{ .Values.reloader.deployment.image.name }}:{{ .Values.reloader.deployment.image.tag }}" imagePullPolicy: {{ .Values.reloader.deployment.image.pullPolicy }} name: {{ template "reloader-fullname" . }} - {{- if or (.Values.reloader.deployment.env.open) (.Values.reloader.deployment.env.secret) (.Values.reloader.deployment.env.field) (eq .Values.reloader.watchGlobally false) }} + {{- if or (.Values.reloader.deployment.env.open) (.Values.reloader.deployment.env.secret) (.Values.reloader.deployment.env.field) (eq .Values.reloader.watchGlobally false) (.Values.reloader.enableHA)}} env: {{- range $name, $value := .Values.reloader.deployment.env.open }} {{- if not (empty $value) }} @@ -92,6 +92,16 @@ spec: fieldRef: fieldPath: metadata.namespace {{- end }} + {{- if .Values.reloader.enableHA }} + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + {{- end }} {{- end }} ports: @@ -123,7 +133,7 @@ spec: - mountPath: /tmp/ name: tmp-volume {{- end }} - {{- if or (.Values.reloader.logFormat) (.Values.reloader.ignoreSecrets) (.Values.reloader.ignoreNamespaces) (.Values.reloader.ignoreConfigMaps) (.Values.reloader.custom_annotations) (eq .Values.reloader.isArgoRollouts true) (eq .Values.reloader.reloadOnCreate true) (ne .Values.reloader.reloadStrategy "default")}} + {{- if or (.Values.reloader.logFormat) (.Values.reloader.ignoreSecrets) (.Values.reloader.ignoreNamespaces) (.Values.reloader.ignoreConfigMaps) (.Values.reloader.custom_annotations) (eq .Values.reloader.isArgoRollouts true) (eq .Values.reloader.reloadOnCreate true) (ne .Values.reloader.reloadStrategy "default") (.Values.reloader.enableHA)}} args: {{- if .Values.reloader.logFormat }} - "--log-format={{ .Values.reloader.logFormat }}" @@ -169,6 +179,9 @@ spec: {{- if ne .Values.reloader.reloadStrategy "default" }} - "--reload-strategy={{ .Values.reloader.reloadStrategy }}" {{- end }} + {{- if or (gt .Values.reloader.deployment.replicas 1.0) (.Values.reloader.enableHA) }} + - "--enable-ha=true" + {{- end}} {{- end }} {{- if .Values.reloader.deployment.resources }} resources: diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index 9ae03a10..eb15ad92 100644 --- a/deployments/kubernetes/chart/reloader/values.yaml +++ b/deployments/kubernetes/chart/reloader/values.yaml @@ -18,13 +18,16 @@ reloader: ignoreNamespaces: "" # Comma separated list of namespaces to ignore logFormat: "" #json watchGlobally: true + # Set to true to enable leadership election allowing you to run multiple replicas + enableHA: true # Set to true if you have a pod security policy that enforces readOnlyRootFilesystem readOnlyRootFileSystem: false legacy: rbac: false matchLabels: {} deployment: - replicas: 1 + # If you wish to run multiple replicas set reloader.enableHA = true + replicas: 2 nodeSelector: # cloud.google.com/gke-nodepool: default-pool From 919f75bb62bb0c2f08fcd648404bb01380b2dae9 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Thu, 15 Sep 2022 12:09:15 +0100 Subject: [PATCH 04/16] Shutdown on leader election loss --- internal/pkg/cmd/reloader.go | 48 +++++++++++++++++---------- internal/pkg/controller/controller.go | 10 ++---- internal/pkg/handler/create.go | 10 ++---- internal/pkg/handler/handler.go | 2 +- internal/pkg/handler/update.go | 10 ++---- 5 files changed, 37 insertions(+), 43 deletions(-) diff --git a/internal/pkg/cmd/reloader.go b/internal/pkg/cmd/reloader.go index 15c8d3c0..963070ca 100644 --- a/internal/pkg/cmd/reloader.go +++ b/internal/pkg/cmd/reloader.go @@ -148,13 +148,12 @@ func startReloader(cmd *cobra.Command, args []string) { logrus.Fatalf("%s", err) } - // If HA is enabled then we need to run leadership election - if options.EnableHA { - c.SetLeader(false) - } - controllers = append(controllers, c) + // If HA is enabled we only run the controller when + if options.EnableHA { + continue + } // Now let's start the controller stop := make(chan struct{}) defer close(stop) @@ -164,11 +163,17 @@ func startReloader(cmd *cobra.Command, args []string) { // Run the leadership election if options.EnableHA { + var stopChannels []chan struct{} + for i := 0; i < len(controllers); i++ { + stop := make(chan struct{}) + stopChannels = append(stopChannels, stop) + } podName, podNamespace := getHAEnvs() lock := getNewLock(clientset, constants.LockName, podName, podNamespace) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - runLeaderElection(lock, ctx, podName, controllers) + runLeaderElection(lock, ctx, cancel, podName, controllers, stopChannels) + return } select {} @@ -187,22 +192,23 @@ func getNewLock(clientset *kubernetes.Clientset, lockName, podname, namespace st } } -func runLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, id string, controllers []*controller.Controller) { +// runLeaderElection runs leadership election. If an instance of the controller is the leader and stops leading it will shutdown. +func runLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel context.CancelFunc, id string, controllers []*controller.Controller, stopChannels []chan struct{}) { leaderelection.RunOrDie(ctx, leaderelection.LeaderElectionConfig{ Lock: lock, ReleaseOnCancel: true, - // TODO Validate that keys persist in the cache for at least one leadership election cycle - LeaseDuration: 15 * time.Second, - RenewDeadline: 10 * time.Second, - RetryPeriod: 2 * time.Second, + LeaseDuration: 15 * time.Second, + RenewDeadline: 10 * time.Second, + RetryPeriod: 2 * time.Second, Callbacks: leaderelection.LeaderCallbacks{ OnStartedLeading: func(c context.Context) { - setLeader(controllers, true) - logrus.Info("became leader") + logrus.Info("became leader, starting controllers") + runControllers(controllers, stopChannels) }, OnStoppedLeading: func() { - setLeader(controllers, false) - logrus.Info("no longer leader") + logrus.Info("no longer leader, shutting down") + stopControllers(stopChannels) + cancel() }, OnNewLeader: func(current_id string) { if current_id == id { @@ -215,10 +221,16 @@ func runLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, id str }) } -func setLeader(controllers []*controller.Controller, isLeader bool) { - for _, c := range controllers { +func runControllers(controllers []*controller.Controller, stopChannels []chan struct{}) { + for i, c := range controllers { c := c - c.SetLeader(isLeader) + go c.Run(1, stopChannels[i]) + } +} + +func stopControllers(stopChannels []chan struct{}) { + for _, c := range stopChannels { + close(c) } } diff --git a/internal/pkg/controller/controller.go b/internal/pkg/controller/controller.go index 29b8c08e..1e75bb39 100644 --- a/internal/pkg/controller/controller.go +++ b/internal/pkg/controller/controller.go @@ -30,7 +30,6 @@ type Controller struct { namespace string ignoredNamespaces util.List collectors metrics.Collectors - isLeader bool } // controllerInitialized flag determines whether controlled is being initialized @@ -58,7 +57,7 @@ func NewController( c.informer = informer c.queue = queue c.collectors = collectors - c.isLeader = true + logrus.Infof("created controller for: %s", resource) return &c, nil } @@ -143,7 +142,7 @@ func (c *Controller) processNextItem() bool { defer c.queue.Done(resourceHandler) // Invoke the method containing the business logic - err := resourceHandler.(handler.ResourceHandler).Handle(c.isLeader) + err := resourceHandler.(handler.ResourceHandler).Handle() // Handle the error if something went wrong during the execution of the business logic c.handleErr(err, resourceHandler) return true @@ -174,8 +173,3 @@ func (c *Controller) handleErr(err error, key interface{}) { runtime.HandleError(err) logrus.Infof("Dropping the key %q out of the queue: %v", key, err) } - -func (c *Controller) SetLeader(isLeader bool) { - c.isLeader = isLeader - logrus.Info("controller active") -} diff --git a/internal/pkg/handler/create.go b/internal/pkg/handler/create.go index da3c75e0..e6cc41aa 100644 --- a/internal/pkg/handler/create.go +++ b/internal/pkg/handler/create.go @@ -1,8 +1,6 @@ package handler import ( - "fmt" - "github.com/sirupsen/logrus" "github.com/stakater/Reloader/internal/pkg/metrics" "github.com/stakater/Reloader/internal/pkg/util" @@ -16,16 +14,12 @@ type ResourceCreatedHandler struct { } // Handle processes the newly created resource -func (r ResourceCreatedHandler) Handle(isLeader bool) error { +func (r ResourceCreatedHandler) Handle() error { if r.Resource == nil { logrus.Errorf("Resource creation handler received nil resource") } else { config, _ := r.GetConfig() - // process resource based on its type - if isLeader { - return doRollingUpgrade(config, r.Collectors) - } - return fmt.Errorf("instance is not leader, will not perform rolling upgrade on %s %s/%s", config.Type, config.ResourceName, config.Namespace) + return doRollingUpgrade(config, r.Collectors) } return nil } diff --git a/internal/pkg/handler/handler.go b/internal/pkg/handler/handler.go index 4dbf10e2..634e080b 100644 --- a/internal/pkg/handler/handler.go +++ b/internal/pkg/handler/handler.go @@ -6,6 +6,6 @@ import ( // ResourceHandler handles the creation and update of resources type ResourceHandler interface { - Handle(isLeader bool) error + Handle() error GetConfig() (util.Config, string) } diff --git a/internal/pkg/handler/update.go b/internal/pkg/handler/update.go index 77da76e0..91f320a6 100644 --- a/internal/pkg/handler/update.go +++ b/internal/pkg/handler/update.go @@ -1,8 +1,6 @@ package handler import ( - "fmt" - "github.com/sirupsen/logrus" "github.com/stakater/Reloader/internal/pkg/metrics" "github.com/stakater/Reloader/internal/pkg/util" @@ -17,17 +15,13 @@ type ResourceUpdatedHandler struct { } // Handle processes the updated resource -func (r ResourceUpdatedHandler) Handle(isLeader bool) error { +func (r ResourceUpdatedHandler) Handle() error { if r.Resource == nil || r.OldResource == nil { logrus.Errorf("Resource update handler received nil resource") } else { config, oldSHAData := r.GetConfig() if config.SHAValue != oldSHAData { - // process resource based on its type - if isLeader { - return doRollingUpgrade(config, r.Collectors) - } - return fmt.Errorf("instance is not leader, will not perform rolling upgrade on %s %s/%s", config.Type, config.ResourceName, config.Namespace) + return doRollingUpgrade(config, r.Collectors) } } return nil From b7e83b74d8a90f9ccba9c07717c7e0d40d184123 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Thu, 15 Sep 2022 12:24:17 +0100 Subject: [PATCH 05/16] Move leadership to its own package --- internal/pkg/cmd/reloader.go | 71 ++----------------------- internal/pkg/leadership/leadership.go | 75 +++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 67 deletions(-) create mode 100644 internal/pkg/leadership/leadership.go diff --git a/internal/pkg/cmd/reloader.go b/internal/pkg/cmd/reloader.go index 963070ca..09f893e7 100644 --- a/internal/pkg/cmd/reloader.go +++ b/internal/pkg/cmd/reloader.go @@ -6,9 +6,9 @@ import ( "fmt" "os" "strings" - "time" "github.com/stakater/Reloader/internal/pkg/constants" + "github.com/stakater/Reloader/internal/pkg/leadership" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -18,9 +18,6 @@ import ( "github.com/stakater/Reloader/internal/pkg/util" "github.com/stakater/Reloader/pkg/kube" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/tools/leaderelection" - "k8s.io/client-go/tools/leaderelection/resourcelock" ) // NewReloaderCommand starts the reloader controller @@ -161,79 +158,19 @@ func startReloader(cmd *cobra.Command, args []string) { go c.Run(1, stop) } - // Run the leadership election + // Run leadership election if options.EnableHA { - var stopChannels []chan struct{} - for i := 0; i < len(controllers); i++ { - stop := make(chan struct{}) - stopChannels = append(stopChannels, stop) - } podName, podNamespace := getHAEnvs() - lock := getNewLock(clientset, constants.LockName, podName, podNamespace) + lock := leadership.GetNewLock(clientset, constants.LockName, podName, podNamespace) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - runLeaderElection(lock, ctx, cancel, podName, controllers, stopChannels) + leadership.RunLeaderElection(lock, ctx, cancel, podName, controllers) return } select {} } -func getNewLock(clientset *kubernetes.Clientset, lockName, podname, namespace string) *resourcelock.LeaseLock { - return &resourcelock.LeaseLock{ - LeaseMeta: v1.ObjectMeta{ - Name: lockName, - Namespace: namespace, - }, - Client: clientset.CoordinationV1(), - LockConfig: resourcelock.ResourceLockConfig{ - Identity: podname, - }, - } -} - -// runLeaderElection runs leadership election. If an instance of the controller is the leader and stops leading it will shutdown. -func runLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel context.CancelFunc, id string, controllers []*controller.Controller, stopChannels []chan struct{}) { - leaderelection.RunOrDie(ctx, leaderelection.LeaderElectionConfig{ - Lock: lock, - ReleaseOnCancel: true, - LeaseDuration: 15 * time.Second, - RenewDeadline: 10 * time.Second, - RetryPeriod: 2 * time.Second, - Callbacks: leaderelection.LeaderCallbacks{ - OnStartedLeading: func(c context.Context) { - logrus.Info("became leader, starting controllers") - runControllers(controllers, stopChannels) - }, - OnStoppedLeading: func() { - logrus.Info("no longer leader, shutting down") - stopControllers(stopChannels) - cancel() - }, - OnNewLeader: func(current_id string) { - if current_id == id { - logrus.Info("still the leader!") - return - } - logrus.Infof("new leader is %s", current_id) - }, - }, - }) -} - -func runControllers(controllers []*controller.Controller, stopChannels []chan struct{}) { - for i, c := range controllers { - c := c - go c.Run(1, stopChannels[i]) - } -} - -func stopControllers(stopChannels []chan struct{}) { - for _, c := range stopChannels { - close(c) - } -} - func getIgnoredNamespacesList(cmd *cobra.Command) (util.List, error) { return getStringSliceFromFlags(cmd, "namespaces-to-ignore") } diff --git a/internal/pkg/leadership/leadership.go b/internal/pkg/leadership/leadership.go new file mode 100644 index 00000000..e0f3db55 --- /dev/null +++ b/internal/pkg/leadership/leadership.go @@ -0,0 +1,75 @@ +package leadership + +import ( + "context" + "time" + + "github.com/sirupsen/logrus" + "github.com/stakater/Reloader/internal/pkg/controller" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/leaderelection" + "k8s.io/client-go/tools/leaderelection/resourcelock" +) + +func GetNewLock(clientset *kubernetes.Clientset, lockName, podname, namespace string) *resourcelock.LeaseLock { + return &resourcelock.LeaseLock{ + LeaseMeta: v1.ObjectMeta{ + Name: lockName, + Namespace: namespace, + }, + Client: clientset.CoordinationV1(), + LockConfig: resourcelock.ResourceLockConfig{ + Identity: podname, + }, + } +} + +// runLeaderElection runs leadership election. If an instance of the controller is the leader and stops leading it will shutdown. +func RunLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel context.CancelFunc, id string, controllers []*controller.Controller) { + // Construct channels for the controllers to use + var stopChannels []chan struct{} + for i := 0; i < len(controllers); i++ { + stop := make(chan struct{}) + stopChannels = append(stopChannels, stop) + } + + leaderelection.RunOrDie(ctx, leaderelection.LeaderElectionConfig{ + Lock: lock, + ReleaseOnCancel: true, + LeaseDuration: 15 * time.Second, + RenewDeadline: 10 * time.Second, + RetryPeriod: 2 * time.Second, + Callbacks: leaderelection.LeaderCallbacks{ + OnStartedLeading: func(c context.Context) { + logrus.Info("became leader, starting controllers") + runControllers(controllers, stopChannels) + }, + OnStoppedLeading: func() { + logrus.Info("no longer leader, shutting down") + stopControllers(stopChannels) + cancel() + }, + OnNewLeader: func(current_id string) { + if current_id == id { + logrus.Info("still the leader!") + return + } + logrus.Infof("new leader is %s", current_id) + }, + }, + }) +} + +func runControllers(controllers []*controller.Controller, stopChannels []chan struct{}) { + for i, c := range controllers { + c := c + go c.Run(1, stopChannels[i]) + } +} + +func stopControllers(stopChannels []chan struct{}) { + for _, c := range stopChannels { + close(c) + } +} From d34c99baf4edb1a202cc7823f91db4c4bd1ca368 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Thu, 15 Sep 2022 12:43:28 +0100 Subject: [PATCH 06/16] Add liveness probe --- internal/pkg/cmd/reloader.go | 21 ++++++++++++++++++--- internal/pkg/leadership/leadership.go | 3 ++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/pkg/cmd/reloader.go b/internal/pkg/cmd/reloader.go index 09f893e7..18f0999d 100644 --- a/internal/pkg/cmd/reloader.go +++ b/internal/pkg/cmd/reloader.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/http" "os" "strings" @@ -20,6 +21,11 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +var ( + // Used for liveness probe + healthy bool = true +) + // NewReloaderCommand starts the reloader controller func NewReloaderCommand() *cobra.Command { cmd := &cobra.Command{ @@ -164,11 +170,11 @@ func startReloader(cmd *cobra.Command, args []string) { lock := leadership.GetNewLock(clientset, constants.LockName, podName, podNamespace) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - leadership.RunLeaderElection(lock, ctx, cancel, podName, controllers) - return + leadership.RunLeaderElection(lock, ctx, cancel, podName, controllers, healthy) } - select {} + http.HandleFunc("/live", healthz) + logrus.Fatal(http.ListenAndServe(":8080", nil)) } func getIgnoredNamespacesList(cmd *cobra.Command) (util.List, error) { @@ -203,3 +209,12 @@ func getIgnoredResourcesList(cmd *cobra.Command) (util.List, error) { return ignoredResourcesList, nil } + +func healthz(w http.ResponseWriter, req *http.Request) { + if healthy { + w.WriteHeader(http.StatusOK) + return + } + + w.WriteHeader(http.StatusInternalServerError) +} diff --git a/internal/pkg/leadership/leadership.go b/internal/pkg/leadership/leadership.go index e0f3db55..cdc37663 100644 --- a/internal/pkg/leadership/leadership.go +++ b/internal/pkg/leadership/leadership.go @@ -26,7 +26,7 @@ func GetNewLock(clientset *kubernetes.Clientset, lockName, podname, namespace st } // runLeaderElection runs leadership election. If an instance of the controller is the leader and stops leading it will shutdown. -func RunLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel context.CancelFunc, id string, controllers []*controller.Controller) { +func RunLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel context.CancelFunc, id string, controllers []*controller.Controller, health bool) { // Construct channels for the controllers to use var stopChannels []chan struct{} for i := 0; i < len(controllers); i++ { @@ -49,6 +49,7 @@ func RunLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel logrus.Info("no longer leader, shutting down") stopControllers(stopChannels) cancel() + health = false }, OnNewLeader: func(current_id string) { if current_id == id { From 11ae057b0a80ef685ae2940656abb0267c9858ea Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Fri, 16 Sep 2022 12:06:04 +0100 Subject: [PATCH 07/16] Add tests for leadership election Pull liveness into leadership to ease testing, logically the liveness probe is directly affected by leadership so it makes sense here. Moved some of the components of the controller tests into the testutil package for reuse in my own tests. --- internal/pkg/cmd/reloader.go | 22 +-- internal/pkg/handler/create.go | 1 + internal/pkg/handler/update.go | 1 + internal/pkg/leadership/leadership.go | 36 +++- internal/pkg/leadership/leadership_test.go | 213 +++++++++++++++++++++ internal/pkg/testutil/kube.go | 14 ++ 6 files changed, 263 insertions(+), 24 deletions(-) create mode 100644 internal/pkg/leadership/leadership_test.go diff --git a/internal/pkg/cmd/reloader.go b/internal/pkg/cmd/reloader.go index 18f0999d..98105839 100644 --- a/internal/pkg/cmd/reloader.go +++ b/internal/pkg/cmd/reloader.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "net/http" "os" "strings" @@ -21,11 +20,6 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -var ( - // Used for liveness probe - healthy bool = true -) - // NewReloaderCommand starts the reloader controller func NewReloaderCommand() *cobra.Command { cmd := &cobra.Command{ @@ -167,14 +161,13 @@ func startReloader(cmd *cobra.Command, args []string) { // Run leadership election if options.EnableHA { podName, podNamespace := getHAEnvs() - lock := leadership.GetNewLock(clientset, constants.LockName, podName, podNamespace) + lock := leadership.GetNewLock(clientset.CoordinationV1(), constants.LockName, podName, podNamespace) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - leadership.RunLeaderElection(lock, ctx, cancel, podName, controllers, healthy) + leadership.RunLeaderElection(lock, ctx, cancel, podName, controllers) } - http.HandleFunc("/live", healthz) - logrus.Fatal(http.ListenAndServe(":8080", nil)) + logrus.Fatal(leadership.Healthz()) } func getIgnoredNamespacesList(cmd *cobra.Command) (util.List, error) { @@ -209,12 +202,3 @@ func getIgnoredResourcesList(cmd *cobra.Command) (util.List, error) { return ignoredResourcesList, nil } - -func healthz(w http.ResponseWriter, req *http.Request) { - if healthy { - w.WriteHeader(http.StatusOK) - return - } - - w.WriteHeader(http.StatusInternalServerError) -} diff --git a/internal/pkg/handler/create.go b/internal/pkg/handler/create.go index e6cc41aa..f6364c5c 100644 --- a/internal/pkg/handler/create.go +++ b/internal/pkg/handler/create.go @@ -19,6 +19,7 @@ func (r ResourceCreatedHandler) Handle() error { logrus.Errorf("Resource creation handler received nil resource") } else { config, _ := r.GetConfig() + // process resource based on its type return doRollingUpgrade(config, r.Collectors) } return nil diff --git a/internal/pkg/handler/update.go b/internal/pkg/handler/update.go index 91f320a6..4854151c 100644 --- a/internal/pkg/handler/update.go +++ b/internal/pkg/handler/update.go @@ -21,6 +21,7 @@ func (r ResourceUpdatedHandler) Handle() error { } else { config, oldSHAData := r.GetConfig() if config.SHAValue != oldSHAData { + // process resource based on its type return doRollingUpgrade(config, r.Collectors) } } diff --git a/internal/pkg/leadership/leadership.go b/internal/pkg/leadership/leadership.go index cdc37663..0e8429bf 100644 --- a/internal/pkg/leadership/leadership.go +++ b/internal/pkg/leadership/leadership.go @@ -2,23 +2,32 @@ package leadership import ( "context" + "net/http" "time" "github.com/sirupsen/logrus" "github.com/stakater/Reloader/internal/pkg/controller" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/leaderelection" "k8s.io/client-go/tools/leaderelection/resourcelock" + + coordinationv1 "k8s.io/client-go/kubernetes/typed/coordination/v1" ) -func GetNewLock(clientset *kubernetes.Clientset, lockName, podname, namespace string) *resourcelock.LeaseLock { +const healthPort string = ":9091" + +var ( + // Used for liveness probe + healthy bool = true +) + +func GetNewLock(client coordinationv1.CoordinationV1Interface, lockName, podname, namespace string) *resourcelock.LeaseLock { return &resourcelock.LeaseLock{ LeaseMeta: v1.ObjectMeta{ Name: lockName, Namespace: namespace, }, - Client: clientset.CoordinationV1(), + Client: client, LockConfig: resourcelock.ResourceLockConfig{ Identity: podname, }, @@ -26,7 +35,7 @@ func GetNewLock(clientset *kubernetes.Clientset, lockName, podname, namespace st } // runLeaderElection runs leadership election. If an instance of the controller is the leader and stops leading it will shutdown. -func RunLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel context.CancelFunc, id string, controllers []*controller.Controller, health bool) { +func RunLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel context.CancelFunc, id string, controllers []*controller.Controller) { // Construct channels for the controllers to use var stopChannels []chan struct{} for i := 0; i < len(controllers); i++ { @@ -49,7 +58,7 @@ func RunLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel logrus.Info("no longer leader, shutting down") stopControllers(stopChannels) cancel() - health = false + healthy = false }, OnNewLeader: func(current_id string) { if current_id == id { @@ -74,3 +83,20 @@ func stopControllers(stopChannels []chan struct{}) { close(c) } } + +// Healthz serves the liveness probe endpoint. If leadership election is +// enabled and a replica stops leading the liveness probe will fail and the +// kubelet will restart the container. +func Healthz() error { + http.HandleFunc("/live", healthz) + return http.ListenAndServe(healthPort, nil) +} + +func healthz(w http.ResponseWriter, req *http.Request) { + if healthy { + w.Write([]byte("alive")) + return + } + + w.WriteHeader(http.StatusInternalServerError) +} diff --git a/internal/pkg/leadership/leadership_test.go b/internal/pkg/leadership/leadership_test.go new file mode 100644 index 00000000..2d64d0cb --- /dev/null +++ b/internal/pkg/leadership/leadership_test.go @@ -0,0 +1,213 @@ +package leadership + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "os" + "testing" + "time" + + "github.com/sirupsen/logrus" + "github.com/stakater/Reloader/internal/pkg/constants" + "github.com/stakater/Reloader/internal/pkg/controller" + "github.com/stakater/Reloader/internal/pkg/handler" + "github.com/stakater/Reloader/internal/pkg/metrics" + "github.com/stakater/Reloader/internal/pkg/options" + "github.com/stakater/Reloader/internal/pkg/testutil" + "github.com/stakater/Reloader/internal/pkg/util" + "github.com/stakater/Reloader/pkg/kube" +) + +func TestMain(m *testing.M) { + + testutil.CreateNamespace(testutil.Namespace, testutil.Clients.KubernetesClient) + + logrus.Infof("Running Testcases") + retCode := m.Run() + + testutil.DeleteNamespace(testutil.Namespace, testutil.Clients.KubernetesClient) + + os.Exit(retCode) +} + +func TestHealthz(t *testing.T) { + request, err := http.NewRequest(http.MethodGet, "/live", nil) + if err != nil { + t.Fatalf(("failed to create request")) + } + + response := httptest.NewRecorder() + + healthz(response, request) + got := response.Code + want := 200 + + if got != want { + t.Fatalf("got: %q, want: %q", got, want) + } + + // Have the liveness probe serve a 500 + healthy = false + + request, err = http.NewRequest(http.MethodGet, "/live", nil) + if err != nil { + t.Fatalf(("failed to create request")) + } + + response = httptest.NewRecorder() + + healthz(response, request) + got = response.Code + want = 500 + + if got != want { + t.Fatalf("got: %q, want: %q", got, want) + } +} + +// TestRunLeaderElection validates that the liveness endpoint serves 500 when +// leadership election fails +func TestRunLeaderElection(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + + lock := GetNewLock(testutil.Clients.KubernetesClient.CoordinationV1(), constants.LockName, testutil.Pod, testutil.Namespace) + + go RunLeaderElection(lock, ctx, cancel, testutil.Pod, []*controller.Controller{}) + + // Liveness probe should be serving OK + request, err := http.NewRequest(http.MethodGet, "/live", nil) + if err != nil { + t.Fatalf(("failed to create request")) + } + + response := httptest.NewRecorder() + + healthz(response, request) + got := response.Code + want := 500 + + if got != want { + t.Fatalf("got: %q, want: %q", got, want) + } + + // Cancel the leader election context, so leadership is released and + // live endpoint serves 500 + cancel() + + request, err = http.NewRequest(http.MethodGet, "/live", nil) + if err != nil { + t.Fatalf(("failed to create request")) + } + + response = httptest.NewRecorder() + + healthz(response, request) + got = response.Code + want = 500 + + if got != want { + t.Fatalf("got: %q, want: %q", got, want) + } +} + +// TestRunLeaderElectionWithControllers tests that leadership election works +// wiht real controllers and that on context cancellation the controllers stop +// running. +func TestRunLeaderElectionWithControllers(t *testing.T) { + t.Logf("Creating controller") + var controllers []*controller.Controller + for k := range kube.ResourceMap { + c, err := controller.NewController(testutil.Clients.KubernetesClient, k, testutil.Namespace, []string{}, metrics.NewCollectors()) + if err != nil { + logrus.Fatalf("%s", err) + } + + controllers = append(controllers, c) + } + time.Sleep(3 * time.Second) + + lock := GetNewLock(testutil.Clients.KubernetesClient.CoordinationV1(), fmt.Sprintf("%s-%d", constants.LockName, 1), testutil.Pod, testutil.Namespace) + + ctx, cancel := context.WithCancel(context.TODO()) + + // Start running leadership election, this also starts the controllers + go RunLeaderElection(lock, ctx, cancel, testutil.Pod, controllers) + time.Sleep(3 * time.Second) + + // Create some stuff and do a thing + configmapName := testutil.ConfigmapNamePrefix + "-update-" + testutil.RandSeq(5) + configmapClient, err := testutil.CreateConfigMap(testutil.Clients.KubernetesClient, testutil.Namespace, configmapName, "www.google.com") + if err != nil { + t.Fatalf("Error while creating the configmap %v", err) + } + + // Creating deployment + _, err = testutil.CreateDeployment(testutil.Clients.KubernetesClient, configmapName, testutil.Namespace, true) + if err != nil { + t.Fatalf("Error in deployment creation: %v", err) + } + + // Updating configmap for first time + updateErr := testutil.UpdateConfigMap(configmapClient, testutil.Namespace, configmapName, "", "www.stakater.com") + if updateErr != nil { + t.Fatalf("Configmap was not updated") + } + time.Sleep(3 * time.Second) + + // Verifying deployment update + logrus.Infof("Verifying pod envvars has been created") + shaData := testutil.ConvertResourceToSHA(testutil.ConfigmapResourceType, testutil.Namespace, configmapName, "www.stakater.com") + config := util.Config{ + Namespace: testutil.Namespace, + ResourceName: configmapName, + SHAValue: shaData, + Annotation: options.ConfigmapUpdateOnChangeAnnotation, + } + deploymentFuncs := handler.GetDeploymentRollingUpgradeFuncs() + updated := testutil.VerifyResourceEnvVarUpdate(testutil.Clients, config, constants.ConfigmapEnvVarPostfix, deploymentFuncs) + if !updated { + t.Fatalf("Deployment was not updated") + } + time.Sleep(testutil.SleepDuration) + + // Cancel the leader election context, so leadership is released + logrus.Info("shutting down controller from test") + cancel() + time.Sleep(5 * time.Second) + + // Updating configmap again + updateErr = testutil.UpdateConfigMap(configmapClient, testutil.Namespace, configmapName, "", "www.stakater.com/new") + if updateErr != nil { + t.Fatalf("Configmap was not updated") + } + + // Verifying that the deployment was not updated as leadership has been lost + logrus.Infof("Verifying pod envvars has not been updated") + shaData = testutil.ConvertResourceToSHA(testutil.ConfigmapResourceType, testutil.Namespace, configmapName, "www.stakater.com/new") + config = util.Config{ + Namespace: testutil.Namespace, + ResourceName: configmapName, + SHAValue: shaData, + Annotation: options.ConfigmapUpdateOnChangeAnnotation, + } + deploymentFuncs = handler.GetDeploymentRollingUpgradeFuncs() + updated = testutil.VerifyResourceEnvVarUpdate(testutil.Clients, config, constants.ConfigmapEnvVarPostfix, deploymentFuncs) + if updated { + t.Fatalf("Deployment was updated") + } + + // Deleting deployment + err = testutil.DeleteDeployment(testutil.Clients.KubernetesClient, testutil.Namespace, configmapName) + if err != nil { + logrus.Errorf("Error while deleting the deployment %v", err) + } + + // Deleting configmap + err = testutil.DeleteConfigMap(testutil.Clients.KubernetesClient, testutil.Namespace, configmapName) + if err != nil { + logrus.Errorf("Error while deleting the configmap %v", err) + } + time.Sleep(testutil.SleepDuration) +} diff --git a/internal/pkg/testutil/kube.go b/internal/pkg/testutil/kube.go index 7397aec9..35c969f1 100644 --- a/internal/pkg/testutil/kube.go +++ b/internal/pkg/testutil/kube.go @@ -16,6 +16,7 @@ import ( "github.com/stakater/Reloader/internal/pkg/callbacks" "github.com/stakater/Reloader/internal/pkg/constants" "github.com/stakater/Reloader/internal/pkg/crypto" + "github.com/stakater/Reloader/internal/pkg/metrics" "github.com/stakater/Reloader/internal/pkg/options" "github.com/stakater/Reloader/internal/pkg/util" "github.com/stakater/Reloader/pkg/kube" @@ -34,6 +35,19 @@ var ( SecretResourceType = "secrets" ) +var ( + Clients = kube.GetClients() + Pod = "test-reloader-" + RandSeq(5) + Namespace = "test-reloader-" + RandSeq(5) + ConfigmapNamePrefix = "testconfigmap-reloader" + SecretNamePrefix = "testsecret-reloader" + Data = "dGVzdFNlY3JldEVuY29kaW5nRm9yUmVsb2FkZXI=" + NewData = "dGVzdE5ld1NlY3JldEVuY29kaW5nRm9yUmVsb2FkZXI=" + UpdatedData = "dGVzdFVwZGF0ZWRTZWNyZXRFbmNvZGluZ0ZvclJlbG9hZGVy" + Collectors = metrics.NewCollectors() + SleepDuration = 3 * time.Second +) + // CreateNamespace creates namespace for testing func CreateNamespace(namespace string, client kubernetes.Interface) { _, err := client.CoreV1().Namespaces().Create(context.TODO(), &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}, metav1.CreateOptions{}) From 6299b1d8e918b0978b0d94b00c58970bb2c567c2 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Fri, 16 Sep 2022 12:08:22 +0100 Subject: [PATCH 08/16] Update helm chart with new liveness probe --- .../kubernetes/chart/reloader/templates/deployment.yaml | 6 ++++-- deployments/kubernetes/chart/reloader/values.yaml | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index 7f313e7d..bb5c9204 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -106,10 +106,12 @@ spec: ports: - name: http + containerPort: 9091 + - name: metrics containerPort: 9090 livenessProbe: httpGet: - path: /metrics + path: /live port: http timeoutSeconds: {{ .Values.reloader.deployment.livenessProbe.timeoutSeconds | default "5" }} failureThreshold: {{ .Values.reloader.deployment.livenessProbe.failureThreshold | default "5" }} @@ -118,7 +120,7 @@ spec: readinessProbe: httpGet: path: /metrics - port: http + port: metrics timeoutSeconds: {{ .Values.reloader.deployment.readinessProbe.timeoutSeconds | default "5" }} failureThreshold: {{ .Values.reloader.deployment.readinessProbe.failureThreshold | default "5" }} periodSeconds: {{ .Values.reloader.deployment.readinessProbe.periodSeconds | default "10" }} diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index eb15ad92..6eec890e 100644 --- a/deployments/kubernetes/chart/reloader/values.yaml +++ b/deployments/kubernetes/chart/reloader/values.yaml @@ -13,7 +13,7 @@ reloader: isOpenshift: false ignoreSecrets: false ignoreConfigMaps: false - reloadOnCreate: false + reloadOnCreate: true reloadStrategy: default # Set to default, env-vars or annotations ignoreNamespaces: "" # Comma separated list of namespaces to ignore logFormat: "" #json From 72a1c59cacae2f4ce0fad071121bdc983d088a8f Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Fri, 16 Sep 2022 12:15:21 +0100 Subject: [PATCH 09/16] Err check response writer --- internal/pkg/leadership/leadership.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/pkg/leadership/leadership.go b/internal/pkg/leadership/leadership.go index 0e8429bf..9e48b7ee 100644 --- a/internal/pkg/leadership/leadership.go +++ b/internal/pkg/leadership/leadership.go @@ -94,7 +94,9 @@ func Healthz() error { func healthz(w http.ResponseWriter, req *http.Request) { if healthy { - w.Write([]byte("alive")) + if i, err := w.Write([]byte("alive")); err != nil { + logrus.Infof("failed to write liveness response, wrote: %d bytes, got err: %s", i, err) + } return } From d043bcf7be51ebd2dcc012aae2f46723cd8e1a44 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Fri, 16 Sep 2022 13:44:37 +0100 Subject: [PATCH 10/16] Fix roles --- .../chart/reloader/templates/clusterrole.yaml | 3 ++- .../kubernetes/chart/reloader/templates/role.yaml | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml index f7a8aea6..a4e78bb4 100644 --- a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml +++ b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml @@ -77,13 +77,14 @@ rules: - get - update - patch -{{- end }} {{- if .Values.reloader.enableHA }} - apiGroups: - "coordination.k8s.io" resources: - leases verbs: + - create - get - update {{- end}} +{{- end }} diff --git a/deployments/kubernetes/chart/reloader/templates/role.yaml b/deployments/kubernetes/chart/reloader/templates/role.yaml index 0a8d5184..8cdb1b07 100644 --- a/deployments/kubernetes/chart/reloader/templates/role.yaml +++ b/deployments/kubernetes/chart/reloader/templates/role.yaml @@ -77,4 +77,14 @@ rules: - get - update - patch +{{- if .Values.reloader.enableHA }} + - apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - create + - get + - update +{{- end}} {{- end }} From a7c3ae37aab0276c1792d664ceb5d392537bc9b5 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Fri, 23 Sep 2022 09:02:09 +0100 Subject: [PATCH 11/16] Expand documentation about reloadOnCreate --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index abf2b0c7..b2d9ab1f 100644 --- a/README.md +++ b/README.md @@ -245,6 +245,14 @@ You can enable to scrape Reloader's Prometheus metrics by setting `serviceMonito | isArgoRollouts | Enable Argo Rollouts. Valid value are either `true` or `false` | boolean | | reloadOnCreate | Enable reload on create events. Valid value are either `true` or `false` | boolean | +**ReloadOnCreate** reloadOnCreate controls how Reloader handles secrets being added to the cache for the first time. If reloadOnCreate is set to true: +* Configmaps/secrets being added to the cache will cause Reloader to perform a rolling update of the associated workload. +* When applications are deployed for the first time, Reloader will perform a rolling update of the associated workload. +* If you are running Reloader in HA mode all workloads will have a rolling update performed when a new leader is elected. + +If ReloadOnCreate is set to false: +* Updates to configMaps/Secrets that occur while there is no leader will not be picked up by the new leader until a subsequent update of the configmap/secret occurs. In the worst case the window in which there can be no leader is 15s as this is the LeaseDuration. + ## Help ### Documentation From 28456ffafe714deff5a77cb55cb00960ff8e583f Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Fri, 23 Sep 2022 09:36:38 +0100 Subject: [PATCH 12/16] Add PodAntiAffinity if HA is enabled --- .../chart/reloader/templates/_helpers.tpl | 17 ++++++++++++++++- .../chart/reloader/templates/deployment.yaml | 5 ++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl index eb7a7010..f481f1e8 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -28,6 +28,21 @@ heritage: {{ .Release.Service | quote }} app.kubernetes.io/managed-by: {{ .Release.Service | quote }} {{- end -}} +{{/* +Create pod anti affinity labels +*/}} +{{- define "reloader-podAntiAffinity" -}} +podAntiAffinity: + preferredDuringSchedulingIgnoredDuringExecution: + - labelSelector: + matchExpressions: + - key: app + operator: In + values: + - {{ template "reloader-fullname" . }} + topologyKey: "kubernetes.io/hostname" +{{- end -}} + {{/* Create the name of the service account to use */}} @@ -45,4 +60,4 @@ Create the annotations to support helm3 {{- define "reloader-helm3.annotations" -}} meta.helm.sh/release-namespace: {{ .Release.Namespace | quote }} meta.helm.sh/release-name: {{ .Release.Name | quote }} -{{- end -}} \ No newline at end of file +{{- end -}} diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index bb5c9204..cf9535c0 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -45,9 +45,12 @@ spec: nodeSelector: {{ toYaml .Values.reloader.deployment.nodeSelector | indent 8 }} {{- end }} - {{- if .Values.reloader.deployment.affinity }} + {{- if or (.Values.reloader.deployment.affinity) (.Values.reloader.enableHA) }} affinity: + {{- if .Values.reloader.deployment.affinity }} {{ toYaml .Values.reloader.deployment.affinity | indent 8 }} + {{- end}} +{{ include "reloader-podAntiAffinity" . | indent 8 }} {{- end }} {{- if .Values.reloader.deployment.tolerations }} tolerations: From eedc8e81d089b2327b4b298e3e41a654a6edf31b Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Tue, 4 Oct 2022 08:28:34 +0100 Subject: [PATCH 13/16] Set enableHA and reloadOnCreate to false --- deployments/kubernetes/chart/reloader/values.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index 6eec890e..27baa54e 100644 --- a/deployments/kubernetes/chart/reloader/values.yaml +++ b/deployments/kubernetes/chart/reloader/values.yaml @@ -13,13 +13,13 @@ reloader: isOpenshift: false ignoreSecrets: false ignoreConfigMaps: false - reloadOnCreate: true + reloadOnCreate: false reloadStrategy: default # Set to default, env-vars or annotations ignoreNamespaces: "" # Comma separated list of namespaces to ignore logFormat: "" #json watchGlobally: true # Set to true to enable leadership election allowing you to run multiple replicas - enableHA: true + enableHA: false # Set to true if you have a pod security policy that enforces readOnlyRootFilesystem readOnlyRootFileSystem: false legacy: @@ -27,7 +27,7 @@ reloader: matchLabels: {} deployment: # If you wish to run multiple replicas set reloader.enableHA = true - replicas: 2 + replicas: 1 nodeSelector: # cloud.google.com/gke-nodepool: default-pool From deec4df12544054bdc98941c6b072bae3bb66807 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Tue, 4 Oct 2022 16:28:47 +0100 Subject: [PATCH 14/16] Fix pod antiaffinity --- .../chart/reloader/templates/_helpers.tpl | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl index f481f1e8..9edf6cad 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -34,13 +34,15 @@ Create pod anti affinity labels {{- define "reloader-podAntiAffinity" -}} podAntiAffinity: preferredDuringSchedulingIgnoredDuringExecution: - - labelSelector: - matchExpressions: - - key: app - operator: In - values: - - {{ template "reloader-fullname" . }} - topologyKey: "kubernetes.io/hostname" + - weight: 100 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: app + operator: In + values: + - {{ template "reloader-fullname" . }} + topologyKey: "kubernetes.io/hostname" {{- end -}} {{/* From 676c3703aa18725bd82fc390efee3df63c5f2c26 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Tue, 4 Oct 2022 16:29:07 +0100 Subject: [PATCH 15/16] Set replicas = 1 by default, override if HA is enabled --- .../kubernetes/chart/reloader/templates/deployment.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index cf9535c0..14341b96 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -17,7 +17,11 @@ metadata: name: {{ template "reloader-fullname" . }} namespace: {{ .Release.Namespace }} spec: +{{- if not (.Values.reloader.enableHA) }} + replicas: 1 +{{- else }} replicas: {{ .Values.reloader.deployment.replicas }} +{{- end}} revisionHistoryLimit: 2 selector: matchLabels: From 488eaa9bef50a8b8e42dd98232787641c05a4356 Mon Sep 17 00:00:00 2001 From: Alex Vest Date: Tue, 4 Oct 2022 16:29:52 +0100 Subject: [PATCH 16/16] Run leadership election as non blocking Liveness probe endpoint will always be blocking on the main thread --- internal/pkg/cmd/reloader.go | 2 +- internal/pkg/leadership/leadership.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/pkg/cmd/reloader.go b/internal/pkg/cmd/reloader.go index 98105839..f81fa3a9 100644 --- a/internal/pkg/cmd/reloader.go +++ b/internal/pkg/cmd/reloader.go @@ -164,7 +164,7 @@ func startReloader(cmd *cobra.Command, args []string) { lock := leadership.GetNewLock(clientset.CoordinationV1(), constants.LockName, podName, podNamespace) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - leadership.RunLeaderElection(lock, ctx, cancel, podName, controllers) + go leadership.RunLeaderElection(lock, ctx, cancel, podName, controllers) } logrus.Fatal(leadership.Healthz()) diff --git a/internal/pkg/leadership/leadership.go b/internal/pkg/leadership/leadership.go index 9e48b7ee..20fa779a 100644 --- a/internal/pkg/leadership/leadership.go +++ b/internal/pkg/leadership/leadership.go @@ -3,6 +3,7 @@ package leadership import ( "context" "net/http" + "sync" "time" "github.com/sirupsen/logrus" @@ -18,6 +19,7 @@ const healthPort string = ":9091" var ( // Used for liveness probe + m sync.Mutex healthy bool = true ) @@ -58,6 +60,8 @@ func RunLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, cancel logrus.Info("no longer leader, shutting down") stopControllers(stopChannels) cancel() + m.Lock() + defer m.Unlock() healthy = false }, OnNewLeader: func(current_id string) { @@ -93,6 +97,8 @@ func Healthz() error { } func healthz(w http.ResponseWriter, req *http.Request) { + m.Lock() + defer m.Unlock() if healthy { if i, err := w.Write([]byte("alive")); err != nil { logrus.Infof("failed to write liveness response, wrote: %d bytes, got err: %s", i, err)