cloudfoundry / cf-deployment-concourse-tasks

Apache License 2.0
23 stars 76 forks source link

Discussion: Merge `upload-stemcells` and `deploy` tasks #78

Closed dsabeti closed 5 years ago

dsabeti commented 5 years ago

We were recently trying to use cf-deployment-concourse-tasks to upload stemcells in our pipeline, and we came across a few pain points:

Originally, we were planning to give feedback about the errors raised in the upload-stemcells task and request that the task be modified so that you can just provide the same list of ops-files to both the deploy and upload-stemcells task.

But, as we thought about it more, we started to wonder if it made more sense for the upload-stemcells task to be subsumed into the deploy task.

The two tasks have very similar interfaces. They have same inputs, except that the deploy task has one additional input called vars-files. All of the parameters for the upload-stemcells task are also included in the deploy, except for INFRASTRUCTURE. The reason the inputs and parameters are so similar is because the two tasks are kind of similar work: generating a manifest (both tasks run bosh interpolate), and then performing a bosh command (deploy or upload-stemcells) using the completely generated manifest. This would solve our original problem (managing to distinct lists of ops-files between the two tasks) by consolidating the logic into one task.

Would it make sense to combine these two tasks? Or, at least, to add the upload-stemcells logic to the deploy task?

cc @flawedmatrix @jvshahid

cf-gitbot commented 5 years ago

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

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

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

heyjcollins commented 5 years ago

Hey @dsabeti - Great suggestion. I'll turn this into a story. Thank you! Josh

heyjcollins commented 5 years ago

Hey @dsabeti @flawedmatrix @jvshahid - Yesterday we released cf-d-ct v8.1 & v8.2 which includes bosh-deploy* tasks updated to include the upload-stemcell functionality as per this issue. Just wanted to close the loop with y'all and please let me know if the solution we provided hit the mark or not. Cheers!