From 3bfdd76f294ac47b90257d8bcfd760ea17cce25f Mon Sep 17 00:00:00 2001 From: Jean-Philippe Evrard Date: Sun, 29 Sep 2024 18:35:08 +0200 Subject: [PATCH] Extract privileged command wrapper into util Without this, it makes the code a bit harder to read. This fixes it by extracting the method. Signed-off-by: Jean-Philippe Evrard --- cmd/kured/main.go | 20 +------ cmd/kured/main_test.go | 127 +++++++++++++++++------------------------ pkg/util/util.go | 12 ++++ pkg/util/util_test.go | 31 ++++++++++ 4 files changed, 98 insertions(+), 92 deletions(-) create mode 100644 pkg/util/util_test.go diff --git a/cmd/kured/main.go b/cmd/kured/main.go index 77217e2..18b7d73 100644 --- a/cmd/kured/main.go +++ b/cmd/kured/main.go @@ -309,17 +309,6 @@ func flagToEnvVar(flag string) string { return fmt.Sprintf("%s_%s", EnvPrefix, envVarSuffix) } -// 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 { cmd := util.NewCommand(sentinelCommand[0], sentinelCommand[1:]...) if err := cmd.Run(); err != nil { @@ -854,16 +843,11 @@ func root(cmd *cobra.Command, args []string) { log.Fatalf("Error parsing provided reboot command: %v", err) } - // 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. - privilegedRestartCommand := buildHostCommand(1, restartCommand) - var rebooter reboot.Rebooter switch { case rebootMethod == "command": log.Infof("Reboot command: %s", restartCommand) - rebooter = reboot.CommandRebooter{NodeID: nodeID, RebootCommand: privilegedRestartCommand} + rebooter = reboot.CommandRebooter{NodeID: nodeID, RebootCommand: util.PrivilegedHostCommand(1, restartCommand)} case rebootMethod == "signal": log.Infof("Reboot signal: %v", rebootSignal) rebooter = reboot.SignalRebooter{NodeID: nodeID, Signal: rebootSignal} @@ -878,7 +862,7 @@ func root(cmd *cobra.Command, args []string) { // Only wrap sentinel-command with nsenter, if a custom-command was configured, otherwise use the host-path mount hostSentinelCommand := sentinelCommand if rebootSentinelCommand != "" { - hostSentinelCommand = buildHostCommand(1, sentinelCommand) + hostSentinelCommand = util.PrivilegedHostCommand(1, sentinelCommand) } go rebootAsRequired(nodeID, rebooter, hostSentinelCommand, window, lockTTL, lockReleaseDelay) diff --git a/cmd/kured/main_test.go b/cmd/kured/main_test.go index 04d4cd4..2659ceb 100644 --- a/cmd/kured/main_test.go +++ b/cmd/kured/main_test.go @@ -1,6 +1,7 @@ package main import ( + "github.com/kubereboot/kured/pkg/util" "reflect" "testing" @@ -23,6 +24,7 @@ func (fbc BlockingChecker) isBlocked() bool { var _ RebootBlocker = BlockingChecker{} // Verify that Type implements Interface. var _ RebootBlocker = (*BlockingChecker)(nil) // Verify that *Type implements Interface. + func Test_flagCheck(t *testing.T) { var cmd *cobra.Command var args []string @@ -107,85 +109,62 @@ func Test_stripQuotes(t *testing.T) { } } + func Test_rebootBlocked(t *testing.T) { - noCheckers := []RebootBlocker{} - nonblockingChecker := BlockingChecker{blocking: false} - blockingChecker := BlockingChecker{blocking: true} + noCheckers := []RebootBlocker{} + nonblockingChecker := BlockingChecker{blocking: false} + blockingChecker := BlockingChecker{blocking: true} - // Instantiate a prometheusClient with a broken_url - promClient, err := alerts.NewPromClient(papi.Config{Address: "broken_url"}) - if err != nil { - log.Fatal("Can't create prometheusClient: ", err) - } - brokenPrometheusClient := PrometheusBlockingChecker{promClient: promClient, filter: nil, firingOnly: false} + // Instantiate a prometheusClient with a broken_url + promClient, err := alerts.NewPromClient(papi.Config{Address: "broken_url"}) + if err != nil { + log.Fatal("Can't create prometheusClient: ", err) + } + brokenPrometheusClient := PrometheusBlockingChecker{promClient: promClient, filter: nil, firingOnly: false} - type args struct { - blockers []RebootBlocker - } - tests := []struct { - name string - args args - want bool - }{ - { - name: "Do not block on no blocker defined", - args: args{blockers: noCheckers}, - want: false, - }, - { - name: "Ensure a blocker blocks", - args: args{blockers: []RebootBlocker{blockingChecker}}, - want: true, - }, - { - name: "Ensure a non-blocker doesn't block", - args: args{blockers: []RebootBlocker{nonblockingChecker}}, - want: false, - }, - { - name: "Ensure one blocker is enough to block", - args: args{blockers: []RebootBlocker{nonblockingChecker, blockingChecker}}, - want: true, - }, - { - name: "Do block on error contacting prometheus API", - args: args{blockers: []RebootBlocker{brokenPrometheusClient}}, - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := rebootBlocked(tt.args.blockers...); got != tt.want { - t.Errorf("rebootBlocked() = %v, want %v", got, tt.want) - } - }) - } + type args struct { + blockers []RebootBlocker + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Do not block on no blocker defined", + args: args{blockers: noCheckers}, + want: false, + }, + { + name: "Ensure a blocker blocks", + args: args{blockers: []RebootBlocker{blockingChecker}}, + want: true, + }, + { + name: "Ensure a non-blocker doesn't block", + args: args{blockers: []RebootBlocker{nonblockingChecker}}, + want: false, + }, + { + name: "Ensure one blocker is enough to block", + args: args{blockers: []RebootBlocker{nonblockingChecker, blockingChecker}}, + want: true, + }, + { + name: "Do block on error contacting prometheus API", + args: args{blockers: []RebootBlocker{brokenPrometheusClient}}, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := rebootBlocked(tt.args.blockers...); got != tt.want { + t.Errorf("rebootBlocked() = %v, want %v", got, tt.want) + } + }) + } } -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 { diff --git a/pkg/util/util.go b/pkg/util/util.go index d32f9d1..ffe7480 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -1,6 +1,7 @@ package util import ( + "fmt" "os/exec" log "github.com/sirupsen/logrus" @@ -21,3 +22,14 @@ func NewCommand(name string, arg ...string) *exec.Cmd { return cmd } + +// PrivilegedHostCommand wraps the command with nsenter. +// It allows to run a command from systemd's namespace for example (pid 1) +// This relies on hostPID:true and privileged:true to enter host mount space +// For info, rancher based need different pid, which should be user given. +// until we have a better discovery mechanism. +func PrivilegedHostCommand(pid int, command []string) []string { + cmd := []string{"/usr/bin/nsenter", fmt.Sprintf("-m/proc/%d/ns/mnt", pid), "--"} + cmd = append(cmd, command...) + return cmd +} diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go new file mode 100644 index 0000000..650fee1 --- /dev/null +++ b/pkg/util/util_test.go @@ -0,0 +1,31 @@ +package util + +import ( + "reflect" + "testing" +) + +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 := PrivilegedHostCommand(tt.args.pid, tt.args.command); !reflect.DeepEqual(got, tt.want) { + t.Errorf("buildHostCommand() = %v, want %v", got, tt.want) + } + }) + } +}