Add flexible commands parameters

Without this patch, you cannot configure the reboot command to use, or the use another command to trigger a reboot. This is a problem, as multiple users have asked for it in the past, and we are lacking flexibility. This fixes it by introducing two new parameters, - one to provide a custom reboot command. This should help people running kured on non systemd OS - one to provide a custom sentinel command. This should help people running non Ubuntu OS, as they can directly use their command instead of generating a file (useful for CentOS/SUSE) For this, several refactors had to be done, to remove global state in some functions. Making those functions closer to "pure functions" helps us increase our test coverage here and later. As commandReboot was very close to rebootCommand, the function to reboot the node has been renamed to invokeReboot.
2026-08-23 21:36:33 +00:00 · 2021-03-29 09:50:56 +02:00
parent 837bd4eb2a
commit fd63e9a74b
5 changed files with 248 additions and 42 deletions
@@ -29,7 +29,8 @@ Kured (KUbernetes REboot Daemon) is a Kubernetes daemonset that
 performs safe automatic node reboots when the need to do so is
 indicated by the package management system of the underlying OS.

-* Watches for the presence of a reboot sentinel e.g. `/var/run/reboot-required`
+* Watches for the presence of a reboot sentinel file e.g. `/var/run/reboot-required`
+  or the successful run of a sentinel command.
 * Utilises a lock in the API server to ensure only one node reboots at
  a time
 * Optionally defers reboots in the presence of active Prometheus alerts or selected pods
@@ -93,8 +94,10 @@ Flags:
      --period duration                     reboot check period (default 1h0m0s)
      --prefer-no-schedule-taint string     Taint name applied during pending node reboot (to prevent receiving additional pods from other rebooting nodes). Disabled by default. Set e.g. to "weave.works/kured-node-reboot" to enable tainting.
      --prometheus-url string               Prometheus instance to probe for active alerts
+      --reboot-command string               command to run when a reboot is required by the sentinel (default "/sbin/systemctl reboot")
      --reboot-days strings                 schedule reboot on these days (default [su,mo,tu,we,th,fr,sa])
      --reboot-sentinel string              path to file whose existence signals need to reboot (default "/var/run/reboot-required")
+      --reboot-sentinel-command string      command for which a successful run signals need to reboot (default ""). If non-empty, sentinel file will be ignored.
      --slack-channel string                slack channel for reboot notfications
      --slack-hook-url string               slack hook URL for reboot notfications
      --slack-username string               slack username for reboot notfications (default "kured")
@@ -110,6 +113,10 @@ values with `--reboot-sentinel` and `--period`. Each replica of the
 daemon uses a random offset derived from the period on startup so that
 nodes don't all contend for the lock simultaneously.

+Alternatively, a reboot sentinel command can be used. If a reboot
+sentinel command is used, the reboot sentinel file presence will be
+ignored.
+
 ### Setting a schedule

 By default, kured will reboot any time it detects the sentinel, but this
@@ -21,6 +21,8 @@ import (
 	"k8s.io/client-go/rest"
 	kubectldrain "k8s.io/kubectl/pkg/drain"

+	"github.com/google/shlex"
+
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/weaveworks/kured/pkg/alerts"
@@ -41,15 +43,17 @@ var (
 	lockAnnotation            string
 	lockTTL                   time.Duration
 	prometheusURL             string
-	alertFilter               *regexp.Regexp
-	rebootSentinel            string
 	preferNoScheduleTaintName string
+	alertFilter               *regexp.Regexp
+	rebootSentinelFile        string
+	rebootSentinelCommand     string
 	slackHookURL              string
 	slackUsername             string
 	slackChannel              string
 	messageTemplateDrain      string
 	messageTemplateReboot     string
 	podSelectors              []string
+	rebootCommand             string

 	rebootDays    []string
 	rebootStart   string
@@ -85,7 +89,7 @@ func main() {
 		Run:   root}

 	rootCmd.PersistentFlags().DurationVar(&period, "period", time.Minute*60,
-		"reboot check period")
+		"sentinel check period")
 	rootCmd.PersistentFlags().StringVar(&dsNamespace, "ds-namespace", "kube-system",
 		"namespace containing daemonset on which to place lock")
 	rootCmd.PersistentFlags().StringVar(&dsName, "ds-name", "kured",
@@ -98,17 +102,21 @@ func main() {
 		"Prometheus instance to probe for active alerts")
 	rootCmd.PersistentFlags().Var(&regexpValue{&alertFilter}, "alert-filter-regexp",
 		"alert names to ignore when checking for active alerts")
-	rootCmd.PersistentFlags().StringVar(&rebootSentinel, "reboot-sentinel", "/var/run/reboot-required",
-		"path to file whose existence signals need to reboot")
+	rootCmd.PersistentFlags().StringVar(&rebootSentinelFile, "reboot-sentinel", "/var/run/reboot-required",
+		"path to file whose existence triggers the reboot command")
 	rootCmd.PersistentFlags().StringVar(&preferNoScheduleTaintName, "prefer-no-schedule-taint", "",
 		"Taint name applied during pending node reboot (to prevent receiving additional pods from other rebooting nodes). Disabled by default. Set e.g. to \"weave.works/kured-node-reboot\" to enable tainting.")
+	rootCmd.PersistentFlags().StringVar(&rebootSentinelCommand, "reboot-sentinel-command", "",
+		"command for which a zero return code will trigger a reboot command")
+	rootCmd.PersistentFlags().StringVar(&rebootCommand, "reboot-command", "/bin/systemctl reboot",
+		"command to run when a reboot is required")

 	rootCmd.PersistentFlags().StringVar(&slackHookURL, "slack-hook-url", "",
-		"slack hook URL for reboot notfications")
+		"slack hook URL for notifications")
 	rootCmd.PersistentFlags().StringVar(&slackUsername, "slack-username", "kured",
-		"slack username for reboot notfications")
+		"slack username for notifications")
 	rootCmd.PersistentFlags().StringVar(&slackChannel, "slack-channel", "",
-		"slack channel for reboot notfications")
+		"slack channel for notifications")
 	rootCmd.PersistentFlags().StringVar(&messageTemplateDrain, "message-template-drain", "Draining node %s",
 		"message template used to notify about a node being drained")
 	rootCmd.PersistentFlags().StringVar(&messageTemplateReboot, "message-template-reboot", "Rebooting node %s",
@@ -134,10 +142,9 @@ func main() {
 	}
 }

-// newCommand creates a new Command with stdout/stderr wired to our standard logger
+// newCommand wires a Command with stdout/stderr to our standard loggers
 func newCommand(name string, arg ...string) *exec.Cmd {
 	cmd := exec.Command(name, arg...)
-
 	cmd.Stdout = log.NewEntry(log.StandardLogger()).
 		WithField("cmd", cmd.Args[0]).
 		WithField("std", "out").
@@ -151,10 +158,19 @@ func newCommand(name string, arg ...string) *exec.Cmd {
 	return cmd
 }

-func sentinelExists() bool {
-	// Relies on hostPID:true and privileged:true to enter host mount space
-	sentinelCmd := newCommand("/usr/bin/nsenter", "-m/proc/1/ns/mnt", "--", "/usr/bin/test", "-f", rebootSentinel)
-	if err := sentinelCmd.Run(); err != nil {
+// buildHostCommand writes a new command to run in the host namespace
+// Rancher based need different pid
+func buildHostCommand(pid int, command []string) []string {
+
+	// From the container, we nsenter into the proper PID to run the hostCommand.
+	// For this, kured daemonset need to be configured with hostPID:true and privileged:true
+	cmd := []string{"/usr/bin/nsenter", fmt.Sprintf("-m/proc/%d/ns/mnt", pid), "--"}
+	cmd = append(cmd, command...)
+	return cmd
+}
+
+func rebootRequired(sentinelCommand []string) bool {
+	if err := newCommand(sentinelCommand[0], sentinelCommand[1:]...).Run(); err != nil {
 		switch err := err.(type) {
 		case *exec.ExitError:
 			// We assume a non-zero exit code means 'reboot not required', but of course
@@ -171,15 +187,6 @@ func sentinelExists() bool {
 	return true
 }

-func rebootRequired() bool {
-	if sentinelExists() {
-		log.Infof("Reboot required")
-		return true
-	}
-	log.Infof("Reboot not required")
-	return false
-}
-
 // RebootBlocker interface should be implemented by types
 // to know if their instantiations should block a reboot
 type RebootBlocker interface {
@@ -333,8 +340,8 @@ func uncordon(client *kubernetes.Clientset, node *v1.Node) {
 	}
 }

-func commandReboot(nodeID string) {
-	log.Infof("Commanding reboot for node: %s", nodeID)
+func invokeReboot(nodeID string, rebootCommand []string) {
+	log.Infof("Running command: %s for node: %s", rebootCommand, nodeID)

 	if slackHookURL != "" {
 		if err := slack.NotifyReboot(slackHookURL, slackUsername, slackChannel, messageTemplateReboot, nodeID); err != nil {
@@ -342,16 +349,14 @@ func commandReboot(nodeID string) {
 		}
 	}

-	// Relies on hostPID:true and privileged:true to enter host mount space
-	rebootCmd := newCommand("/usr/bin/nsenter", "-m/proc/1/ns/mnt", "/bin/systemctl", "reboot")
-	if err := rebootCmd.Run(); err != nil {
-		log.Fatalf("Error invoking reboot command: %v", err)
+	if err := newCommand(rebootCommand[0], rebootCommand[1:]...).Run(); err != nil {
+		log.Fatalf("Error invoking %s command: %v", rebootCommand, err)
 	}
 }

-func maintainRebootRequiredMetric(nodeID string) {
+func maintainRebootRequiredMetric(nodeID string, sentinelCommand []string) {
 	for {
-		if sentinelExists() {
+		if rebootRequired(sentinelCommand) {
 			rebootRequiredGauge.WithLabelValues(nodeID).Set(1)
 		} else {
 			rebootRequiredGauge.WithLabelValues(nodeID).Set(0)
@@ -403,7 +408,7 @@ func deleteNodeAnnotation(client *kubernetes.Clientset, nodeID, key string) {
 	}
 }

-func rebootAsRequired(nodeID string, window *timewindow.TimeWindow, TTL time.Duration) {
+func rebootAsRequired(nodeID string, rebootCommand []string, sentinelCommand []string, window *timewindow.TimeWindow, TTL time.Duration) {
 	config, err := rest.InClusterConfig()
 	if err != nil {
 		log.Fatal(err)
@@ -430,7 +435,7 @@ func rebootAsRequired(nodeID string, window *timewindow.TimeWindow, TTL time.Dur
 		// And (2) check if we previously annotated the node that it was in the process of being rebooted,
 		// And finally (3) if it has that annotation, to delete it.
 		// This indicates to other node tools running on the cluster that this node may be a candidate for maintenance
-		if annotateNodes && !rebootRequired() {
+		if annotateNodes && !rebootRequired(sentinelCommand) {
 			if _, ok := node.Annotations[KuredRebootInProgressAnnotation]; ok {
 				deleteNodeAnnotation(client, nodeID, KuredRebootInProgressAnnotation)
 			}
@@ -441,7 +446,7 @@ func rebootAsRequired(nodeID string, window *timewindow.TimeWindow, TTL time.Dur
 	preferNoScheduleTaint := taints.New(client, nodeID, preferNoScheduleTaintName, v1.TaintEffectPreferNoSchedule)

 	// Remove taint immediately during startup to quickly allow scheduling again.
-	if !rebootRequired() {
+	if !rebootRequired(sentinelCommand) {
 		preferNoScheduleTaint.Disable()
 	}

@@ -454,10 +459,12 @@ func rebootAsRequired(nodeID string, window *timewindow.TimeWindow, TTL time.Dur
 			continue
 		}

-		if !rebootRequired() {
+		if !rebootRequired(sentinelCommand) {
+			log.Infof("Reboot not required")
 			preferNoScheduleTaint.Disable()
 			continue
 		}
+		log.Infof("Reboot required")

 		var blockCheckers []RebootBlocker
 		if prometheusURL != "" {
@@ -497,7 +504,7 @@ func rebootAsRequired(nodeID string, window *timewindow.TimeWindow, TTL time.Dur
 		}

 		drain(client, node)
-		commandReboot(nodeID)
+		invokeReboot(nodeID, rebootCommand)
 		for {
 			log.Infof("Waiting for reboot")
 			time.Sleep(time.Minute)
@@ -505,6 +512,29 @@ func rebootAsRequired(nodeID string, window *timewindow.TimeWindow, TTL time.Dur
 	}
 }

+// buildSentinelCommand creates the shell command line which will need wrapping to escape
+// the container boundaries
+func buildSentinelCommand(rebootSentinelFile string, rebootSentinelCommand string) []string {
+	if rebootSentinelCommand != "" {
+		cmd, err := shlex.Split(rebootSentinelCommand)
+		if err != nil {
+			log.Fatalf("Error parsing provided sentinel command: %v", err)
+		}
+		return cmd
+	}
+	return []string{"test", "-f", rebootSentinelFile}
+}
+
+// parseRebootCommand creates the shell command line which will need wrapping to escape
+// the container boundaries
+func parseRebootCommand(rebootCommand string) []string {
+	command, err := shlex.Split(rebootCommand)
+	if err != nil {
+		log.Fatalf("Error parsing provided reboot command: %v", err)
+	}
+	return command
+}
+
 func root(cmd *cobra.Command, args []string) {
 	log.Infof("Kubernetes Reboot Daemon: %s", version)

@@ -518,6 +548,9 @@ func root(cmd *cobra.Command, args []string) {
 		log.Fatalf("Failed to build time window: %v", err)
 	}

+	sentinelCommand := buildSentinelCommand(rebootSentinelFile, rebootSentinelCommand)
+	restartCommand := parseRebootCommand(rebootCommand)
+
 	log.Infof("Node ID: %s", nodeID)
 	log.Infof("Lock Annotation: %s/%s:%s", dsNamespace, dsName, lockAnnotation)
 	if lockTTL > 0 {
@@ -526,15 +559,22 @@ func root(cmd *cobra.Command, args []string) {
 		log.Info("Lock TTL not set, lock will remain until being released")
 	}
 	log.Infof("PreferNoSchedule taint: %s", preferNoScheduleTaintName)
-	log.Infof("Reboot Sentinel: %s every %v", rebootSentinel, period)
 	log.Infof("Blocking Pod Selectors: %v", podSelectors)
-	log.Infof("Reboot on: %v", window)
+	log.Infof("Reboot schedule: %v", window)
+	log.Infof("Reboot check command: %s every %v", sentinelCommand, period)
+	log.Infof("Reboot command: %s", restartCommand)
 	if annotateNodes {
 		log.Infof("Will annotate nodes during kured reboot operations")
 	}

-	go rebootAsRequired(nodeID, window, lockTTL)
-	go maintainRebootRequiredMetric(nodeID)
+	// To run those commands as it was the host, we'll use nsenter
+	// Relies on hostPID:true and privileged:true to enter host mount space
+	// PID set to 1, until we have a better discovery mechanism.
+	hostSentinelCommand := buildHostCommand(1, sentinelCommand)
+	hostRestartCommand := buildHostCommand(1, restartCommand)
+
+	go rebootAsRequired(nodeID, hostRestartCommand, hostSentinelCommand, window, lockTTL)
+	go maintainRebootRequiredMetric(nodeID, hostSentinelCommand)

 	http.Handle("/metrics", promhttp.Handler())
 	log.Fatal(http.ListenAndServe(":8080", nil))
@@ -1,6 +1,12 @@
 package main

-import "testing"
+import (
+	"reflect"
+	"testing"
+
+	log "github.com/sirupsen/logrus"
+	assert "gotest.tools/v3/assert"
+)

 type BlockingChecker struct {
 	blocking bool
@@ -61,3 +67,150 @@ func Test_rebootBlocked(t *testing.T) {
 		})
 	}
 }
+
+func Test_buildHostCommand(t *testing.T) {
+	type args struct {
+		pid     int
+		command []string
+	}
+	tests := []struct {
+		name string
+		args args
+		want []string
+	}{
+		{
+			name: "Ensure command will run with nsenter",
+			args: args{pid: 1, command: []string{"ls", "-Fal"}},
+			want: []string{"/usr/bin/nsenter", "-m/proc/1/ns/mnt", "--", "ls", "-Fal"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := buildHostCommand(tt.args.pid, tt.args.command); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("buildHostCommand() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_buildSentinelCommand(t *testing.T) {
+	type args struct {
+		rebootSentinelFile    string
+		rebootSentinelCommand string
+	}
+	tests := []struct {
+		name string
+		args args
+		want []string
+	}{
+		{
+			name: "Ensure a sentinelFile generates a shell 'test' command with the right file",
+			args: args{
+				rebootSentinelFile:    "/test1",
+				rebootSentinelCommand: "",
+			},
+			want: []string{"test", "-f", "/test1"},
+		},
+		{
+			name: "Ensure a sentinelCommand has priority over a sentinelFile if both are provided (because sentinelFile is always provided)",
+			args: args{
+				rebootSentinelFile:    "/test1",
+				rebootSentinelCommand: "/sbin/reboot-required -r",
+			},
+			want: []string{"/sbin/reboot-required", "-r"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := buildSentinelCommand(tt.args.rebootSentinelFile, tt.args.rebootSentinelCommand); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("buildSentinelCommand() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_parseRebootCommand(t *testing.T) {
+	type args struct {
+		rebootCommand string
+	}
+	tests := []struct {
+		name string
+		args args
+		want []string
+	}{
+		{
+			name: "Ensure a reboot command is properly parsed",
+			args: args{
+				rebootCommand: "/sbin/systemctl reboot",
+			},
+			want: []string{"/sbin/systemctl", "reboot"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := parseRebootCommand(tt.args.rebootCommand); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("parseRebootCommand() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_rebootRequired(t *testing.T) {
+	type args struct {
+		sentinelCommand []string
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "Ensure rc = 0 means reboot required",
+			args: args{
+				sentinelCommand: []string{"true"},
+			},
+			want: true,
+		},
+		{
+			name: "Ensure rc != 0 means reboot NOT required",
+			args: args{
+				sentinelCommand: []string{"false"},
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := rebootRequired(tt.args.sentinelCommand); got != tt.want {
+				t.Errorf("rebootRequired() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_rebootRequired_fatals(t *testing.T) {
+	cases := []struct {
+		param       []string
+		expectFatal bool
+	}{
+		{
+			param:       []string{"true"},
+			expectFatal: false,
+		},
+		{
+			param:       []string{"./babar"},
+			expectFatal: true,
+		},
+	}
+
+	defer func() { log.StandardLogger().ExitFunc = nil }()
+	var fatal bool
+	log.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+	for _, c := range cases {
+		fatal = false
+		rebootRequired(c.param)
+		assert.Equal(t, c.expectFatal, fatal)
+	}
+
+}
@@ -3,10 +3,12 @@ module github.com/weaveworks/kured
 go 1.15

 require (
+	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/prometheus/client_golang v1.8.0
 	github.com/prometheus/common v0.15.0
 	github.com/sirupsen/logrus v1.8.1
 	github.com/spf13/cobra v1.1.3
+	gotest.tools/v3 v3.0.3
 	k8s.io/api v0.20.1
 	k8s.io/apimachinery v0.20.1
 	k8s.io/client-go v0.20.1
@@ -215,6 +215,8 @@ github.com/google/pprof v0.0.0-20191218002539-d4f498aebedc/go.mod h1:ZgVRPoUq/hf
 github.com/google/pprof v0.0.0-20200212024743-f11f1df84d12/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM=
 github.com/google/pprof v0.0.0-20200229191704-1ebb73c60ed3/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
+github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
+github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
 github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
@@ -756,6 +758,8 @@ gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C
 gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=
 gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
 gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=
+gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0=
+gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8=
 honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=