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

Add CI/testing to examples #12491

Merged
merged 28 commits into from
Sep 8, 2020
Merged

Conversation

phlax
Copy link
Member

@phlax phlax commented Aug 5, 2020

Commit Message: Add CI/testing to examples
Additional Description:

This PR incorporates the tests that i created for the envoy examples (originally here: https://github.com/phlax/envoy-examples)

It also adds a test to ensure that example configs have been added to the examples BUILD config (ref: #12256 (comment))

the tests are documented in #12874

original description - no longer accurate...

the tests are run inside a docker-in-docker container to ensure a clean environment, and to allow docker system pruning without affecting the hosts container setup.

The container user's umask is set to 027 to catch any potential permissions problems related to running envoy as non-root in containers

The tests can be run locally with ./ci/verify_examples.sh from the root of the envoy repo. This will mount the current directory as the envoy source

Alternatively you can do eg the following to test a different branch/source:

export ENVOY_REPO="-b some-other-branch https://github.com/phlax/envoy"
./ci/verify_examples.sh

In this case the build config test is skipped

Risk Level: low
Testing: yes
Docs Changes:
Release Notes:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]

@phlax
Copy link
Member Author

phlax commented Aug 5, 2020

@dio @lizan ive created this PR to add the example tests i created, and hoping it would trigger in ci etc

i added a separate job in azure pipelines but that doesnt seem to have worked.

im wondering if that is the best way to trigger this in ci - and if so - whether something needs to be added in azure to allow this job to run

@lizan
Copy link
Member

lizan commented Aug 5, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

ci/run_example_tests.sh Outdated Show resolved Hide resolved
ci/run_example_tests.sh Outdated Show resolved Hide resolved
@dio dio assigned lizan Aug 5, 2020
@phlax
Copy link
Member Author

phlax commented Aug 7, 2020

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 12491 in repo envoyproxy/envoy

@phlax
Copy link
Member Author

phlax commented Aug 7, 2020

@lizan would you mind triggering azure. I need to see if the tests are passing

@lizan
Copy link
Member

lizan commented Aug 7, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@phlax phlax force-pushed the verify-examples-reprise branch 2 times, most recently from 80c89e4 to 6fbd9db Compare August 7, 2020 08:31
@lizan
Copy link
Member

lizan commented Aug 7, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lizan
Copy link
Member

lizan commented Aug 7, 2020

btw please dont force push

@phlax
Copy link
Member Author

phlax commented Aug 7, 2020

btw please dont force push

sorry - i figured its ok on a wip pr - im testing against this branch - but ill shift to another branch

@phlax phlax changed the title [WIP] Add CI/testing to examples Add CI/testing to examples Aug 8, 2020
@phlax phlax marked this pull request as ready for review August 8, 2020 03:52
ci/verify_examples.sh Outdated Show resolved Hide resolved
ci/verify_examples.sh Outdated Show resolved Hide resolved
@phlax
Copy link
Member Author

phlax commented Aug 8, 2020

@lizan i have updated the verify_examples script, it now runs standalone, and works on azure

i have tidied the script, and dewipped the PR - i think it should be ready for review

i have left the pipeline where it is for now so that tests are triggered/immediately.

once the script itself has approval, ill move it.

im wondering if the test for configs in BUILD needs to be separated out - i think this test probably wants to run early/always

one concern - which i think is an issue with the example itself or perhaps envoy is that while the mysql tests work (db/table created etc) they are triggering protocol errors in stats

you can see the mysql issue on travis here https://travis-ci.org/github/phlax/envoy-examples/builds/715797859#L5389 - it happens on azure/locally also

ci/verify_examples.sh Outdated Show resolved Hide resolved
@phlax
Copy link
Member Author

phlax commented Aug 13, 2020

@lizan i have split the tests into verify.sh scripts in the respective examples folders

i have also added the helper scripts mentioned, and made some small improvements to the error handling - partly to compensate for moving the tests out of the scope of the error handling in verify_examples.sh

verify_examples now accepts a glob arg so you can run it with individual tests, and the tests should be runnable directly from the example folders also

this script probs needswork - but im not sure about whether the other test - the one for build configs - needs to be separated or at least runnable separately

i ran the mysql example locally and can confirm that the protocol errors occur outside of these tests when run manually

once we are certain of the format of the tests i can write a small dev doc - preferrably in a subsequent pr

@phlax
Copy link
Member Author

phlax commented Aug 13, 2020

...also - is it ok to rebase/force push as i think the conflict is blocking azure

examples/verify-common.sh Outdated Show resolved Hide resolved
Copy link
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

Thanks, the script looks good.

examples/verify-common.sh Outdated Show resolved Hide resolved
phlax added 3 commits August 20, 2020 09:46
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax force-pushed the verify-examples-reprise branch from 8443b8f to 03c89e4 Compare August 20, 2020 08:46
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax marked this pull request as draft August 21, 2020 15:04
phlax added 3 commits August 22, 2020 11:15
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax marked this pull request as ready for review August 22, 2020 10:16
@phlax
Copy link
Member Author

phlax commented Aug 22, 2020

The example tests have now been moved to after the docker build, and i think are using the correct images.

Im wondering if publishing the docker images should be moved after these tests. If these tests fail - only on master, for some reason - you would not find out until after the images had been published

examples/verify-common.sh Outdated Show resolved Hide resolved
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
@sunjayBhatia
Copy link
Member

cc @envoyproxy/windows-dev might be interesting to get Windows coverage of these as well for more confidence etc.

@phlax
Copy link
Member Author

phlax commented Sep 1, 2020

@lizan are there any further changes required to land this ?

Copy link
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great.

Comment on lines 174 to 187
- bash: |
docker load < $(Build.StagingDirectory)/docker/envoy-docker-images.tar.xz
images=($(docker image list --format "{{.Repository}}"))
tags=($(docker image list --format "{{.Tag}}"))
for i in "${!images[@]}"; do
if [[ "${images[i]}" =~ "envoy" ]]; then
docker tag ${images[$i]}:${tags[$i]} ${images[$i]}:latest
fi
done
docker images
sudo apt-get update -y
sudo apt-get install -y -qq --no-install-recommends redis-tools
export DOCKER_NO_PULL=1
ci/verify_examples.sh
Copy link
Member

Choose a reason for hiding this comment

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

Can you split this to a bash file?

Copy link
Member Author

@phlax phlax Sep 7, 2020

Choose a reason for hiding this comment

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

moved to do_ci.sh - lmk if you want it somewhere else

Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Member Author

phlax commented Sep 7, 2020

@lizan should be ready for review again

@lizan lizan merged commit 69a09f4 into envoyproxy:master Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants