Skip to content

Commit

Permalink
fix: consistently set executor log options (#12979)
Browse files Browse the repository at this point in the history
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
(cherry picked from commit 217b598)
  • Loading branch information
Anton Gilgur authored and Joibel committed Nov 22, 2024
1 parent 486d25c commit e721cfe
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 31 deletions.
8 changes: 8 additions & 0 deletions util/cmd/glog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
4 changes: 2 additions & 2 deletions workflow/controller/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion workflow/controller/artifact_gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)},
},
Expand Down
4 changes: 2 additions & 2 deletions workflow/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -3397,7 +3397,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)
Expand All @@ -3420,7 +3420,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)
Expand Down
6 changes: 2 additions & 4 deletions workflow/controller/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = `
Expand Down
14 changes: 7 additions & 7 deletions workflow/controller/workflowpod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
27 changes: 12 additions & 15 deletions workflow/controller/workflowpod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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) {
Expand All @@ -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)
})
}

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit e721cfe

Please sign in to comment.