Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

server: Ensure RunPipeline api returns correct order for AllJobIds for pipelines with embedded pipeline graphs #3946

Merged
merged 6 commits into from
Sep 29, 2022

Conversation

briancain
Copy link
Member

This pull request fixes a bug where the RunPipeline API was returning the wrong order of AllJobIds based on how it constructed and sorted the full pipeline graph. This resulted in the jobstream client reading pipeline jobs out of order from the actual execution order that the jobs ran in. It also adds additional unit tests that check the expected order of job ids to steps.

Fixes #3869

briancain and others added 4 commits September 27, 2022 14:31
Include the pipeline name and step name that was already visisted.
This test verifies that when running a pipeline, it returns the job ids
in the expected order for which a pipeline will run each step.

Embedded pipeline test is next
This commit adds a unit test that reliably repros the issue:

#3869

It has two pipeline step refs back to back.
Prior to this commit, if you built a pipeline that had two neighboring
pipeline step refs, i.e. two adjacent pipeline graphs, the graph builder
would not correctly draw the relationship between steps and the graphs
child steps, resulting in job stream ids to be read out of order when a
RunPipeline was called. This commit fixes that by drawing the edge flow
properly, as well as handling the parent steps dependencies while
traveling through each pipeline ref graph.
@briancain briancain added pr/no-changelog No automatic changelog entry required for this pull request backport/0.10.x labels Sep 28, 2022
@briancain briancain removed the pr/no-changelog No automatic changelog entry required for this pull request label Sep 28, 2022
@briancain briancain changed the title Maint/server/runpipeline/jobids out of order server: Ensure RunPipeline api returns correct order for AllJobIds for pipelines with embedded pipeline graphs Sep 28, 2022
@briancain briancain added bug Something isn't working core/pipeline Issues related to the pipelines feature labels Sep 28, 2022
.changelog/3946.txt Outdated Show resolved Hide resolved
pkg/server/singleprocess/service_pipeline.go Show resolved Hide resolved
pkg/server/singleprocess/service_pipeline.go Show resolved Hide resolved
pkg/server/singleprocess/service_pipeline_test.go Outdated Show resolved Hide resolved
pkg/server/singleprocess/service_pipeline_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@paladin-devops paladin-devops left a comment

Choose a reason for hiding this comment

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

Looks awesome! Re-reading and "re-understanding" pipelineGraphFull since the initial merge of pipelines was fun 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/0.10.x bug Something isn't working core/pipeline Issues related to the pipelines feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

embedded pipelines: RunPipeline occasionally returns wrong order of JobIds
3 participants