Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Only set ARGO_TEMPLATE env for init container. #13761

Merged
merged 3 commits into from
Oct 18, 2024
Merged

refactor: Only set ARGO_TEMPLATE env for init container. #13761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0eaae26
feat: Only set `ARGO_TEMPLATE` env for init container.
jswxstw Oct 15, 2024
6aadfae
fix(test): adapt `ARGO_TEMPLATE` env changes.
jswxstw Oct 16, 2024
c758c69
fix: remove `ARGO_TEMPLATE_WITH_INPUTS_PARAMETERS` env.
jswxstw Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .spelling
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading