From 7fb16fed9bddd61dcd1830385b2ff25fea0f05a3 Mon Sep 17 00:00:00 2001 From: Michal Schott Date: Mon, 9 Mar 2020 22:37:22 +0100 Subject: [PATCH 1/7] Adding annotationTTL. --- README.md | 11 ++++++++++- cmd/kured/main.go | 16 +++++++++++----- pkg/daemonsetlock/daemonsetlock.go | 29 ++++++++++++++++++++++++----- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 5a82bab..4925d9e 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ * [Testing](#testing) * [Disabling Reboots](#disabling-reboots) * [Manual Unlock](#manual-unlock) + * [Automatic Unlock](#automatic-unlock) * [Building](#building) * [Frequently Asked/Anticipated Questions](#frequently-askedanticipated-questions) * [Getting Help](#getting-help) @@ -46,7 +47,7 @@ compatibility of one minor version between client and server: | 1.3.0 | 1.15.10 | v12.0.0 | release-1.15 | 1.15.x, 1.16.x, 1.17.x | | 1.2.0 | 1.13.6 | v10.0.0 | release-1.13 | 1.12.x, 1.13.x, 1.14.x | | 1.1.0 | 1.12.1 | v9.0.0 | release-1.12 | 1.11.x, 1.12.x, 1.13.x | -| 1.0.0 | 1.7.6 | v4.0.0 | release-1.7 | 1.6.x, 1.7.x, 1.8.x | +| 1.0.0 | 1.7.6 | v4.0.0 | release-1.7 | 1.6.x, 1.7.x, 1.8.x | See the [release notes](https://github.com/weaveworks/kured/releases) for specific version compatibility information, including which @@ -73,6 +74,7 @@ The following arguments can be passed to kured via the daemonset pod template: ``` Flags: + --annotationTTL time force clean annotation after this ammount of time (default 0, disabled) --alert-filter-regexp regexp.Regexp alert names to ignore when checking for active alerts --blocking-pod-selector stringArray label selector identifying pods whose presence should prevent reboots --ds-name string name of daemonset on which to place lock (default "kured") @@ -259,6 +261,13 @@ kubectl -n kube-system annotate ds kured weave.works/kured-node-lock- > NB the `-` at the end of the command is important - it instructs > `kubectl` to remove that annotation entirely. +### Automatic Unlock + +In exceptional circumstances (especially when used with cluster-autoscaler) a node +which holds lock might be killed thus annotation will stay there for ever. + +Using `--annotationTTL=30m` will allow other nodes to take over if TTL has expired (in this case 30min) and continue reboot process. + ## Building See the [CircleCI config](.circleci/config.yml) for the preferred diff --git a/cmd/kured/main.go b/cmd/kured/main.go index 387e609..6bc0aed 100644 --- a/cmd/kured/main.go +++ b/cmd/kured/main.go @@ -45,6 +45,8 @@ var ( rebootEnd string timezone string + annotationTTL time.Duration + // Metrics rebootRequiredGauge = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Subsystem: "kured", @@ -97,6 +99,9 @@ func main() { rootCmd.PersistentFlags().StringVar(&timezone, "time-zone", "UTC", "use this timezone for schedule inputs") + rootCmd.PersistentFlags().DurationVar(&annotationTTL, "annotationTTL", 0, + "force clean annotation after this ammount of time (default 0, disabled)") + if err := rootCmd.Execute(); err != nil { log.Fatal(err) } @@ -204,8 +209,8 @@ func holding(lock *daemonsetlock.DaemonSetLock, metadata interface{}) bool { return holding } -func acquire(lock *daemonsetlock.DaemonSetLock, metadata interface{}) bool { - holding, holder, err := lock.Acquire(metadata) +func acquire(lock *daemonsetlock.DaemonSetLock, metadata interface{}, TTL time.Duration) bool { + holding, holder, err := lock.Acquire(metadata, TTL) switch { case err != nil: log.Fatalf("Error acquiring lock: %v", err) @@ -283,7 +288,7 @@ type nodeMeta struct { Unschedulable bool `json:"unschedulable"` } -func rebootAsRequired(nodeID string, window *timewindow.TimeWindow) { +func rebootAsRequired(nodeID string, window *timewindow.TimeWindow, TTL time.Duration) { config, err := rest.InClusterConfig() if err != nil { log.Fatal(err) @@ -314,7 +319,7 @@ func rebootAsRequired(nodeID string, window *timewindow.TimeWindow) { } nodeMeta.Unschedulable = node.Spec.Unschedulable - if acquire(lock, &nodeMeta) { + if acquire(lock, &nodeMeta, TTL) { if !nodeMeta.Unschedulable { drain(nodeID) } @@ -346,8 +351,9 @@ func root(cmd *cobra.Command, args []string) { log.Infof("Reboot Sentinel: %s every %v", rebootSentinel, period) log.Infof("Blocking Pod Selectors: %v", podSelectors) log.Infof("Reboot on: %v", window) + log.Infof("Force annotation cleanup after: %v", annotationTTL) - go rebootAsRequired(nodeID, window) + go rebootAsRequired(nodeID, window, annotationTTL) go maintainRebootRequiredMetric(nodeID) http.Handle("/metrics", promhttp.Handler()) diff --git a/pkg/daemonsetlock/daemonsetlock.go b/pkg/daemonsetlock/daemonsetlock.go index be9b782..47da093 100644 --- a/pkg/daemonsetlock/daemonsetlock.go +++ b/pkg/daemonsetlock/daemonsetlock.go @@ -19,15 +19,17 @@ type DaemonSetLock struct { } type lockAnnotationValue struct { - NodeID string `json:"nodeID"` - Metadata interface{} `json:"metadata,omitempty"` + NodeID string `json:"nodeID"` + Metadata interface{} `json:"metadata,omitempty"` + Created time.Time `json:"created"` + TTL time.Duration `json:"TTL"` } func New(client *kubernetes.Clientset, nodeID, namespace, name, annotation string) *DaemonSetLock { return &DaemonSetLock{client, nodeID, namespace, name, annotation} } -func (dsl *DaemonSetLock) Acquire(metadata interface{}) (acquired bool, owner string, err error) { +func (dsl *DaemonSetLock) Acquire(metadata interface{}, TTL time.Duration) (acquired bool, owner string, err error) { for { ds, err := dsl.client.AppsV1().DaemonSets(dsl.namespace).Get(dsl.name, metav1.GetOptions{}) if err != nil { @@ -40,13 +42,18 @@ func (dsl *DaemonSetLock) Acquire(metadata interface{}) (acquired bool, owner st if err := json.Unmarshal([]byte(valueString), &value); err != nil { return false, "", err } + + if ttlExpired(value.Created, value.TTL) { + return true, value.NodeID, nil + } + return value.NodeID == dsl.nodeID, value.NodeID, nil } if ds.ObjectMeta.Annotations == nil { ds.ObjectMeta.Annotations = make(map[string]string) } - value := lockAnnotationValue{NodeID: dsl.nodeID, Metadata: metadata} + value := lockAnnotationValue{NodeID: dsl.nodeID, Metadata: metadata, Created: time.Now().UTC(), TTL: TTL} valueBytes, err := json.Marshal(&value) if err != nil { return false, "", err @@ -79,6 +86,11 @@ func (dsl *DaemonSetLock) Test(metadata interface{}) (holding bool, err error) { if err := json.Unmarshal([]byte(valueString), &value); err != nil { return false, err } + + if ttlExpired(value.Created, value.TTL) { + return true, nil + } + return value.NodeID == dsl.nodeID, nil } @@ -98,7 +110,7 @@ func (dsl *DaemonSetLock) Release() error { if err := json.Unmarshal([]byte(valueString), &value); err != nil { return err } - if value.NodeID != dsl.nodeID { + if value.NodeID != dsl.nodeID && !ttlExpired(value.Created, value.TTL) { return fmt.Errorf("Not lock holder: %v", value.NodeID) } } else { @@ -120,3 +132,10 @@ func (dsl *DaemonSetLock) Release() error { return nil } } + +func ttlExpired(created time.Time, ttl time.Duration) bool { + if ttl > 0 && time.Now().UTC().Sub(created) >= ttl { + return true + } + return false +} From 1fc2522c0f99e6c685b2232275d3c87b818d221b Mon Sep 17 00:00:00 2001 From: Michal Schott Date: Tue, 5 May 2020 13:41:31 +0200 Subject: [PATCH 2/7] Removing spurious change. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4925d9e..83c3ff9 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ compatibility of one minor version between client and server: | 1.3.0 | 1.15.10 | v12.0.0 | release-1.15 | 1.15.x, 1.16.x, 1.17.x | | 1.2.0 | 1.13.6 | v10.0.0 | release-1.13 | 1.12.x, 1.13.x, 1.14.x | | 1.1.0 | 1.12.1 | v9.0.0 | release-1.12 | 1.11.x, 1.12.x, 1.13.x | -| 1.0.0 | 1.7.6 | v4.0.0 | release-1.7 | 1.6.x, 1.7.x, 1.8.x | +| 1.0.0 | 1.7.6 | v4.0.0 | release-1.7 | 1.6.x, 1.7.x, 1.8.x | See the [release notes](https://github.com/weaveworks/kured/releases) for specific version compatibility information, including which From 1257d97eadb2ed9a47a8ac3232ddf54869d5c452 Mon Sep 17 00:00:00 2001 From: Michal Schott Date: Tue, 5 May 2020 13:56:02 +0200 Subject: [PATCH 3/7] Be clean when this feature is disabled. --- cmd/kured/main.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/kured/main.go b/cmd/kured/main.go index 6bc0aed..ccc5457 100644 --- a/cmd/kured/main.go +++ b/cmd/kured/main.go @@ -351,7 +351,11 @@ func root(cmd *cobra.Command, args []string) { log.Infof("Reboot Sentinel: %s every %v", rebootSentinel, period) log.Infof("Blocking Pod Selectors: %v", podSelectors) log.Infof("Reboot on: %v", window) - log.Infof("Force annotation cleanup after: %v", annotationTTL) + if annotationTTL > 0 { + log.Info("Force annotation cleanup disabled.") + } else { + log.Infof("Force annotation cleanup after: %v", annotationTTL) + } go rebootAsRequired(nodeID, window, annotationTTL) go maintainRebootRequiredMetric(nodeID) From 615e3d4840c96153db48fbc9d0f3b195cbf60af3 Mon Sep 17 00:00:00 2001 From: Michal Schott Date: Tue, 5 May 2020 13:57:11 +0200 Subject: [PATCH 4/7] Calculate time difference easier. --- pkg/daemonsetlock/daemonsetlock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/daemonsetlock/daemonsetlock.go b/pkg/daemonsetlock/daemonsetlock.go index 47da093..84de9df 100644 --- a/pkg/daemonsetlock/daemonsetlock.go +++ b/pkg/daemonsetlock/daemonsetlock.go @@ -134,7 +134,7 @@ func (dsl *DaemonSetLock) Release() error { } func ttlExpired(created time.Time, ttl time.Duration) bool { - if ttl > 0 && time.Now().UTC().Sub(created) >= ttl { + if ttl > 0 && time.Since(created) >= ttl { return true } return false From 64ebf53264a2f6a90b4498ad1775b6694eb816dd Mon Sep 17 00:00:00 2001 From: Michal Schott Date: Tue, 5 May 2020 14:32:41 +0200 Subject: [PATCH 5/7] Typo in logic. --- cmd/kured/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/kured/main.go b/cmd/kured/main.go index ccc5457..5c7b74a 100644 --- a/cmd/kured/main.go +++ b/cmd/kured/main.go @@ -352,9 +352,9 @@ func root(cmd *cobra.Command, args []string) { log.Infof("Blocking Pod Selectors: %v", podSelectors) log.Infof("Reboot on: %v", window) if annotationTTL > 0 { - log.Info("Force annotation cleanup disabled.") - } else { log.Infof("Force annotation cleanup after: %v", annotationTTL) + } else { + log.Info("Force annotation cleanup disabled.") } go rebootAsRequired(nodeID, window, annotationTTL) From 59a6700addcd6e65ecb3058c25e660761a6ffe4c Mon Sep 17 00:00:00 2001 From: Michal Schott Date: Tue, 5 May 2020 20:52:10 +0200 Subject: [PATCH 6/7] Renaming flag as suggested. --- README.md | 4 ++-- cmd/kured/main.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 83c3ff9..aff9985 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ The following arguments can be passed to kured via the daemonset pod template: ``` Flags: - --annotationTTL time force clean annotation after this ammount of time (default 0, disabled) + --annotation-ttl time force clean annotation after this ammount of time (default 0, disabled) --alert-filter-regexp regexp.Regexp alert names to ignore when checking for active alerts --blocking-pod-selector stringArray label selector identifying pods whose presence should prevent reboots --ds-name string name of daemonset on which to place lock (default "kured") @@ -266,7 +266,7 @@ kubectl -n kube-system annotate ds kured weave.works/kured-node-lock- In exceptional circumstances (especially when used with cluster-autoscaler) a node which holds lock might be killed thus annotation will stay there for ever. -Using `--annotationTTL=30m` will allow other nodes to take over if TTL has expired (in this case 30min) and continue reboot process. +Using `--annotation-ttl=30m` will allow other nodes to take over if TTL has expired (in this case 30min) and continue reboot process. ## Building diff --git a/cmd/kured/main.go b/cmd/kured/main.go index 5c7b74a..a35d674 100644 --- a/cmd/kured/main.go +++ b/cmd/kured/main.go @@ -99,7 +99,7 @@ func main() { rootCmd.PersistentFlags().StringVar(&timezone, "time-zone", "UTC", "use this timezone for schedule inputs") - rootCmd.PersistentFlags().DurationVar(&annotationTTL, "annotationTTL", 0, + rootCmd.PersistentFlags().DurationVar(&annotationTTL, "annotation-ttl", 0, "force clean annotation after this ammount of time (default 0, disabled)") if err := rootCmd.Execute(); err != nil { From cf03bc587ca6a94c1bba748fd39d0158e1de4a59 Mon Sep 17 00:00:00 2001 From: Michal Schott Date: Tue, 5 May 2020 22:37:18 +0200 Subject: [PATCH 7/7] Adding unit tests for ttlExpired. --- pkg/daemonsetlock/daemonsetlock_test.go | 28 +++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 pkg/daemonsetlock/daemonsetlock_test.go diff --git a/pkg/daemonsetlock/daemonsetlock_test.go b/pkg/daemonsetlock/daemonsetlock_test.go new file mode 100644 index 0000000..3afd57e --- /dev/null +++ b/pkg/daemonsetlock/daemonsetlock_test.go @@ -0,0 +1,28 @@ +package daemonsetlock + +import ( + "testing" + "time" +) + +func TestTtlExpired(t *testing.T) { + d := time.Date(2020, 05, 05, 14, 15, 0, 0, time.UTC) + second, _ := time.ParseDuration("1s") + zero, _ := time.ParseDuration("0m") + + tests := []struct { + created time.Time + ttl time.Duration + result bool + }{ + {d, second, true}, + {time.Now(), second, false}, + {d, zero, false}, + } + + for i, tst := range tests { + if ttlExpired(tst.created, tst.ttl) != tst.result { + t.Errorf("Test %d failed, expected %v but got %v", i, tst.result, !tst.result) + } + } +}