cloudfoundry / cf-deployment-concourse-tasks

Apache License 2.0
23 stars 76 forks source link

Allow submodule resource to be input of bosh-deploy-with-created-release task #37

Closed Samze closed 6 years ago

Samze commented 6 years ago

At the moment the bosh-deploy-with-created-release task takes a release, a submodule path and a branch name.

The flaw of this strategy is that the task takes the latest HEAD of the branch and does not allow the author to control the exact commit that the release is built with.

We want to commit to a submodule, run tests, build a release from the submodule and deploy it.

Example problem scenario:

  1. Pair A commit & push to the branch of the submodule.
  2. The pipeline runs unit tests against the submodule, they pass.
  3. The pipeline triggers the bosh-deploy-with-created-release task.
  4. Pair B also commit & push to the branch of the submodule.
  5. The pipeline job fetches the branch from git, building the release with Pair Bs commit, not Pair As.

In an ideal world, create-release could take the submodule as a input resource, and then instead of checking out from a branch, it creates the release from the input. This would allow us to use concourse passing of resources to control this flow.

Thanks, Sam & @ablease

cf-gitbot commented 6 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/152145354

The labels on this github issue will be updated when the story is started.

dsabeti commented 6 years ago

Hey @Samze, I think you and I have discussed this over Slack before. It sounds like a reasonable change to make. Unfortunately, we don't have a ton of bandwidth to address it right now. How urgently do you want this?

As for design, I think it might make sense to make an entirely new concourse task for this -- something like bosh-deploy-with-updated-submodule or something like that. That way users who don't need to bump a submodule don't need to provide a dummy input.

dsabeti commented 6 years ago

I'm going to close this issue now that we've merged the PR. Let me know if there's anything else.