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

Users who specify a commitish of "master" may get unexpected behavior #1401

Closed
bobcatfish opened this issue Oct 9, 2019 · 4 comments
Closed
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@bobcatfish
Copy link
Collaborator

Expected Behavior

If a user provides a PipelineResource of type git to a PipelineRun and specifies master as the commitish, they probably meant that they want the entire Pipeline to run against the lastest commit at the time the PipelineRun starts.

Actual Behavior

What will actually happen is that every Task will re-fetch master when it executes, meaning that every Task in the Pipeline could end up using a different commit.

Additional Info

A couple ideas:

  • Initialize PipelineResources in some way a the beginning of a PipelineRun
  • Do not allow the git PipelineResource to specify master (probably a bad idea)
  • Just let it be but make it very clear in our docs and intros what will happen
  • ?

In #1392 we add the ability for the PipelineResource to at least record what commit it was used with.

@afrittoli
Copy link
Member

Expected Behavior

If a user provides a PipelineResource of type git to a PipelineRun and specifies master as the commitish, they probably meant that they want the entire Pipeline to run against the lastest commit at the time the PipelineRun starts.

Actual Behavior

What will actually happen is that every Task will re-fetch master when it executes, meaning that every Task in the Pipeline could end up using a different commit.

Good point!

Additional Info

A couple ideas:

* Initialize PipelineResources in some way a the beginning of a PipelineRun

We could have a resource controller that prepares a specific version of the git repo on a PVC and serves it to the various Tasks - not an easy problem to solve though.

* Do not allow the git PipelineResource to specify master (probably a bad idea)

Well, the same problem exists with other branches as well...

* Just let it be but make it very clear in our docs and intros what will happen

+1
I think in use cases like CI/CD master will typically not be used, as the pipeline will be triggered with a very specific commit sha. However in other cases this might be an issue.

* ?

In #1392 we add the ability for the PipelineResource to at least record what commit it was used with.

Nice, this might allow tasks to clone the repo from the correct sha, but they somehow need to lookup into their parent pipeline to find the correct sha.

Alternatively could avoid re-cloning a git input resource and use the same input to all tasks, however this poses several issues:

  • some kind of shared storage is required, unless we run all Tasks on the same node using affinity
  • one Task may render the git clone "dirty"

@vdemeester vdemeester added kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Nov 7, 2019
@vdemeester vdemeester added this to the Pipelines 1.0/beta 🐱 milestone Nov 7, 2019
@vdemeester
Copy link
Member

This needs to get fixed before beta, and should be resolve, by not using PipelineResource as of today or with #1438, or with a re-design of PipelineResource, or a combinaison of those 😛

@bobcatfish
Copy link
Collaborator Author

TODO before beta release:

  1. Document this clearly
  2. Make sure our examples do not use master (since it would be a bad practice)

@bobcatfish bobcatfish removed this from the Pipelines 1.0/beta 🐱 milestone Feb 18, 2020
@bobcatfish
Copy link
Collaborator Author

With #1673 this has completely changed, going forward folks will likely be using Tasks to checkout from git so:

  1. it will be explicit when the pull is happening
  2. the results will show the actual commit that was pulled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

3 participants