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.

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

check data version when accept from executor done chan #3402

Merged

Conversation

chengjoey
Copy link
Contributor

What type of this PR

Add one of the following kinds:
/kind bugfix

What this PR does / why we need it:

check data version when accept from executor done chan

Which issue(s) this PR fixes:

Specified Reviewers:

/assign @your-reviewer

ChangeLog

Language Changelog
🇺🇸 English Fix the bug that get outputs error from pre-task
🇨🇳 中文 修复了无法从前面loop类型的task获取出参

Need cherry-pick to release versions?

Add comment like /cherry-pick release/1.0 when this PR is merged.

For details on the cherry pick process, see the cherry pick requests section under CONTRIBUTING.md.

@chengjoey chengjoey added pipeline pipeline service bug labels Dec 17, 2021
@codecov
Copy link

codecov bot commented Dec 17, 2021

Codecov Report

Merging #3402 (94f2bbf) into master (53400d2) will increase coverage by 0.00%.
The diff coverage is 45.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3402   +/-   ##
=======================================
  Coverage   18.14%   18.14%           
=======================================
  Files        1411     1411           
  Lines      144748   144764   +16     
=======================================
+ Hits        26263    26273   +10     
- Misses     115857   115862    +5     
- Partials     2628     2629    +1     
Impacted Files Coverage Δ
...peline/pipengine/reconciler/taskrun/taskop/wait.go 8.91% <0.00%> (+0.66%) ⬆️
...s/pipeline/pipengine/reconciler/taskrun/taskrun.go 0.00% <0.00%> (ø)
modules/pipeline/spec/pipeline_task.go 65.97% <75.00%> (+1.27%) ⬆️

@chengjoey chengjoey force-pushed the fix/pipeline-loop-task-outputs branch from 16f30ef to 78b332e Compare December 17, 2021 08:14
@chengjoey
Copy link
Contributor Author

tested in terminus-test
image
image
all this task loop reports have result:
image
image

Copy link
Member

@sfwn sfwn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjust all related var names.

Version string
}

func (pt *PipelineTask) GenerateExecutorVersion() string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is executor version? No, executor doesn't have version.
You should use GenerateExecutorDoneChanDataVersion.

return fmt.Sprintf("%s-%d-loop-%d", CtxExecutorChVersionPrefix, pt.ID, pt.Extra.LoopOptions.LoopedTimes)
}

func (pt *PipelineTask) CheckExecutorVersion(actualVersion string) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same.

@chengjoey chengjoey force-pushed the fix/pipeline-loop-task-outputs branch from 78b332e to 19d4d07 Compare December 21, 2021 08:52
@@ -67,11 +67,12 @@ func (d *define) Create(ctx context.Context, task *spec.PipelineTask) (interface
func (d *define) Start(ctx context.Context, task *spec.PipelineTask) (interface{}, error) {

go func(ctx context.Context, task *spec.PipelineTask) {
executorVersion := task.GenerateExecutorDoneChanDataVersion()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xxx

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doneChanDataVersion

@@ -278,6 +279,26 @@ func (pt *PipelineTask) GetMetadata() apistructs.Metadata {
return pt.Result.Metadata
}

type ExecutorChanData struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExecutorDoneChanData

@chengjoey chengjoey force-pushed the fix/pipeline-loop-task-outputs branch from 19d4d07 to 94f2bbf Compare December 21, 2021 09:19
@sfwn
Copy link
Member

sfwn commented Dec 21, 2021

/approve

@erda-bot erda-bot merged commit 84cb4ce into erda-project:master Dec 21, 2021
@Effet
Copy link
Member

Effet commented Dec 21, 2021

/cherry-pick release/1.5

erda-bot pushed a commit to erda-bot/erda that referenced this pull request Dec 21, 2021
Effet pushed a commit that referenced this pull request Dec 22, 2021
Co-authored-by: chengjoey <30427474+chengjoey@users.noreply.github.com>
@chengjoey chengjoey deleted the fix/pipeline-loop-task-outputs branch April 18, 2022 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants