fix: cannot access HTTP template's outputs #7200

book987 · 2021-11-10T08:59:23Z

fix #7199

Signed-off-by: book987 book87987book@gmail.com

Don't bother creating a PR until you've done this:

Run make pre-commit -B to fix codegen, lint, and commit message problems.

Create your PR as a draft.

Your PR needs to pass the required checks before it can be approved. If the check is not required (e.g. E2E tests) it
does not need to pass.
Once required tests have passed, you can make it "Ready for review".
Say how how you tested your changes. If you changed the UI, attach screenshots.

Tips:

If changes were requested, and you've made them, then dismiss the review to get it looked at again.
Add you organization to USERS.md if you like.
You can ask for help!

Signed-off-by: book987 <book78987book@gmail.com>

alexec · 2021-11-10T15:49:36Z

🥇

sarabala1979

LGTM

sarabala1979 · 2021-11-10T16:51:56Z

@book987 Can you fix the lint issue?

alexec · 2021-11-10T16:57:45Z

looks like lint job is timing out, if this passes lint locally, it probably is transient problem

alexec · 2021-11-10T17:50:57Z

I think we need to increase the lint timeout:

argo-workflows/.golangci.yml

Line 4 in 57d894c

timeout: 5m

This could be fixed in this PR.

test/e2e/functional_test.go

book987 · 2021-11-11T09:24:40Z

I think we need to increase the lint timeout:

argo-workflows/.golangci.yml

Line 4 in 57d894c

timeout: 5m

This could be fixed in this PR.

Lint passed, I also increase timeout to 8m in this PR.

alexec · 2021-11-12T16:56:46Z

Test failure:

Checking expectation http-outputs-cqhc2
1040
http-outputs-cqhc2 : Succeeded 
1041
    functional_test.go:825: 
1042
        	Error Trace:	functional_test.go:825
1043
        	            				then.go:66
1044
        	            				then.go:41
1045
        	            				functional_test.go:823
1046
        	Error:      	Expected value not to be nil.
1047
        	Test:       	TestFunctionalSuite/TestHTTPOutputs
1048
=== FAIL: FunctionalSuite/TestHTTPOutputs

Signed-off-by: book987 <book78987book@gmail.com>

book987 · 2021-11-15T06:50:25Z

Test failure:

Checking expectation http-outputs-cqhc2
1040
http-outputs-cqhc2 : Succeeded 
1041
    functional_test.go:825: 
1042
        	Error Trace:	functional_test.go:825
1043
        	            				then.go:66
1044
        	            				then.go:41
1045
        	            				functional_test.go:823
1046
        	Error:      	Expected value not to be nil.
1047
        	Test:       	TestFunctionalSuite/TestHTTPOutputs
1048
=== FAIL: FunctionalSuite/TestHTTPOutputs

Hi, this e2e test passed on my laptop, I need some help 😢 Is there any way we can get the full workflow yaml?

/usr/local/Cellar/go/1.16/libexec/bin/go tool test2json -t /private/var/folders/c7/xsy5l1m919sbbzdlvnk40rlm0000gn/T/___TestHTTPOutputs_in_github_com_argoproj_argo_workflows_v3_test_e2e -test.v -test.paniconexit0 -test.run ^\QTestFunctionalSuite\E$/^\QTestHTTPOutputs\E$ -testify.m ^TestHTTPOutputs$
=== RUN   TestFunctionalSuite
time="2021-11-15T14:37:47+08:00" level=info msg="config map" name=workflow-controller-configmap
--- PASS: TestFunctionalSuite (10.26s)
=== RUN   TestFunctionalSuite/TestHTTPOutputs
Submitting workflow  http-outputs-
Waiting 30s for workflow metadata.name=http-outputs-g8wcb
 ? http-outputs-g8wcb Workflow 0s      

 ● http-outputs-g8wcb   Workflow 0s      
 └ ● http-outputs-g8wcb DAG      0s      
 └ ◷ http               HTTP     0s      

 ● http-outputs-g8wcb   Workflow 0s      
 └ ● http-outputs-g8wcb DAG      0s      
 └ ✔ http               HTTP     0s      

 ● http-outputs-g8wcb   Workflow 0s      
 └ ● http-outputs-g8wcb DAG      0s      
 └ ✔ http               HTTP     0s      
 └ ◷ echo               Pod      0s      

 ✔ http-outputs-g8wcb   Workflow 10s     
 └ ✔ http-outputs-g8wcb DAG      10s     
 └ ✔ http               HTTP     0s      
 └ ✔ echo               Pod      1s      

Condition "to be Succeeded" met after 10s
Checking expectation http-outputs-g8wcb
http-outputs-g8wcb : Succeeded 
=== PASS: FunctionalSuite/TestHTTPOutputs
    --- PASS: TestFunctionalSuite/TestHTTPOutputs (10.24s)
PASS

Process finished with exit code 0

alexec · 2021-11-15T18:02:22Z

I wonder if test is not using executor image built?

book987 · 2021-11-16T05:47:23Z

I wonder if test is not using executor image built?

You're right, I found this in agent's log, it still put the result in outputs.parameters, that means CI is not using executor image built I think.

https://github.com/argoproj/argo-workflows/runs/4208211529?check_suite_focus=true#step:15:3186

alexec · 2021-11-16T16:46:42Z

That's is odd. This works fine for other builds. I wonder if it fails on forks?

alexec · 2021-11-16T16:55:54Z

Ok. So HTTP templates are run by the agent. Perhaps the agent does not have correct pull policy.

alexec · 2021-11-16T16:56:34Z

Agent does not respect pull policy:

argo-workflows/workflow/controller/agent.go

Line 95 in eb9a428

Spec: apiv1.PodSpec{

Can you please fix that in this PR too?

Signed-off-by: book987 <book78987book@gmail.com>

book987 · 2021-11-17T05:29:52Z

Agent does not respect pull policy:

argo-workflows/workflow/controller/agent.go

Line 95 in eb9a428

Spec: apiv1.PodSpec{

Can you please fix that in this PR too?

awesome! fixed.

alexec · 2021-11-17T17:05:40Z

test/e2e/testdata/http-outputs.yaml

+kind: Workflow
+metadata:
+  generateName: http-outputs-
+  labels:


please delete all the labels, not needed

alexec · 2021-11-17T17:06:14Z

workflow/executor/agent.go

@@ -133,7 +134,7 @@ func (ae *AgentExecutor) executeHTTPTemplate(ctx context.Context, tmpl wfv1.Temp
 		return nil, err
 	}
 	outputs := &wfv1.Outputs{}
-	outputs.Parameters = append(outputs.Parameters, wfv1.Parameter{Name: "result", Value: wfv1.AnyStringPtr(response)})


@sarabala1979 is this a problem?

Signed-off-by: book987 <book78987book@gmail.com>

book987 · 2021-11-22T04:19:44Z

Hi, could you please take another look @alexec @sarabala1979

alexec · 2021-11-22T16:37:16Z

I've requested @sarabala1979 thoughts on this.

Bala - can you please add to you TODO list?

sarabala1979 · 2021-11-22T18:32:25Z

workflow/executor/agent.go

@@ -133,7 +134,7 @@ func (ae *AgentExecutor) executeHTTPTemplate(ctx context.Context, tmpl wfv1.Temp
 		return nil, err
 	}
 	outputs := &wfv1.Outputs{}
-	outputs.Parameters = append(outputs.Parameters, wfv1.Parameter{Name: "result", Value: wfv1.AnyStringPtr(response)})
+	outputs.Result = pointer.StringPtr(response)


This is breaking change. it may break existing workflows. Please revert it. We did as a parameter so downline step can refer to it. But now it is too late to change.

I've try access HTTP template's output with steps and getting same error Internal Server Error: templates.main.steps failed to resolve {{steps.good.outputs.parameters.result}}, in v3.2.3 and v3.2.4. So looks like no one can use it's output in downline step currently. So this isn't a breaking change I think.

I rarely use steps, if I tested it in wrong way pls forgive me.

sarabala1979

This is breaking change. it may break existing workflows. Please revert it. We did as a parameter so the downline step can refer to it. But now it is too late to change.

alexec

LGTM. But need sync with master.

sarabala1979

LGMT

Signed-off-by: book987 <book78987book@gmail.com>

book987 · 2021-11-24T06:34:09Z

LGTM. But need sync with master.

sync completed.

simster7 · 2021-12-02T18:55:21Z

@alexec Looks like it needs another review from you

Signed-off-by: book987 <book78987book@gmail.com>

book987 force-pushed the dev branch from c3003e3 to 52b6f93 Compare November 10, 2021 09:05

book987 changed the title ~~fix: cannot access HTTP template's outputs && cannot connect to HTTP node in DAG~~ fix: cannot access HTTP template's outputs Nov 10, 2021

fix: cannot access HTTP template's outputs

0d98ebe

Signed-off-by: book987 <book78987book@gmail.com>

book987 force-pushed the dev branch from 52b6f93 to 0d98ebe Compare November 10, 2021 09:45

alexec approved these changes Nov 10, 2021

View reviewed changes

alexec enabled auto-merge (squash) November 10, 2021 15:49

sarabala1979 approved these changes Nov 10, 2021

View reviewed changes

simster7 reviewed Nov 10, 2021

View reviewed changes

test/e2e/functional_test.go Outdated Show resolved Hide resolved

simster7 disabled auto-merge November 10, 2021 20:05

book987 force-pushed the dev branch from 1a4c42c to 764f62a Compare November 11, 2021 08:17

fix: add e2e test

31e9d37

Signed-off-by: book987 <book78987book@gmail.com>

book987 force-pushed the dev branch from 764f62a to 31e9d37 Compare November 15, 2021 06:14

book987 added 2 commits November 17, 2021 12:41

fix: agent doesn't respect executor pull policy

4cb897e

Signed-off-by: book987 <book78987book@gmail.com>

fix: lint

51523e8

Signed-off-by: book987 <book78987book@gmail.com>

alexec requested changes Nov 17, 2021

View reviewed changes

fix: remove useless line

b38f446

Signed-off-by: book987 <book78987book@gmail.com>

alexec assigned sarabala1979 Nov 22, 2021

sarabala1979 reviewed Nov 22, 2021

View reviewed changes

sarabala1979 requested changes Nov 22, 2021

View reviewed changes

alexec requested changes Nov 23, 2021

View reviewed changes

sarabala1979 approved these changes Nov 23, 2021

View reviewed changes

book987 added 2 commits November 24, 2021 12:45

Merge branch 'master' into dev

ca35cd3

fix: lint

1033b8f

Signed-off-by: book987 <book78987book@gmail.com>

book987 force-pushed the dev branch from d8e7430 to 1033b8f Compare November 24, 2021 05:50

simster7 requested a review from alexec December 2, 2021 18:55

simster7 approved these changes Dec 2, 2021

View reviewed changes

alexec approved these changes Dec 2, 2021

View reviewed changes

alexec merged commit cb8c063 into argoproj:master Dec 2, 2021

sarabala1979 mentioned this pull request Dec 15, 2021

v3.2.5 Release #7414

Closed

73 tasks

sarabala1979 pushed a commit that referenced this pull request Dec 15, 2021

fix: cannot access HTTP template's outputs (#7200)

5a472dd

Signed-off-by: book987 <book78987book@gmail.com>

sarabala1979 mentioned this pull request Mar 1, 2022

v3.2.9 Cherry-pick #8043

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: cannot access HTTP template's outputs #7200

fix: cannot access HTTP template's outputs #7200

book987 commented Nov 10, 2021 •

edited

Loading

alexec commented Nov 10, 2021

sarabala1979 left a comment

sarabala1979 commented Nov 10, 2021

alexec commented Nov 10, 2021

alexec commented Nov 10, 2021 •

edited

Loading

book987 commented Nov 11, 2021 •

edited

Loading

alexec commented Nov 12, 2021

book987 commented Nov 15, 2021

alexec commented Nov 15, 2021

book987 commented Nov 16, 2021 •

edited

Loading

alexec commented Nov 16, 2021

alexec commented Nov 16, 2021

alexec commented Nov 16, 2021

book987 commented Nov 17, 2021

alexec Nov 17, 2021

book987 Nov 18, 2021

alexec Nov 17, 2021

book987 commented Nov 22, 2021

alexec commented Nov 22, 2021

sarabala1979 Nov 22, 2021

book987 Nov 22, 2021

sarabala1979 Nov 23, 2021

sarabala1979 left a comment

alexec left a comment

sarabala1979 left a comment

book987 commented Nov 24, 2021

simster7 commented Dec 2, 2021

fix: cannot access HTTP template's outputs #7200

fix: cannot access HTTP template's outputs #7200

Conversation

book987 commented Nov 10, 2021 • edited Loading

alexec commented Nov 10, 2021

sarabala1979 left a comment

Choose a reason for hiding this comment

sarabala1979 commented Nov 10, 2021

alexec commented Nov 10, 2021

alexec commented Nov 10, 2021 • edited Loading

book987 commented Nov 11, 2021 • edited Loading

alexec commented Nov 12, 2021

book987 commented Nov 15, 2021

alexec commented Nov 15, 2021

book987 commented Nov 16, 2021 • edited Loading

alexec commented Nov 16, 2021

alexec commented Nov 16, 2021

alexec commented Nov 16, 2021

book987 commented Nov 17, 2021

alexec Nov 17, 2021

Choose a reason for hiding this comment

book987 Nov 18, 2021

Choose a reason for hiding this comment

alexec Nov 17, 2021

Choose a reason for hiding this comment

book987 commented Nov 22, 2021

alexec commented Nov 22, 2021

sarabala1979 Nov 22, 2021

Choose a reason for hiding this comment

book987 Nov 22, 2021

Choose a reason for hiding this comment

sarabala1979 Nov 23, 2021

Choose a reason for hiding this comment

sarabala1979 left a comment

Choose a reason for hiding this comment

alexec left a comment

Choose a reason for hiding this comment

sarabala1979 left a comment

Choose a reason for hiding this comment

book987 commented Nov 24, 2021

simster7 commented Dec 2, 2021

book987 commented Nov 10, 2021 •

edited

Loading

alexec commented Nov 10, 2021 •

edited

Loading

book987 commented Nov 11, 2021 •

edited

Loading

book987 commented Nov 16, 2021 •

edited

Loading