cloudfoundry / cf-deployment-concourse-tasks

Apache License 2.0
23 stars 76 forks source link

Add bosh-upload-release #96

Closed joshuatcasey closed 5 years ago

joshuatcasey commented 5 years ago

What is this change about?

The UAA team would like to have a task to upload a release, since a dedicated job in our pipeline creates the release tarball and a separate job deploys cf-deployment (currently using bosh-deploy-with-created-release). We'd like to upload the pre-made tarball.

Please provide contextual information.

N/A

Please check all that apply for this PR:

Did you update the README as appropriate for this change?

How should this change be described in release notes?

Adds task bosh-upload-release, which uploads a single release to the BOSH director.

What is the level of urgency for publishing this change?

Tag your pair, your PM, and/or team!

@cloudfoundry/cf-uaa

cfdreddbot commented 5 years ago

:white_check_mark: Hey joshuatcasey! The commit authors and yourself have already signed the CLA.

cf-gitbot commented 5 years ago

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

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

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

joshuatcasey commented 5 years ago

@davewalter @acosta11 Thanks for the feedback, see changes!

joshuatcasey commented 5 years ago

@davewalter @acosta11 things should be ready to go now.

davewalter commented 5 years ago

Hi @joshuatcasey,

I just added a few comments. Could you please take a look at update as required?

Thanks, Dave

joshuatcasey commented 5 years ago

Given the number of tweaks and updating to the latest Docker image I went ahead and rebased/squashed the commits. All feedback should be taken into account now!

davewalter commented 5 years ago

Hi @joshuatcasey,

Sorry for the back and forth. After discussing this with the team, we think that we would actually prefer to support it in the existing bosh-deploy-with-created-release task, rather than adding a new task that we would have to start supporting. How would you feel about us modifying that task to optionally accept a directory containing a release tarball instead of the actual release repository? The underlying bosh_interpolate helper function already has logic to add an ops-file to tell BOSH to upload a release from a local directory, so we think that this would work without the need for an extra task.

Let us know what you think.

Regards, Dave and @vitreuz

joshuatcasey commented 5 years ago

@davewalter and @vitreuz - makes sense to me!

acosta11 commented 5 years ago

Hi @joshuatcasey

We went ahead and wrote what we think this would look like and pushed it to a branch. The main difference you will notice is that we've tried to remain as generic as possible because the task shouldn't have an opinion on naming convention of the file itself if other teams want to use it.

Check it out at https://github.com/cloudfoundry/cf-deployment-concourse-tasks/commit/6ecf2f8a524dbbb43dc4b1009c41460896952d69 and verify it does in fact cover your use case. If so, we would like to merge from there and close this out. Leaving this open in the meantime for discussion.

Thanks, Andrew and @davewalter

joshuatcasey commented 5 years ago

@acosta11 @davewalter

I like the change in 6ecf2f8 - looks good to me!

davewalter commented 5 years ago

Rebased and merged to master as https://github.com/cloudfoundry/cf-deployment-concourse-tasks/commit/1e0a4a9c61057bc08674d8d63beb417c3c124c43

davewalter commented 5 years ago

Hi @joshuatcasey,

Just wanted to reach out to see if you have had a chance to implement this in your CI yet, and if so, whether it is working as desired.

Regards, Dave

joshuatcasey commented 5 years ago

@davewalter I think we had some trouble when supplying a tarball directory for input release, because then it's not able to retrieve the release name because release/config/final.yml doesn't exist.

https://github.com/cloudfoundry/cf-deployment-concourse-tasks/blob/7b0e1f2de4472d77618d1a68cbea4935be7033fe/bosh-deploy-with-created-release/task#L35-L36

davewalter commented 5 years ago

@davewalter I think we had some trouble when supplying a tarball directory for input release, because then it's not able to retrieve the release name because release/config/final.yml doesn't exist.

https://github.com/cloudfoundry/cf-deployment-concourse-tasks/blob/7b0e1f2de4472d77618d1a68cbea4935be7033fe/bosh-deploy-with-created-release/task#L35-L36

You're right @joshuatcasey. The only solution I can come up with off the top of my head is to change the release_tarball_name input into a more generic release_info input with a file for the release name and a separate file for the release tarball name. What do you think?