From 217b598684c6d0cb9384e8c649f8e73659c5f9e5 Mon Sep 17 00:00:00 2001 From: Anton Gilgur <4970083+agilgur5@users.noreply.github.com> Date: Sun, 10 Nov 2024 21:48:13 -0500 Subject: [PATCH] fix: consistently set executor log options (#12979) Signed-off-by: Anton Gilgur --- util/cmd/glog.go | 8 ++++++++ workflow/controller/agent.go | 4 ++-- workflow/controller/artifact_gc.go | 2 +- workflow/controller/operator.go | 4 ++-- workflow/controller/operator_test.go | 6 ++---- workflow/controller/workflowpod.go | 14 ++++++------- workflow/controller/workflowpod_test.go | 27 +++++++++++-------------- 7 files changed, 34 insertions(+), 31 deletions(-) diff --git a/util/cmd/glog.go b/util/cmd/glog.go index 6fd5e49d2dee..b0f844b425e8 100644 --- a/util/cmd/glog.go +++ b/util/cmd/glog.go @@ -15,3 +15,11 @@ func SetGLogLevel(glogLevel int) { _ = flag.Set("logtostderr", "true") _ = flag.Set("v", strconv.Itoa(glogLevel)) } + +func GetGLogLevel() string { + f := flag.Lookup("v") + if f == nil { + return "" + } + return f.Value.String() +} diff --git a/workflow/controller/agent.go b/workflow/controller/agent.go index 106b5c641284..de484272e802 100644 --- a/workflow/controller/agent.go +++ b/workflow/controller/agent.go @@ -191,11 +191,11 @@ func (woc *wfOperationCtx) createAgentPod(ctx context.Context) (*apiv1.Pod, erro // the `init` container populates the shared empty-dir volume with tokens agentInitCtr := agentCtrTemplate.DeepCopy() agentInitCtr.Name = common.InitContainerName - agentInitCtr.Args = []string{"agent", "init", "--loglevel", getExecutorLogLevel()} + agentInitCtr.Args = append([]string{"agent", "init"}, woc.getExecutorLogOpts()...) // the `main` container runs the actual work agentMainCtr := agentCtrTemplate.DeepCopy() agentMainCtr.Name = common.MainContainerName - agentMainCtr.Args = []string{"agent", "main", "--loglevel", getExecutorLogLevel()} + agentMainCtr.Args = append([]string{"agent", "main"}, woc.getExecutorLogOpts()...) pod := &apiv1.Pod{ ObjectMeta: metav1.ObjectMeta{ diff --git a/workflow/controller/artifact_gc.go b/workflow/controller/artifact_gc.go index 5002cbb8fc57..c485458a8fec 100644 --- a/workflow/controller/artifact_gc.go +++ b/workflow/controller/artifact_gc.go @@ -438,7 +438,7 @@ func (woc *wfOperationCtx) createArtifactGCPod(ctx context.Context, strategy wfv Name: common.MainContainerName, Image: woc.controller.executorImage(), ImagePullPolicy: woc.controller.executorImagePullPolicy(), - Args: []string{"artifact", "delete", "--loglevel", getExecutorLogLevel()}, + Args: append([]string{"artifact", "delete"}, woc.getExecutorLogOpts()...), Env: []corev1.EnvVar{ {Name: common.EnvVarArtifactGCPodHash, Value: woc.artifactGCPodLabel(podName)}, }, diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index c4120f0c6d2e..bc86b71da857 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -3398,7 +3398,7 @@ func (woc *wfOperationCtx) executeResource(ctx context.Context, nodeName string, } mainCtr := woc.newExecContainer(common.MainContainerName, tmpl) - mainCtr.Command = []string{"argoexec", "resource", tmpl.Resource.Action} + mainCtr.Command = append([]string{"argoexec", "resource", tmpl.Resource.Action}, woc.getExecutorLogOpts()...) _, err = woc.createWorkflowPod(ctx, nodeName, []apiv1.Container{*mainCtr}, tmpl, &createWorkflowPodOpts{onExitPod: opts.onExitTemplate, executionDeadline: opts.executionDeadline}) if err != nil { return woc.requeueIfTransientErr(err, node.Name) @@ -3421,7 +3421,7 @@ func (woc *wfOperationCtx) executeData(ctx context.Context, nodeName string, tem } mainCtr := woc.newExecContainer(common.MainContainerName, tmpl) - mainCtr.Command = []string{"argoexec", "data", string(dataTemplate)} + mainCtr.Command = append([]string{"argoexec", "data", string(dataTemplate)}, woc.getExecutorLogOpts()...) _, err = woc.createWorkflowPod(ctx, nodeName, []apiv1.Container{*mainCtr}, tmpl, &createWorkflowPodOpts{onExitPod: opts.onExitTemplate, executionDeadline: opts.executionDeadline, includeScriptOutput: true}) if err != nil { return woc.requeueIfTransientErr(err, node.Name) diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index 4378ef2010ae..5b772e2f33f6 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -3432,11 +3432,9 @@ func TestResolveIOPathPlaceholders(t *testing.T) { require.NoError(t, err) assert.NotEmpty(t, pods.Items, "pod was not created successfully") - assert.Equal(t, []string{ - "/var/run/argo/argoexec", "emissary", - "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, + assert.Equal(t, append(append([]string{"/var/run/argo/argoexec", "emissary"}, woc.getExecutorLogOpts()...), "--", "sh", "-c", "head -n 3 <\"/inputs/text/data\" | tee \"/outputs/text/data\" | wc -l > \"/outputs/actual-lines-count/data\"", - }, pods.Items[0].Spec.Containers[1].Command) + ), pods.Items[0].Spec.Containers[1].Command) } var outputValuePlaceholders = ` diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go index f793060d1fcf..735b6ac04e07 100644 --- a/workflow/controller/workflowpod.go +++ b/workflow/controller/workflowpod.go @@ -20,6 +20,7 @@ import ( "github.com/argoproj/argo-workflows/v3/errors" "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" + cmdutil "github.com/argoproj/argo-workflows/v3/util/cmd" "github.com/argoproj/argo-workflows/v3/util/deprecation" errorsutil "github.com/argoproj/argo-workflows/v3/util/errors" "github.com/argoproj/argo-workflows/v3/util/intstr" @@ -396,9 +397,8 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin c.Args = x.Cmd } } - c.Command = append([]string{common.VarRunArgoPath + "/argoexec", "emissary", - "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.executorLogFormat(), - "--"}, c.Command...) + execCmd := append(append([]string{common.VarRunArgoPath + "/argoexec", "emissary"}, woc.getExecutorLogOpts()...), "--") + c.Command = append(execCmd, c.Command...) } if c.Image == woc.controller.executorImage() { // mount tmp dir to wait container @@ -598,18 +598,18 @@ func substitutePodParams(pod *apiv1.Pod, globalParams common.Parameters, tmpl *w func (woc *wfOperationCtx) newInitContainer(tmpl *wfv1.Template) apiv1.Container { ctr := woc.newExecContainer(common.InitContainerName, tmpl) - ctr.Command = []string{"argoexec", "init", "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.executorLogFormat()} + ctr.Command = append([]string{"argoexec", "init"}, woc.getExecutorLogOpts()...) return *ctr } func (woc *wfOperationCtx) newWaitContainer(tmpl *wfv1.Template) *apiv1.Container { ctr := woc.newExecContainer(common.WaitContainerName, tmpl) - ctr.Command = []string{"argoexec", "wait", "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.executorLogFormat()} + ctr.Command = append([]string{"argoexec", "wait"}, woc.getExecutorLogOpts()...) return ctr } -func getExecutorLogLevel() string { - return log.GetLevel().String() +func (woc *wfOperationCtx) getExecutorLogOpts() []string { + return []string{"--loglevel", log.GetLevel().String(), "--log-format", woc.controller.executorLogFormat(), "--gloglevel", cmdutil.GetGLogLevel()} } func (woc *wfOperationCtx) createEnvVars() []apiv1.EnvVar { diff --git a/workflow/controller/workflowpod_test.go b/workflow/controller/workflowpod_test.go index 688d79fe8765..42b5b359a54a 100644 --- a/workflow/controller/workflowpod_test.go +++ b/workflow/controller/workflowpod_test.go @@ -648,6 +648,8 @@ func Test_createWorkflowPod_containerName(t *testing.T) { assert.Equal(t, common.MainContainerName, pod.Spec.Containers[1].Name) } +var emissaryCmd = []string{"/var/run/argo/argoexec", "emissary"} + func Test_createWorkflowPod_emissary(t *testing.T) { t.Run("NoCommand", func(t *testing.T) { woc := newWoc() @@ -658,26 +660,23 @@ func Test_createWorkflowPod_emissary(t *testing.T) { woc := newWoc() pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Command: []string{"foo"}}}, &wfv1.Template{}, &createWorkflowPodOpts{}) require.NoError(t, err) - assert.Equal(t, []string{"/var/run/argo/argoexec", "emissary", - "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, - "--", "foo"}, pod.Spec.Containers[1].Command) + cmd := append(append(emissaryCmd, woc.getExecutorLogOpts()...), "--", "foo") + assert.Equal(t, cmd, pod.Spec.Containers[1].Command) }) t.Run("NoCommandWithImageIndex", func(t *testing.T) { woc := newWoc() pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Image: "my-image"}}, &wfv1.Template{}, &createWorkflowPodOpts{}) require.NoError(t, err) - assert.Equal(t, []string{"/var/run/argo/argoexec", "emissary", - "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, - "--", "my-entrypoint"}, pod.Spec.Containers[1].Command) + cmd := append(append(emissaryCmd, woc.getExecutorLogOpts()...), "--", "my-entrypoint") + assert.Equal(t, cmd, pod.Spec.Containers[1].Command) assert.Equal(t, []string{"my-cmd"}, pod.Spec.Containers[1].Args) }) t.Run("NoCommandWithArgsWithImageIndex", func(t *testing.T) { woc := newWoc() pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Image: "my-image", Args: []string{"foo"}}}, &wfv1.Template{}, &createWorkflowPodOpts{}) require.NoError(t, err) - assert.Equal(t, []string{"/var/run/argo/argoexec", "emissary", - "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, - "--", "my-entrypoint"}, pod.Spec.Containers[1].Command) + cmd := append(append(emissaryCmd, woc.getExecutorLogOpts()...), "--", "my-entrypoint") + assert.Equal(t, cmd, pod.Spec.Containers[1].Command) assert.Equal(t, []string{"foo"}, pod.Spec.Containers[1].Args) }) t.Run("CommandFromPodSpecPatch", func(t *testing.T) { @@ -691,9 +690,8 @@ func Test_createWorkflowPod_emissary(t *testing.T) { require.NoError(t, err) pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Command: []string{"foo"}}}, &wfv1.Template{PodSpecPatch: string(podSpecPatch)}, &createWorkflowPodOpts{}) require.NoError(t, err) - assert.Equal(t, []string{"/var/run/argo/argoexec", "emissary", - "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, - "--", "bar"}, pod.Spec.Containers[1].Command) + cmd := append(append(emissaryCmd, woc.getExecutorLogOpts()...), "--", "bar") + assert.Equal(t, cmd, pod.Spec.Containers[1].Command) }) } @@ -747,9 +745,8 @@ func TestVolumeAndVolumeMounts(t *testing.T) { assert.Equal(t, "tmp-dir-argo", wait.VolumeMounts[1].Name) assert.Equal(t, "var-run-argo", wait.VolumeMounts[2].Name) main := containers[1] - assert.Equal(t, []string{"/var/run/argo/argoexec", "emissary", - "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, - "--", "cowsay"}, main.Command) + cmd := append(append(emissaryCmd, woc.getExecutorLogOpts()...), "--", "cowsay") + assert.Equal(t, cmd, main.Command) require.Len(t, main.VolumeMounts, 2) assert.Equal(t, "volume-name", main.VolumeMounts[0].Name) assert.Equal(t, "var-run-argo", main.VolumeMounts[1].Name)