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.

Sign up for GitHub

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

Jump to bottom

Automatic dockerhub builds #408

Merged
merged 9 commits into from
Feb 7, 2017
Merged

Automatic dockerhub builds #408

merged 9 commits into from
Feb 7, 2017

Conversation

junr03
Copy link
Member

@junr03 junr03 commented Feb 1, 2017 •
edited
Loading

@lyft/network-team

I have added two new scripts to CI:

  1. ci/docker_push.sh: this file creates a lyft/envoy:latest image that is used by script 2. and gets push to dockerhub if the current branch is master. This will cause our lyft/envoy:latest image to be updated every time a PR is merged to master.
  2. ci/verify_examples.sh: this file builds our examples and verifies that all the containers run. This is the way I devised to verify that our examples build and run on every PR. The grpc-bridge example is a little convoluted because travis does not allow multi-lingual environments so I had to install go, and setup the project on a gopath.

Both scripts are part of the script portion of CI so that their failure causes test failure and people notice them. Both scripts only run on the normal env.

@junr03 junr03 changed the title Test Automatic dockerhub builds Feb 1, 2017
mattklein123
mattklein123 reviewed Feb 1, 2017
View reviewed changes
examples/Dockerfile-envoy-image Outdated
@@ -4,3 +4,6 @@ ADD build/source/exe/envoy /usr/local/bin/envoy
RUN apt-get update && apt-get install -y \
curl \
python-pip
RUN objcopy --only-keep-debug /usr/local/bin/envoy /usr/local/bin/envoy.dbg
Copy link
Member

Choose a reason for hiding this comment

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

For now I would just strip and drop the debug symbols to make the image smaller. If people want the debug symbols we can deal with that later. Also, do we need curl and python above? Can that be deleted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, dropping debug. The curl and python above are needed in the examples. I could move their addition to the examples' docker file. If we want that, then I would move this Dockerfile to ci/; at that point this dockerfile would belong there rather than here. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah let's keep this as clean/simple as possible.

rshriram
rshriram reviewed Feb 2, 2017
View reviewed changes
ci/docker_push.sh Outdated
then
# docker login -e $DOCKER_EMAIL -u $DOCKER_USERNAME -p $DOCKER_PASSWORD
export TAG=`if [ "$TRAVIS_BRANCH" == "master" ]; then echo "latest"; else echo $TRAVIS_BRANCH; fi`
echo "TAG is $TAG"
Copy link
Member

Choose a reason for hiding this comment

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

Any thoughts on tagging based on semantic versioning?
i.e., master branch can be "unstable", while a.b.c could be tagged with tags: envoy:a, envoy:a.b, envoy:a.b.c, and envoy:latest pointing to the a.b.c
However, if you are maintaining 1.0.0 and 1.1.0 branch for internal purposes, then please disregard the comment above.

Copy link
Member

Choose a reason for hiding this comment

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

The 1.0.0 and 1.1.0 stuff was copied from somewhere else. We will delete that. I have no preference on whether we call master unstable or latest. Whatever folks want is fine.

I think for now, as long as we are building on every master commit, we will end up manually tagging each release, and can also point latest at that release. (Need docs on this, and eventually potentially a script for doing an official release).

mattklein123
mattklein123 reviewed Feb 2, 2017
View reviewed changes
ci/Dockerfile-envoy-image
@@ -2,5 +2,6 @@ FROM ubuntu:14.04

ADD build/source/exe/envoy /usr/local/bin/envoy
RUN apt-get update && apt-get install -y \
curl \
python-pip
build-essential
Copy link
Member

Choose a reason for hiding this comment

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

what is this? do we need it?

Copy link
Member

Choose a reason for hiding this comment

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

just for strip?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah just for strip. I was trying to find if it could be installed individually but didn't find an alt package for it.

mattklein123
mattklein123 reviewed Feb 2, 2017
View reviewed changes
ci/docker_push.sh Outdated
#!/bin/bash
set -ev
want_push='false'
for branch in "master" "1.0.0" "1.1.0"
Copy link
Member

Choose a reason for hiding this comment

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

we don't do versioned branches right now, let's just delete 1.0.0 and 1.1.0

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that's going away

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially I thought these would work for our tagged releases but then I realized this would require us maintaining actual release branches not just having a tagged release.

@junr03 junr03 closed this Feb 6, 2017
@junr03 junr03 reopened this Feb 6, 2017
@junr03 junr03 mentioned this pull request Feb 6, 2017
Closed
mattklein123
mattklein123 reviewed Feb 6, 2017
View reviewed changes
ci/build_container/update_build_container.sh Outdated
read -r -p "Are you on master and have a clean branch? [y/N] " response
if [[ $response =~ ^([yY][eE][sS]|[yY])$ ]]
then
read -r -p "What SHA do you want to tag the envoy-build with? [y/N] " tag
Copy link
Member

Choose a reason for hiding this comment

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

can we just read this from git rev-parse and not ask the user

Copy link
Member Author

Choose a reason for hiding this comment

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

cool, I didn't know about this.

mattklein123
mattklein123 reviewed Feb 6, 2017
View reviewed changes
ci/build_container/update_build_container.sh Outdated
eval $(docker-machine env default)
docker build --rm -t lyft/envoy-build:$tag .
docker login -u $DOCKER_USERNAME -p $DOCKER_PASSWORD
docker push lyft/envoy-build:$tag
Copy link
Member

Choose a reason for hiding this comment

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

can we also tag latest, better for dev

Copy link
Member Author

Choose a reason for hiding this comment

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

done

mattklein123
mattklein123 reviewed Feb 7, 2017
View reviewed changes
ci/build_container/update_build_container.sh Outdated
read -r -p "Do you have master checked out with most recent changes? [y/N] " response
if [[ $response =~ ^([yY][eE][sS]|[yY])$ ]]
then
TAG="$(git rev-parse origin/master)"
Copy link
Member

Choose a reason for hiding this comment

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

this should be master, not origin/master, as your local my not be up to date and that is what you are building.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, that way there is no divergence between the tag and what was actually built.

@junr03 junr03 merged commit dd56c54 into master Feb 7, 2017
@junr03 junr03 deleted the automatic-builds branch February 7, 2017 17:55
@mattklein123 mattklein123 mentioned this pull request Feb 8, 2017
Merged
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
@qiwzhang
Added source.ip port for http. (envoyproxy#408)
dbdf74c
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
@JimmyCYJ
Tcp filter report delta (envoyproxy#408)
dbd2cde 
Automatic merge from submit-queue.

Tcp filter report delta

**What this PR does / why we need it**: Let MixerClient send periodical reports for long TCP connections. Use config TcpClientConfig::report_interval to set report interval. If report_interval is not set, or report_interval is not correct, then use 10 seconds as default interval.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #[539](https://github.com/istio/proxy/issues/539)

**Special notes for your reviewer**:

**Release note**:

```release-note
```
PiotrSikora pushed a commit to istio/envoy that referenced this pull request Feb 24, 2020
@jplevyak
Switch example to use v8. (envoyproxy#408)
d6ebd63 
Signed-off-by: John Plevyak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rshriram rshriram rshriram left review comments

@mattklein123 mattklein123 mattklein123 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@junr03 @mattklein123 @rshriram