Build: Save build artifact and publish release by taion809 · Pull Request #2633 · envoyproxy/envoy

taion809 · 2018-02-16T15:26:57Z

Description:
This pull request saves the release binary to
circleci artifacts. Additionally adds a release
script to create and publish a github release with
the linux binary attached to the github release as an asset.

A CNCF member will need to:

create a github token with repo access to cut releases and upload artifacts
Add the following environment variables to the circleci envoy project
- GITHUB_TOKEN
- GITHUB_USER
- GITHUB_REPO

Risk Level: low

Testing:
I manually tested using this github repo:
https://github.com/taion809/envoy-build-tests

fixes #2095

Signed-off-by: Nicholas Johns nicholas.a.johns5@gmail.com

Description: This pull request saves the release binary to circleci artifacts. Additionally adds a release script to create and publish a github release with the linux binary attached to the github release as an asset. Risk Level: low Testing: I manually tested using this github repo: https://github.com/taion809/envoy-build-tests Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

dnoe · 2018-02-16T16:27:53Z

Looks like there is some issue with this on CircleCI. Do you have enough info to debug?

taion809 · 2018-02-16T16:36:22Z

yep, it looks like the initial release step moves the gitconfig file, so we just need to wrap it in a test

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

dnoe · 2018-02-16T20:50:42Z

ci/do_ci.sh

+
+  if [[ -n "${TAG:-}" ]]; then
+      ghrelease release --tag "${TAG:-}" --user taion809 --repo envoy-build-tests --name "${TAG:-}"
+      ghrelease upload --tag "${TAG:-}" --user taion809 --repo envoy-build-tests --name "envoy-linux-amd64" --file "${ENVOY_SRCDIR}/build_release_stripped/envoy"


Does this need to be parameterized somehow to publish to a CNCF user? Looks like this is using your credentials somehow unless I misunderstand it.

😱

yeah it does, that should have been removed.

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

taion809 · 2018-02-16T21:51:18Z

@dnoe Updated, i've also updated the PR description with some prerequisite work

htuch · 2018-02-18T04:41:50Z

ci/do_ci.sh

  docs/publish.sh
  exit 0
+elif [[ "$1" == "github_release" ]]; then
+  if [[ ! -f "${ENVOY_SRCDIR}/build_release_stripped/envoy" ]]; then


Can you move this into a new script ci/publish_github_release.sh? There's a fair bit of logic here that would be easier to understand in isolation. In the new script, can you also add a comment at the top explicitly warning not to set set -x debug, since we can get dangerous leakage of GH creds via CI logs if we don't take this step. This hit us previously in Docker CI image push, see https://github.com/envoyproxy/envoy/blob/master/ci/build_container/docker_push.sh#L3.

htuch · 2018-02-18T04:43:26Z

ci/do_ci.sh

+  if [[ ! -f "${ENVOY_SRCDIR}/build_release_stripped/envoy" ]]; then
+      echo "could not locate envoy binary at path: ${ENVOY_SRCDIR}/build_release_stripped/envoy"
+
+      # TODO(taion809): discuss whether or not failing to publish to github warrents failing the build itself


I think misconfiguration that is this bad, i.e. we're in a completely unexpected environment, should warrant failure via exit 1.

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

htuch

LGTM, if you can address the TODOs we can ship..

htuch · 2018-02-21T16:07:28Z

ci/publish_github_release.sh

+    echo "environment variable GITHUB_TOKEN unset; cannot continue with publishing."
+
+    # TODO(taion809): discuss whether or not failing to publish to github warrents failing the build itself
+    exit 0


Yeah, please make these all exit 1, I think if we don't have the expected envs, then something has gone badly wrong and we should be notified early.

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

taion809 · 2018-02-21T16:51:03Z

@htuch addressed

from the circleci side the environment variables will need to be added prior to cutting a release

htuch

Thanks!

htuch · 2018-02-22T04:06:45Z

@taion809 this is ready to ship when we have the CircleCI changes made.

htuch · 2018-02-28T20:45:19Z

@taion809 friendly ping.

bndw · 2018-02-28T21:43:45Z

.circleci/config.yml

     steps:
       - checkout
       - run: ci/do_circle_ci.sh bazel.release
+       - run: ci/do_circle_ci.sh github_release


I think this will be run on every repository event, resulting in a release created for every merge?

We've historically handled this by adding a check in the script to only publish/release for tag events. Here's an example

this should be fixed, great catch!

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

bndw · 2018-02-28T21:58:31Z

ci/publish_github_release.sh

+    exit 1
+fi
+
+if [[ -z "${CIRCLE_TAG:-}" ]]; then


I'd move this to be the very first check since this file runs on every build. We should bail early if it's not a tag event rather than failing on unnecessary dependencies

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

bndw · 2018-02-28T22:07:14Z

ci/publish_github_release.sh

+cp /tmp/bin/linux/amd64/github-release /usr/local/bin/ghrelease
+chmod +x /usr/local/bin/ghrelease
+
+ghrelease release --tag "${CIRCLE_TAG:-}" --name "${CIRCLE_TAG:-}"


@taion809 From a cursory look, this appears to create a Github release?

Currently, I think the process is roughly as follows:

@mattklein123 manually creates a release on Github

CircleCI receives a webhook with $CIRCLE_TAG set to the tag e.g. v1.5.0

Scripts that need to do things on releases, like publish Docker images, check for the existence of $CIRCLE_TAG.

So I think this needs to be changed to add the binary to the existing Github release, rather than creating a release, otherwise we'll need to revise the release process for Envoy.

releases are done manually today through the github interface so we should only upload a file to the existing release. Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

bndw · 2018-02-28T22:47:25Z

LGTM

Once merged we'll need to do a test release to validate functionality, cc @mattklein123

htuch · 2018-03-01T21:39:58Z

@taion809 waiting for your go-ahead to merge.

taion809 · 2018-03-01T22:27:27Z

@htuch SGTM

cc @mattklein123 we need the above env var's in the envoy circleci project before the next release is cut

mattklein123 · 2018-03-02T20:15:53Z

@taion809 ACK. @htuch and others, let's leave this open until I'm back in the office in a week and a half and I can merge this and setup the creds and do some tests, etc.

mattklein123 · 2018-03-02T20:16:17Z

Assigning to myself to merge this when I'm back.

mattklein123 · 2018-03-13T19:39:03Z

@taion809 getting back to this. Am I reading this correct that this requires a bot user? Is there any way to do this using a deploy key?

taion809 · 2018-03-13T20:08:41Z

i replied in slack, but for consistency i'll put it here too

I'm not sure it's possible with deploy keys, as far as I know deploy keys only give you access to git and not the github api.

mattklein123 · 2018-03-14T19:28:13Z

OK I made a bot user. I'm going to merge this and then do a test release.

This reverts commit 19b57d8.

This reverts commit 19b57d8. Signed-off-by: Matt Klein <mklein@lyft.com>

This reverts commit 19b57d8. This reverts commit 1392066. Signed-off-by: Matt Klein <mklein@lyft.com>

* bump SHA Signed-off-by: Kuat Yessenov <kuat@google.com> * update workspace Signed-off-by: Kuat Yessenov <kuat@google.com> * no RBE Signed-off-by: Kuat Yessenov <kuat@google.com>

Description: As a follow up for PR2568, the PR maps the network_disconnected and quic exception errors. The network_changed error has been removed as this is not relevant to envoymobile. Signed-off-by: Chidera Olibie <colibie@google.com> Signed-off-by: JP Simard <jp@jpsim.com>

taion809 added 2 commits February 16, 2018 10:26

Added save_artifacts to save artifacts on ci release

7fd5126

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

wrap mv in a file test

3c28ffd

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

dnoe assigned htuch Feb 16, 2018

dnoe reviewed Feb 16, 2018

View reviewed changes

dnoe self-assigned this Feb 16, 2018

fixed github_release to validate and use envvar

73023f3

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

htuch suggested changes Feb 18, 2018

View reviewed changes

moved publishing to separate script

442f364

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

taion809 force-pushed the feat-add-linux-binaries branch from 724ae6e to 442f364 Compare February 18, 2018 16:27

Fix publish script path

7ff0f07

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

htuch suggested changes Feb 21, 2018

View reviewed changes

exit 1 when environment variables are unset

65d34d9

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

htuch previously approved these changes Feb 21, 2018

View reviewed changes

bndw reviewed Feb 28, 2018

View reviewed changes

taion809 dismissed htuch’s stale review via 2fb00b3 February 28, 2018 21:52

bugfix: ensure we only run when CIRCLE_TAG is populated

9ee83f6

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

taion809 force-pushed the feat-add-linux-binaries branch from 2fb00b3 to 9ee83f6 Compare February 28, 2018 21:53

remove unnecessary if block

4e3a215

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

bndw reviewed Feb 28, 2018

View reviewed changes

move tag check to beginning for early exit

6327141

Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

bndw reviewed Feb 28, 2018

View reviewed changes

only attach a file to an existing release

557e8ae

releases are done manually today through the github interface so we should only upload a file to the existing release. Signed-off-by: Nicholas Johns <nicholas.a.johns5@gmail.com>

bndw approved these changes Feb 28, 2018

View reviewed changes

mattklein123 assigned mattklein123 and unassigned dnoe and htuch Mar 2, 2018

mattklein123 approved these changes Mar 14, 2018

View reviewed changes

mattklein123 merged commit 19b57d8 into envoyproxy:master Mar 14, 2018

taion809 deleted the feat-add-linux-binaries branch March 14, 2018 19:47

mattklein123 added a commit that referenced this pull request Mar 14, 2018

Revert "Build: Save build artifact and publish release (#2633)"

9979511

This reverts commit 19b57d8.

mattklein123 added a commit that referenced this pull request Mar 14, 2018

Revert "Build: Save build artifact and publish release (#2633)"

c810d75

This reverts commit 19b57d8. Signed-off-by: Matt Klein <mklein@lyft.com>

mattklein123 added a commit that referenced this pull request Mar 15, 2018

Revert "Build: Save build artifact and publish release (#2633)" (#2816)

c037e0f

This reverts commit 19b57d8. This reverts commit 1392066. Signed-off-by: Matt Klein <mklein@lyft.com>

junr03 mentioned this pull request May 14, 2018

Publish Envoy binary in addition to docker containers #3374

Closed

Conversation

taion809 commented Feb 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnoe commented Feb 16, 2018

Uh oh!

taion809 commented Feb 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taion809 commented Feb 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taion809 commented Feb 21, 2018

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

htuch commented Feb 22, 2018

Uh oh!

htuch commented Feb 28, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bndw Feb 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bndw commented Feb 28, 2018

Uh oh!

htuch commented Mar 1, 2018

Uh oh!

taion809 commented Mar 1, 2018

Uh oh!

mattklein123 commented Mar 2, 2018

Uh oh!

mattklein123 commented Mar 2, 2018

Uh oh!

mattklein123 commented Mar 13, 2018

Uh oh!

taion809 commented Mar 13, 2018

Uh oh!

mattklein123 commented Mar 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

taion809 commented Feb 16, 2018 •

edited

Loading

bndw Feb 28, 2018 •

edited

Loading