-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
fix: avoid fan out matrix task failed due to result ref #8487
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @l-qing. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@@ -782,7 +782,7 @@ func TestPipelineTask_CountCombinations(t *testing.T) { | |||
}{{ | |||
name: "combinations count is zero", | |||
matrix: &v1.Matrix{ | |||
Params: v1.Params{{}}}, | |||
Params: v1.Params{}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior is different between having no param and having a param with an empty value.
Params: v1.Params{{ | ||
Name: "GOARCH", Value: v1.ParamValue{ArrayVal: []string{"linux/amd64", "linux/ppc64le", "linux/s390x"}}, | ||
}, { | ||
Name: "version", Value: v1.ParamValue{StringVal: "$(tasks.platforms.results.str[*])"}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem to be solved this time involves referencing an array parameter within a string value.
@@ -8942,8 +8942,14 @@ spec: | |||
script: | | |||
echo "$(params.platform)" | |||
- name: b-task | |||
taskRef: | |||
name: mytask | |||
taskSpec: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid returning early due to the PipelineRunReasonCouldntGetTask
error.
@@ -3801,10 +3801,10 @@ func TestResolvePipelineRunTask_WithMatrix(t *testing.T) { | |||
name: "task with matrix - whole array results", | |||
pt: pts[2], | |||
want: &ResolvedPipelineTask{ | |||
TaskRunNames: nil, | |||
TaskRunNames: []string{"pipelinerun-pipelinetask-with-whole-array-results"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current behavior will return a definition of TaskSpec, facilitating subsequent verification.
matrix: | ||
params: | ||
- name: platform | ||
value: $(tasks.matrix-include.results.str[*]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a usage that is supported this time.
Signed-off-by: chengjoey <[email protected]>
d4c465d
to
def2c31
Compare
It seems that this usage might bypass the limitation of pipeline/pkg/apis/pipeline/v1/matrix_types.go Lines 294 to 301 in 1dd488e
|
fix #8324
Continuing development based on this PR: #8327
Changes
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>
. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes
/kind bug
1. Reproducible steps
2. Error message
invalid result reference in pipeline task "printer-matrix": unable to validate result referencing pipeline task "platforms": task spec not found
3. Analysis
a.
validateResultRef
:unable to validate result referencing pipeline task
if ptMap[ref.PipelineTask].ResolvedTask == nil || ptMap[ref.PipelineTask].ResolvedTask.TaskSpec == nil {
pipeline/pkg/reconciler/pipelinerun/resources/validate_dependencies.go
Lines 75 to 77 in 1dd488e
b.
ValidatePipelineTaskResults
:invalid result reference in pipeline task
pipeline/pkg/reconciler/pipelinerun/resources/validate_dependencies.go
Lines 31 to 36 in 1dd488e
c.
PipelineRun-Reconcile
: callValidatePipelineTaskResults
pipeline/pkg/reconciler/pipelinerun/pipelinerun.go
Lines 697 to 702 in 1dd488e
d.
pipelineRunFacts.State
: come fromresolvePipelineState
pipeline/pkg/reconciler/pipelinerun/pipelinerun.go
Lines 612 to 627 in 1dd488e
e.
resolvePipelineState
: resolvedTask -ResolvePipelineTask
pipeline/pkg/reconciler/pipelinerun/pipelinerun.go
Lines 368 to 377 in 1dd488e
f.
ResolvePipelineTask
: callCountCombinations
pipeline/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Lines 602 to 630 in 1dd488e
h1.
CountCombinations
: Calculate the number of TaskRunspipeline/pkg/apis/pipeline/v1/matrix_types.go
Lines 220 to 242 in 1dd488e
h2. Error: The calculated count is 0.
pipeline/pkg/apis/pipeline/v1/matrix_types.go
Lines 233 to 239 in 1dd488e
Because the
param.Value.StringVale
is$(tasks.platforms.results.str[*])
andparam.Value.ArrayVal
is empty.j1.
ResolvePipelineTask
: callGetNamesOfTaskRuns
pipeline/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Lines 628 to 630 in 1dd488e
j2.
GetNamesOfTaskRuns
: callgetNewRunNames
thenumberOfRuns
is 0, the resulttaskRunNames
is emptypipeline/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Lines 749 to 771 in 1dd488e
k.
ResolvePipelineTask
:setTaskRunsAndResolvedTask
has not been called.pipeline/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Lines 628 to 632 in 1dd488e
l.
setTaskRunsAndResolvedTask
:ResolvedTask
has not been set.pipeline/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Lines 641 to 662 in 1dd488e
m. So it led to the error seen at the top.