-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
[NET-11154] test: fix Envoy int tests and add container logs #21674
[NET-11154] test: fix Envoy int tests and add container logs #21674
Conversation
- name: Docker build | ||
run: docker build -t consul:local -f ./build-support/docker/Consul-Dev.dockerfile ./bin | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and above matches later nightly files for consistency
Correctly set the the version of Consul built by the `dev-build` job, which is then copied into the Consul dev image used in integration tests. This was causing failures starting sidecar proxies via `consul connect envoy` due to a mismatch between the (incorrect) Consul binary's supported Envoy versions and the (correct) Envoy version under test. Also add debug log uploads to each int test so we can more easily diagnose this sort of failure in the future, as it was entirely hidden in test output.
92f0aa2
to
a7f5962
Compare
branch-name: "release/1.17.x" | ||
branch-name: "release/1.18.x" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good find!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a ton for tracking this down and fixing it!
To call out explicitly as well: I think the best next improvement here would be to convert all of these to a reusable workflow invoked once per nightly workflow. That said, I wanted to keep scope tight for this change. Hopefully we can follow up on that soon. |
Thanks @nathancoleman and np! |
Correctly set the the version of Consul built by the `dev-build` job, which is then copied into the Consul dev image used in integration tests. This was causing failures starting sidecar proxies via `consul connect envoy` due to a mismatch between the (incorrect) Consul binary's supported Envoy versions and the (correct) Envoy version under test. Also add debug log uploads to each int test so we can more easily diagnose this sort of failure in the future, as it was entirely hidden in test output.
Correctly set the the version of Consul built by the
dev-build
job, which is then copied into the Consul dev image used in integration tests.This was causing failures starting sidecar proxies via
consul connect envoy
due to a mismatch between the (incorrect) Consul binary's supported Envoy versions and the (correct) Envoy version under test.Also add debug log uploads to each int test so we can more easily diagnose this sort of failure in the future, as it was entirely hidden in test output.
What's most unintuitive about this, is that the Consul Docker image used as a base for the tests was correct - it was only the replaced binary in that image that was wrong.
This change does not need to be backported since it's made within scheduled workflows run from
main
. Note that only 1.19 tests run on CE; the rest will exclusively affect Ent.PR Checklist