Skip to content

Commit

Permalink
refactor: Only set ARGO_TEMPLATE env for init container. (#13761)
Browse files Browse the repository at this point in the history 
Signed-off-by: oninowang <[email protected]>
  • Loading branch information
@jswxstw
jswxstw authored Oct 18, 2024
1 parent 415007c commit c9b1477
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 45 deletions.
2 changes: 0 additions & 2 deletions .spelling
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
5xx
8Ki
90m
ARGO_TEMPLATE
Alexandre
Alibaba
Ang
Expand Down Expand Up @@ -166,7 +165,6 @@ govaluate
gzipped
i.e.
idempotence
inputs.parameters
instantiator
instantiators
jenkins
Expand Down
9 changes: 7 additions & 2 deletions cmd/argoexec/commands/root.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,13 @@ func initExecutor() *executor.WorkflowExecutor {
}

tmpl := &wfv1.Template{}
envVarTemplateValue := os.Getenv(common.EnvVarTemplate)
if envVarTemplateValue == common.EnvVarTemplateOffloaded {
envVarTemplateValue, ok := os.LookupEnv(common.EnvVarTemplate)
// wait container reads template from the file written by init container, instead of from environment variable.
if !ok {
data, err := os.ReadFile(varRunArgo + "/template")
checkErr(err)
envVarTemplateValue = string(data)
} else if envVarTemplateValue == common.EnvVarTemplateOffloaded {
data, err := os.ReadFile(filepath.Join(common.EnvConfigMountPath, common.EnvVarTemplate))
checkErr(err)
envVarTemplateValue = string(data)
Expand Down
1 change: 0 additions & 1 deletion docs/environment-variables.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ This document outlines environment variables that can be used to customize behav
| `ARGO_AGENT_CPU_LIMIT` | `resource.Quantity` | `100m` | CPU resource limit for the agent. |
| `ARGO_AGENT_MEMORY_LIMIT` | `resource.Quantity` | `256m` | Memory resource limit for the agent. |
| `ARGO_POD_STATUS_CAPTURE_FINALIZER` | `bool` | `false` | The finalizer blocks the deletion of pods until the controller captures their status.
| `ARGO_TEMPLATE_WITH_INPUTS_PARAMETERS` | `bool` | `true` | Whether to keep inputs.parameters inside the ARGO_TEMPLATE environment variable of pods.
| `BUBBLE_ENTRY_TEMPLATE_ERR` | `bool` | `true` | Whether to bubble up template errors to workflow. |
| `CACHE_GC_PERIOD` | `time.Duration` | `0s` | How often to perform memoization cache GC, which is disabled by default and can be enabled by providing a non-zero duration. |
| `CACHE_GC_AFTER_NOT_HIT_DURATION` | `time.Duration` | `30s` | When a memoization cache has not been hit after this duration, it will be deleted. |
Expand Down
12 changes: 6 additions & 6 deletions workflow/controller/operator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1787,7 +1787,7 @@ func TestAssessNodeStatus(t *testing.T) {

func getPodTemplate(pod *apiv1.Pod) (*wfv1.Template, error) {
tmpl := &wfv1.Template{}
for _, c := range pod.Spec.Containers {
for _, c := range pod.Spec.InitContainers {
for _, e := range c.Env {
if e.Name == common.EnvVarTemplate {
return tmpl, json.Unmarshal([]byte(e.Value), tmpl)
Expand Down Expand Up @@ -1817,7 +1817,7 @@ func TestGetPodTemplate(t *testing.T) {
name: "missing template",
pod: &apiv1.Pod{
Spec: apiv1.PodSpec{
Containers: []apiv1.Container{
InitContainers: []apiv1.Container{
{
Env: []apiv1.EnvVar{},
},
Expand All @@ -1829,7 +1829,7 @@ func TestGetPodTemplate(t *testing.T) {
name: "empty template",
pod: &apiv1.Pod{
Spec: apiv1.PodSpec{
Containers: []apiv1.Container{
InitContainers: []apiv1.Container{
{
Env: []apiv1.EnvVar{
{
Expand All @@ -1846,7 +1846,7 @@ func TestGetPodTemplate(t *testing.T) {
name: "simple template",
pod: &apiv1.Pod{
Spec: apiv1.PodSpec{
Containers: []apiv1.Container{
InitContainers: []apiv1.Container{
{
Env: []apiv1.EnvVar{
{
Expand Down Expand Up @@ -7310,8 +7310,8 @@ func TestWFWithRetryAndWithParam(t *testing.T) {
ctrs := pods.Items[0].Spec.Containers
assert.Len(t, ctrs, 2)
envs := ctrs[1].Env
assert.Len(t, envs, 8)
assert.Equal(t, apiv1.EnvVar{Name: "ARGO_INCLUDE_SCRIPT_OUTPUT", Value: "true"}, envs[3])
assert.Len(t, envs, 7)
assert.Equal(t, apiv1.EnvVar{Name: "ARGO_INCLUDE_SCRIPT_OUTPUT", Value: "true"}, envs[2])
})
}

Expand Down
47 changes: 13 additions & 34 deletions workflow/controller/workflowpod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,25 +296,15 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin
pod.Spec.InitContainers[i] = c
}

envVarTemplateValue := wfv1.MustMarshallJSON(tmpl)
if os.Getenv("ARGO_TEMPLATE_WITH_INPUTS_PARAMETERS") == "false" {
tmplWithoutInputs := tmpl.DeepCopy()
// Preserve Inputs.Artifacts and clear other inputs
var artifacts []wfv1.Artifact
if len(tmplWithoutInputs.Inputs.Artifacts) > 0 {
artifacts = tmplWithoutInputs.Inputs.Artifacts
} else {
artifacts = []wfv1.Artifact{}
}
tmplWithoutInputs.Inputs = wfv1.Inputs{
Artifacts: artifacts,
}
envVarTemplateValue = wfv1.MustMarshallJSON(tmplWithoutInputs)
// simplify template by clearing useless `inputs.parameters` and preserving `inputs.artifacts`.
simplifiedTmpl := tmpl.DeepCopy()
simplifiedTmpl.Inputs = wfv1.Inputs{
Artifacts: simplifiedTmpl.Inputs.Artifacts,
}
envVarTemplateValue := wfv1.MustMarshallJSON(simplifiedTmpl)

// Add standard environment variables, making pod spec larger
envVars := []apiv1.EnvVar{
{Name: common.EnvVarTemplate, Value: envVarTemplateValue},
{Name: common.EnvVarNodeID, Value: nodeID},
{Name: common.EnvVarIncludeScriptOutput, Value: strconv.FormatBool(opts.includeScriptOutput)},
{Name: common.EnvVarDeadline, Value: woc.getDeadline(opts).Format(time.RFC3339)},
Expand All @@ -341,6 +331,7 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin

for i, c := range pod.Spec.InitContainers {
c.Env = append(c.Env, apiv1.EnvVar{Name: common.EnvVarContainerName, Value: c.Name})
c.Env = append(c.Env, apiv1.EnvVar{Name: common.EnvVarTemplate, Value: envVarTemplateValue})
c.Env = append(c.Env, envVars...)
pod.Spec.InitContainers[i] = c
}
Expand All @@ -362,7 +353,7 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin
// only to check ArchiveLocation for now, since everything else should have been substituted
// earlier (i.e. in executeTemplate). But archive location is unique in that the variables
// are formulated from the configmap. We can expand this to other fields as necessary.
for _, c := range pod.Spec.Containers {
for _, c := range pod.Spec.InitContainers {
for _, e := range c.Env {
if e.Name == common.EnvVarTemplate {
err = json.Unmarshal([]byte(e.Value), tmpl)
Expand Down Expand Up @@ -441,14 +432,12 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin
}

offloadEnvVarTemplate := false
for _, c := range pod.Spec.Containers {
if c.Name == common.MainContainerName {
for _, e := range c.Env {
if e.Name == common.EnvVarTemplate {
envVarTemplateValue = e.Value
if len(envVarTemplateValue) > maxEnvVarLen {
offloadEnvVarTemplate = true
}
for _, c := range pod.Spec.InitContainers {
for _, e := range c.Env {
if e.Name == common.EnvVarTemplate {
envVarTemplateValue = e.Value
if len(envVarTemplateValue) > maxEnvVarLen {
offloadEnvVarTemplate = true
}
}
}
Expand Down Expand Up @@ -511,16 +500,6 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin
c.VolumeMounts = append(c.VolumeMounts, volumeMountConfig)
pod.Spec.InitContainers[i] = c
}
for i, c := range pod.Spec.Containers {
for j, e := range c.Env {
if e.Name == common.EnvVarTemplate {
e.Value = common.EnvVarTemplateOffloaded
c.Env[j] = e
}
}
c.VolumeMounts = append(c.VolumeMounts, volumeMountConfig)
pod.Spec.Containers[i] = c
}
}

// Check if the template has exceeded its timeout duration. If it hasn't set the applicable activeDeadlineSeconds
Expand Down

0 comments on commit c9b1477

Please sign in to comment.