-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
tproxy: E2E tests #20296
tproxy: E2E tests #20296
Conversation
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.
it's all coming together! few little nitpicks, and one question about the test assertions.
curl -LO https://go.dev/dl/go1.22.2.linux-amd64.tar.gz | ||
sudo tar -C /usr/local -xzf go1.22.2.linux-amd64.tar.gz |
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.
we really didn't already have Go installed on these boxes? I'd put this under its own little heading, even if it's only currently used for "consul-k8s".
unless you're piling these things together to be also removed in bulk after 1.4.2 is shipped?
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.
No, we don't generally need toolchains on these machines. And yeah, I was putting them all together so they're easy to find and remove once 1.4.2 ships.
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.
LGTM
Add the `consul-cni` plugin to the Linux AMI for E2E, and add a test case that covers the transparent proxy feature. Ref: #20175
After addressing code review comments:
|
In #20296 we added a Go tool chain to the AMI we use for E2E tests, so that we can build `consul-cni` for tproxy testing. This is intended to be temporary until `consul-k8s` 1.4.2 is officially released. But the Go cache from building `consul-k8s` uses up roughly 1.5GiB of space and the test machines have fairly small disks. This causes the Nomad clients to aggressively GC client allocations that stop, which breaks tests that run batch workloads and then read their logs.
In #20296 we added a Go tool chain to the AMI we use for E2E tests, so that we can build `consul-cni` for tproxy testing. This is intended to be temporary until `consul-k8s` 1.4.2 is officially released. But the Go cache from building `consul-k8s` uses up roughly 1.5GiB of space and the test machines have fairly small disks. This causes the Nomad clients to aggressively GC client allocations that stop, which breaks tests that run batch workloads and then read their logs.
Add the `consul-cni` plugin to the Linux AMI for E2E, and add a test case that covers the transparent proxy feature. Add test assertions to the Connect tests for upstream reachability Ref: #20175
In #20296 we added a Go tool chain to the AMI we use for E2E tests, so that we can build `consul-cni` for tproxy testing. This is intended to be temporary until `consul-k8s` 1.4.2 is officially released. But the Go cache from building `consul-k8s` uses up roughly 1.5GiB of space and the test machines have fairly small disks. This causes the Nomad clients to aggressively GC client allocations that stop, which breaks tests that run batch workloads and then read their logs.
Add the
consul-cni
plugin to the Linux AMI for E2E, and add a test case that covers the transparent proxy feature.Ref: #20175