Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Publish svcat binaries during build #1725

Merged
merged 6 commits into from
Feb 23, 2018

Conversation

carolynvs
Copy link
Contributor

@carolynvs carolynvs commented Feb 8, 2018

  • make svcat-all builds all supported client platforms (darwin, linux, windows).
  • make svcat-for-X builds a specific platform.
  • make svcat builds for the current dev's platform.
  • make svcat-publish compiles everything and uploads the binaries.
  • Versions for svcat are tracked using svcat-vX.Y.Z tags. UPDATE: The same tags are used for both client and server. The cli uses the format that always includes a tag, so that it's clear which release you are "closest" to, e.g. v1.2.3 for official releases and v1.2.3-2-gabc123 for untagged commits.
  • Versions for service catalog continue to use vX.Y.Z.
  • I've fixed the VERSION variable to contain the tag value to account for not using annotated tags. UPDATE: Reverted to maintain current docker image tags.
  • Download from https://download.svcat.sh/cli/latest/GOOS/amd64/svcat (hosted by my team). If you want a specific version it's at https://download.svcat.sh/cli/vX.Y.Z/GOOS/amd64/svcat.

Since there are no existing versions for svcat yet I have a small hack to print v0 so that the first PR can build. We can remove that once we have svcat tags in the repo.

TODO:

  • I'm waiting on an SSL cert for that domain but should have that fixed by tomorrow. 😊
  • A secret environment variable needs to be set. Not sure if y'all prefer putting that encrypted value in the travis file or if you set it in the website.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 8, 2018
@carolynvs carolynvs changed the title Publish svcat binaries during build [WIP]Publish svcat binaries during build Feb 8, 2018
@arschles
Copy link
Contributor

arschles commented Feb 8, 2018

RE the secret, we generally put them into travis as env vars

@carolynvs carolynvs changed the title [WIP]Publish svcat binaries during build Publish svcat binaries during build Feb 14, 2018
@carolynvs
Copy link
Contributor Author

@arschles Rebased and ready for review.

Copy link
Contributor

@arschles arschles left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @carolynvs

@arschles arschles added the LGTM1 label Feb 20, 2018
@MHBauer
Copy link
Contributor

MHBauer commented Feb 20, 2018

Does this also result in PR/master merge binaries as their git sha?
I think I understand this, but getting a list of the possible outputs would be nice, I think it corresponds to the same thing as svc-cat bins.

  • a v###-git-sha-ish from master merges, (which is also available as latest?)
  • a v### from the actual tags

@carolynvs
Copy link
Contributor Author

carolynvs commented Feb 20, 2018

I tweaked how we set VERSION so that it includes the tag along with the hash. Currently we are using --always which doesn't actually include the tag just the commit hash because our tags are lightweight and not annotated*. With my change to use --tags, the tag is combined with the sha to uniquely identify the commit.

This was necessary so that svcat version prints the release (along with a hash if it wasn't built off of a tagged commit).

* UPDATE: I am confused. 😀 Looking at quay we do have images tagged this way but when I run git describe --always I never get the tag name, just the hash. So I'm not sure what why I see one thing and yet the builds are obviously working just fine today.

tagged commit

$ git checkout v0.0.1
$ git describe --always --abbrev=7 --dirty --match=v*
2f0aebc
$ git describe --tags --abbrev=7 --dirty --match=v*
v0.0.1

untagged commit

$ git checkout 0c785db
$ git describe --tags --abbrev=7 --dirty --match=v*
v0.1.7-10-g0c785db
$ git describe --always --abbrev=7 --dirty --match=v*
0c785db

Deploy Logic

  • Merge to master - nothing is pushed for svcat, the same logic for pushing to docker is executed today but the image tag will include the new format, i.e. v0.1.7-10-g0c785db.
  • Tag a commit on master with vX.Y.Z - The same logic for pushing to docker is executed, but the image tag will only be the release, and not include the sha, i.e. v0.1.7.
  • Tag a commit on master with svcat-vX.Y.Z - publish the binaries to latest and vX.Y.Z.

The idea behind latest is that we can provide a permanent link to the most recent stable release of svcat. If someone wants to install a unreleased version, I would recommend building locally.

Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

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

Thanks for the summary. Maybe we want to save this into the repo as a commit message or docs. Both the first opening message details as well the summary of behavior

LGTM

@@ -363,13 +373,30 @@ release-push-%:
$(MAKE) ARCH=$* build
$(MAKE) ARCH=$* push

# SvCat Kubectl plugin stuff
# svcat kubectl plugin
Copy link
Contributor

Choose a reason for hiding this comment

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

I dunno, I like stuff

@MHBauer MHBauer added the LGTM2 label Feb 20, 2018
Makefile Outdated
@@ -32,7 +32,8 @@ SRC_DIRS = $(shell sh -c "find $(TOP_SRC_DIRS) -name \\*.go \
-exec dirname {} \\; | sort | uniq")
TEST_DIRS ?= $(shell sh -c "find $(TOP_SRC_DIRS) -name \\*_test.go \
-exec dirname {} \\; | sort | uniq")
VERSION ?= $(shell git describe --always --abbrev=7 --dirty)
VERSION ?= $(shell git describe --tags --abbrev=7 --dirty --match=v*)
Copy link
Contributor

Choose a reason for hiding this comment

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

I worry that this will confuse people who check quay.io for what version they of Service Catalog they should run. The tag name will now look something like v0.1.8-8-gb1da783, which is very verbose and not clear at first glance what its structure is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll undo this change.

@kibbles-n-bytes
Copy link
Contributor

@carolynvs Do you mean how we have sha-tagged images and v0.X.X-tagged images in quay? That's because contrib/travis/deploy.sh swaps out VERSION for TRAVIS_TAG if it's a release.

@kibbles-n-bytes
Copy link
Contributor

kibbles-n-bytes commented Feb 20, 2018

@carolynvs So, the intent is then that we will have our "core" releases just push service catalog images, and different "svcat" releases that push the svcat binaries? If so, that seems fine, though I'm curious what other repos do in this situation.

For kubernetes/kubernetes vs kubectl, it looks like the versions are kept in strict alignment and released simultaneously for every patch; is that correct?

@carolynvs
Copy link
Contributor Author

For kubernetes/kubernetes vs kubectl, it looks like the versions are kept in strict alignment and released simultaneously for every patch; is that correct?

Yes, that's what it looks like. I originally did it this way because I wasn't confident that y'all would be okay with bumping the server version when perhaps all of the changes were in the client. If you are okay with that, then I would prefer to version them together. That way it would be easy to know that one has the proper client version to go with their server.

If you agree, I'll redo this to use the same version for both (along with tweaking the tag changes so that pushes to master result in images without any version information included to preserve existing behavior).

@carolynvs
Copy link
Contributor Author

I just added a related issue, #1752, to have svcat version print the version of both the client and the server.

@kibbles-n-bytes
Copy link
Contributor

If you agree, I'll redo this to use the same version for both (along with tweaking the tag changes so that pushes to master result in images without any version information included to preserve existing behavior).

I think I like using the same version better, though it would make things like trying to add breaking changes to the CLI more difficult. But we're probably in a stable enough state at the moment that that hopefully won't be too much of an issue.

@carolynvs
Copy link
Contributor Author

I'll update the PR today to make that change then and resist the temptation to constantly make breaking changes to svcat. 😉

@kibbles-n-bytes kibbles-n-bytes added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 22, 2018
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 22, 2018
@carolynvs carolynvs force-pushed the publish-svcat branch 3 times, most recently from d49c895 to 9c47974 Compare February 22, 2018 13:32
@carolynvs
Copy link
Contributor Author

I have updated the PR to:

  • Use the same tag for both server and client (svcat) versions. The cli uses a format that always includes a tag, so that it's clear which release you are "closest" to, e.g. v1.2.3 for official releases and v1.2.3-2-gabc123 for untagged commits.
  • The versioning and image tagging for the server docker image remains unchanged.
  • Include some of my explanations from this PR about the new make targets, and what's published during a release in the devguide.

@kibbles-n-bytes kibbles-n-bytes added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. LGTM3 and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 22, 2018
@kibbles-n-bytes
Copy link
Contributor

LGTM, just one final .travis.yml rebase and I'm happy to merge!

* svcat-all builds all supported client platforms (darwin, linux, windows)
* svcat-for-X builds a specific platform
* svcat builds for the current dev's platform
Versions for svcat are tracked using svcat-vX.Y.Z tags.
Versions for service catalog continue to use vX.Y.Z. I've fixed the
VERSION variable to contain the tag value to account for not using
annotated tags.

Since there are no existing versions for svcat yet I have a small hack
to print v0 so that the first PR can build. We can remove that once we
have tags.
* Renamed from SVCAT_VERSION to TAG_VERSION to make it clear that it's
only a formatting difference.
* The cli uses the format that always includes a tag, so that it's clear
which release you are "closest" to, e.g. v1.2.3 for official releases
and v1.2.3-2-gabc123 for untagged commits.
@carolynvs
Copy link
Contributor Author

I've rebased. The travis build needs to be retried however; it hit a transient network error.

# Verify that vendor/ is in sync with Gopkg.lock
docker run --security-opt label:disable --rm -v /home/travis/build/kubernetes-incubator/service-catalog:/go/src/github.com/kubernetes-incubator/service-catalog -v /home/travis/build/kubernetes-incubator/service-catalog/.pkg:/go/pkg --env AZURE_STORAGE_CONNECTION_STRING scbuildimage build/verify-vendor.sh
grouped write of manifest, lock and vendor: error while writing out vendor tree: failed to write dep tree: failed to export golang.org/x/crypto: https://go.googlesource.com/crypto does not exist in the local cache and fetching failed: unable to get repository: Cloning into '/go/pkg/dep/sources/https---go.googlesource.com-crypto'...
POST git-upload-pack (141 bytes)
fatal: git fetch-pack: expected ACK/NAK, got '�error: Access denied to IP 35.224.58.159'
fatal: The remote end hung up unexpectedly
: command failed: [git clone --recursive -v --progress https://go.googlesource.com/crypto /go/pkg/dep/sources/https---go.googlesource.com-crypto]: exit status 128
make: *** [verify-vendor] Error 1

@kibbles-n-bytes kibbles-n-bytes removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2018
@kibbles-n-bytes kibbles-n-bytes merged commit 62284da into kubernetes-retired:master Feb 23, 2018
@carolynvs carolynvs deleted the publish-svcat branch February 23, 2018 17:57
kibbles-n-bytes pushed a commit to kibbles-n-bytes/service-catalog that referenced this pull request Feb 23, 2018
@carolynvs
Copy link
Contributor Author

🎉 IT'S ALIVE!

n3wscott pushed a commit that referenced this pull request Feb 23, 2018
@MHBauer
Copy link
Contributor

MHBauer commented Feb 24, 2018

bumped travis. green nbuiuld.

@kibbles-n-bytes kibbles-n-bytes added this to the 0.1.9 milestone Mar 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. LGTM1 LGTM2 LGTM3 size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants