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

Publish artifacts from release branches separately #8062

Merged
merged 1 commit into from
Jun 21, 2018

Conversation

zjs
Copy link
Member

@zjs zjs commented Jun 16, 2018

By default, the build-ova.sh script used by vic-product to create the unified OVA picks the most recent build uploaded to vic-engine-builds.

Because builds from all branches are written to the same directory, this can lead to unexpected combinations of components being assembled into an OVA.

To allow downstream jobs more reliably consume desired dependencies, publish artifacts from release branches into sub-directories of a releases directory.

This alone will improve the behavior of downstream builds expecting to consume dependencies from master. With additional changes, this can be leveraged to allow push builds on release branches to behave more like push builds on master: consuming the most up-to-date build from the appropriate branch of the upstream.

@zjs zjs self-assigned this Jun 16, 2018
@zjs zjs requested review from hickeng and lcastellano June 16, 2018 05:37
@zjs zjs requested a review from a team as a code owner June 16, 2018 05:37
.drone.yml Outdated
@@ -213,7 +213,7 @@ pipeline:
event: [push, tag]
status: [success, failure]

publish-gcs-builds-on-pass:
publish-gcs-builds-on-master-push-pass:
Copy link
Member

Choose a reason for hiding this comment

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

I know it's just going to end up requiring more changes, but I'm inclined towards having master have a folder as well so structure is consistent. Do you have a feel for much of a cascade change this would be?

Copy link
Member

Choose a reason for hiding this comment

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

Given the timelines, if you think this will rathole, let me know and we can do it in stages. My inclination in that case would be to publish master in the block below as well, so that we have it in both the old location and the new and can try moving pieces over without a comprehensive change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changing where we publish artifacts from master would impact vic-product's build scripts. I like the idea of publishing in both locations to allow for a gradual transition.

.drone.yml Outdated
secrets:
- google_key
source: bundle
target: vic-engine-builds/${DRONE_BRANCH}
Copy link
Member

Choose a reason for hiding this comment

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

The jenkins-nightly-run.sh script references these builds for download. I suggest we update it to allow specifying the branch of the vic bundle as well as the build number. It doesn't currently support a subfolder here at all, so some change is needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

Copy link
Member

Choose a reason for hiding this comment

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

It depends how drone implements it, but the gsutil semantics suggest a / on the end is a useful precaution.

@zjs zjs force-pushed the topic/release-build-directory branch from fdae7f4 to 49daa78 Compare June 18, 2018 19:19
.drone.yml Outdated
secrets:
- google_key
source: bundle
target: vic-engine-builds/${DRONE_BRANCH}
Copy link
Member

Choose a reason for hiding this comment

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

It depends how drone implements it, but the gsutil semantics suggest a / on the end is a useful precaution.

@@ -23,6 +23,7 @@ ESX_67_VERSION="ob-8169922"
VC_67_VERSION="ob-8217866"

DEFAULT_LOG_UPLOAD_DEST="vic-ci-logs"
DEFAULT_VCH_BRANCH=""
Copy link
Member

Choose a reason for hiding this comment

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

We could straight up fail if VCH_BRANCH isnt' set. This requires us to upload to two locations:

${VCH_BRANCH:?Branch must be set}

I suspect we want to unify the upload/download path construction in a utility function at some point, but I've no idea if drone can use the output of commands or shell functions in the target field. Defer this enhancement from this PR for now.

@@ -45,7 +46,8 @@ testcases=("${@:-${DEFAULT_TESTCASES[@]}}")
# TODO: the version downloaded by this logic is not coupled with the tests that will be run against it. This should be altered to pull a version that matches the commit SHA of the tests
# we will be running or similar mechanism.
VCH_BUILD=${VCH_BUILD:-${DEFAULT_VCH_BUILD}}
input=$(gsutil ls -l gs://vic-engine-builds/${VIC_BINARY_PREFIX}${VCH_BUILD} | grep -v TOTAL | sort -k2 -r | head -n1 | xargs | cut -d ' ' -f 3 | cut -d '/' -f 4)
VCH_BRANCH=${VCH_BRANCH:-${DEFAULT_VCH_BRANCH}}/
input=$(gsutil ls -l gs://vic-engine-builds/${VCH_BRANCH#/}${VIC_BINARY_PREFIX}${VCH_BUILD} | grep -v TOTAL | sort -k2 -r | head -n1 | xargs | cut -d ' ' -f 3 | cut -d '/' -f 4)
Copy link
Member

Choose a reason for hiding this comment

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

A / between branch and filename rather than in the variable value? I found this construction confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

This weird pattern is to allow VCH_BRANCH to be empty (for when we're pulling a build from the top-level directory). Perhaps this script doesn't need to be backwards compatible, but it seemed safer to make it so.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this clearer?

VCH_BRANCH=${VCH_BRANCH:-${DEFAULT_VCH_BRANCH}}
input=$(gsutil ls -l gs://vic-engine-builds/${VCH_BRANCH:+${VCH_BRANCH}/}${VIC_BINARY_PREFIX}${VCH_BUILD} | grep -v TOTAL | sort -k2 -r | head -n1 | xargs | cut -d ' ' -f 3 | cut -d '/' -f 4)

@zjs zjs force-pushed the topic/release-build-directory branch 2 times, most recently from 48482d2 to 52154ab Compare June 18, 2018 21:46
@zjs zjs force-pushed the topic/release-build-directory branch 2 times, most recently from aad4834 to 18d93bd Compare June 20, 2018 17:30
By default, the build-ova.sh script used by vic-product to create the
unified OVA picks the most recent build uploaded to vic-engine-builds.

Because builds from all branches are written to the same directory,
this can lead to unexpected combinations of components being assembled
into an OVA.

To allow downstream jobs more reliably consume desired dependencies,
publish artifacts from release branches into sub-directories of a
releases directory.

This alone will improve the behavior of downstream builds expecting to
consume dependencies from master. With additional changes, this can be
leveraged to allow push builds on release branches to behave more like
push builds on master: consuming the most up-to-date build from the
appropriate branch of the upstream.

Update jenkins-nightly-run.sh to allow download of builds published to
release branch directories.

Additionally, publish artifacts from PR builds into separate bucket.
@zjs zjs force-pushed the topic/release-build-directory branch from 18d93bd to 1d12f7c Compare June 20, 2018 17:48
@zjs zjs merged commit 00d377d into vmware:master Jun 21, 2018
zjs added a commit to zjs/vic that referenced this pull request Jul 27, 2018
By default, the build-ova.sh script used by vic-product to create the
unified OVA picks the most recent build uploaded to vic-engine-builds.

Because builds from all branches are written to the same directory,
this can lead to unexpected combinations of components being assembled
into an OVA.

To allow downstream jobs more reliably consume desired dependencies,
publish artifacts from release branches into sub-directories of a
releases directory.

This alone will improve the behavior of downstream builds expecting to
consume dependencies from master. With additional changes, this can be
leveraged to allow push builds on release branches to behave more like
push builds on master: consuming the most up-to-date build from the
appropriate branch of the upstream.

Update jenkins-nightly-run.sh to allow download of builds published to
release branch directories.

Additionally, publish artifacts from PR builds into separate bucket.

(cherry picked from commit 00d377d)
zjs added a commit that referenced this pull request Jul 27, 2018
By default, the build-ova.sh script used by vic-product to create the
unified OVA picks the most recent build uploaded to vic-engine-builds.

Because builds from all branches are written to the same directory,
this can lead to unexpected combinations of components being assembled
into an OVA.

To allow downstream jobs more reliably consume desired dependencies,
publish artifacts from release branches into sub-directories of a
releases directory.

This alone will improve the behavior of downstream builds expecting to
consume dependencies from master. With additional changes, this can be
leveraged to allow push builds on release branches to behave more like
push builds on master: consuming the most up-to-date build from the
appropriate branch of the upstream.

Update jenkins-nightly-run.sh to allow download of builds published to
release branch directories.

Additionally, publish artifacts from PR builds into separate bucket.

(cherry picked from commit 00d377d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants