From e31ffcd339370d6000f86d552845d7d378620d29 Mon Sep 17 00:00:00 2001 From: Alex Collins Date: Mon, 13 Jun 2022 10:02:29 -0700 Subject: [PATCH] fix: Correct kill command. Fixes #8687 (#8908) * fix: Correct kill command. Fixes #8687 Signed-off-by: Alex Collins * fix: ok Signed-off-by: Alex Collins * fix: ok Signed-off-by: Alex Collins --- cmd/argoexec/commands/kill.go | 37 +++++++++++++++++++ cmd/argoexec/commands/root.go | 1 + cmd/argoexec/commands/wait.go | 19 +++++++--- test/e2e/signals_test.go | 8 ---- ...car-injected-kill-annotation-workflow.yaml | 25 ------------- workflow/executor/executor.go | 6 --- workflow/signal/signal.go | 13 ++++++- 7 files changed, 63 insertions(+), 46 deletions(-) create mode 100644 cmd/argoexec/commands/kill.go delete mode 100644 test/e2e/testdata/sidecar-injected-kill-annotation-workflow.yaml diff --git a/cmd/argoexec/commands/kill.go b/cmd/argoexec/commands/kill.go new file mode 100644 index 000000000000..05ebcadf5cde --- /dev/null +++ b/cmd/argoexec/commands/kill.go @@ -0,0 +1,37 @@ +package commands + +import ( + "fmt" + "os" + "strconv" + "syscall" + + "github.com/spf13/cobra" +) + +func NewKillCommand() *cobra.Command { + return &cobra.Command{ + Use: "kill SIGNAL PID", + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + signum, err := strconv.Atoi(args[0]) + if err != nil { + return err + } + pid, err := strconv.Atoi(args[1]) + if err != nil { + return err + } + sig := syscall.Signal(signum) + p, err := os.FindProcess(pid) + if err != nil { + return err + } + fmt.Printf("killing %d with %v\n", pid, sig) + if err := p.Signal(sig); err != nil { + return err + } + return nil + }, + } +} diff --git a/cmd/argoexec/commands/root.go b/cmd/argoexec/commands/root.go index 12c6044a1c3f..c01aa58c02dc 100644 --- a/cmd/argoexec/commands/root.go +++ b/cmd/argoexec/commands/root.go @@ -61,6 +61,7 @@ func NewRootCommand() *cobra.Command { command.AddCommand(NewAgentCommand()) command.AddCommand(NewEmissaryCommand()) command.AddCommand(NewInitCommand()) + command.AddCommand(NewKillCommand()) command.AddCommand(NewResourceCommand()) command.AddCommand(NewWaitCommand()) command.AddCommand(NewDataCommand()) diff --git a/cmd/argoexec/commands/wait.go b/cmd/argoexec/commands/wait.go index 1ccc2aa29179..c3b66e39e4fa 100644 --- a/cmd/argoexec/commands/wait.go +++ b/cmd/argoexec/commands/wait.go @@ -2,6 +2,8 @@ package commands import ( "context" + "os/signal" + "syscall" "time" "github.com/argoproj/pkg/stats" @@ -30,13 +32,20 @@ func waitContainer(ctx context.Context) error { defer stats.LogStats() stats.StartStatsTicker(5 * time.Minute) - // Wait for main container to complete - err := wfExecutor.Wait(ctx) - if err != nil { - wfExecutor.AddError(err) + // use a block to constrain the scope of ctx + { + // this allows us to gracefully shutdown, capturing artifacts + ctx, cancel := signal.NotifyContext(ctx, syscall.SIGTERM) + defer cancel() + + // Wait for main container to complete + err := wfExecutor.Wait(ctx) + if err != nil { + wfExecutor.AddError(err) + } } // Capture output script result - err = wfExecutor.CaptureScriptResult(ctx) + err := wfExecutor.CaptureScriptResult(ctx) if err != nil { wfExecutor.AddError(err) } diff --git a/test/e2e/signals_test.go b/test/e2e/signals_test.go index 0550a274065a..6f9f9c1e91bb 100644 --- a/test/e2e/signals_test.go +++ b/test/e2e/signals_test.go @@ -128,14 +128,6 @@ func (s *SignalsSuite) TestInjectedSidecar() { WaitForWorkflow(fixtures.ToBeSucceeded, kill2xDuration) } -func (s *SignalsSuite) TestInjectedSidecarKillAnnotation() { - s.Given(). - Workflow("@testdata/sidecar-injected-kill-annotation-workflow.yaml"). - When(). - SubmitWorkflow(). - WaitForWorkflow(fixtures.ToBeSucceeded, kill2xDuration) -} - func (s *SignalsSuite) TestSubProcess() { s.Given(). Workflow("@testdata/subprocess-workflow.yaml"). diff --git a/test/e2e/testdata/sidecar-injected-kill-annotation-workflow.yaml b/test/e2e/testdata/sidecar-injected-kill-annotation-workflow.yaml deleted file mode 100644 index dcf376e29d0d..000000000000 --- a/test/e2e/testdata/sidecar-injected-kill-annotation-workflow.yaml +++ /dev/null @@ -1,25 +0,0 @@ -apiVersion: argoproj.io/v1alpha1 -kind: Workflow -metadata: - generateName: sidecar-injected-kill-annotation- -spec: - entrypoint: main - podMetadata: - annotations: - workflows.argoproj.io/kill-cmd-sidecar: '["sh", "-c", "kill -s%d -- -1"]' - podSpecPatch: | - terminationGracePeriodSeconds: 3 - containers: - - name: wait - - name: main - - name: sidecar - image: argoproj/argosay:v1 - command: - - sh - - -c - args: - - "sleep 999" - templates: - - name: main - container: - image: argoproj/argosay:v1 \ No newline at end of file diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index eb2580dd91cd..15edd254e89a 100644 --- a/workflow/executor/executor.go +++ b/workflow/executor/executor.go @@ -13,13 +13,11 @@ import ( "io/fs" "io/ioutil" "os" - "os/signal" "path" "path/filepath" "runtime/debug" "strconv" "strings" - "syscall" "time" "github.com/argoproj/argo-workflows/v3/util/file" @@ -1022,10 +1020,6 @@ func (we *WorkflowExecutor) Wait(ctx context.Context) error { log.WithField("annotationPatchTickDuration", we.annotationPatchTickDuration).WithField("readProgressFileTickDuration", we.readProgressFileTickDuration).Info("monitoring progress disabled") } - // this allows us to gracefully shutdown, capturing artifacts - ctx, cancel := signal.NotifyContext(ctx, syscall.SIGTERM) - defer cancel() - go we.monitorDeadline(ctx, containerNames) err := retryutil.OnError(executorretry.ExecutorRetry, errorsutil.IsTransientErr, func() error { diff --git a/workflow/signal/signal.go b/workflow/signal/signal.go index c93874a9f2fb..fe2fd4bbc8d4 100644 --- a/workflow/signal/signal.go +++ b/workflow/signal/signal.go @@ -3,6 +3,7 @@ package signal import ( "encoding/json" "fmt" + "path/filepath" "strings" "syscall" @@ -15,8 +16,16 @@ import ( func SignalContainer(restConfig *rest.Config, pod *corev1.Pod, container string, s syscall.Signal) error { command := []string{"/bin/sh", "-c", "kill -%d 1"} - if container == common.WaitContainerName { - command = []string{"/bin/sh", "-c", "kill -%d $(pidof argoexec)"} + + // If the container has the /var/run/argo volume mounted, this it will have access to `argoexec`. + for _, c := range pod.Spec.Containers { + if c.Name == container { + for _, m := range c.VolumeMounts { + if m.MountPath == common.VarRunArgoPath { + command = []string{filepath.Join(common.VarRunArgoPath, "argoexec"), "kill", "%d", "1"} + } + } + } } if v, ok := pod.Annotations[common.AnnotationKeyKillCmd(container)]; ok {