e2e test for connect with consul acls #6982
Conversation
shoenig
force-pushed
the
f-e2e-consul-acls
branch
from
cba0f8c
to
d51f8c9
Compare
shoenig
changed the title
|
|
||
| ## Usage | ||
|
|
||
| The `consul-acls-manage.sh` script can be used to manipulate the Consul cluster |
There was a problem hiding this comment.
The README should probably mention that it must be run from the e2e/ directory?
| @@ -1,25 +1,57 @@ | |||
| package provisioning | |||
There was a problem hiding this comment.
Thanks for taking another crack at this file... I admit it was fairly under-developed. 😀
| sleep 1 | ||
| done | ||
|
|
||
| for linux_client in ${linux_clients}; do |
There was a problem hiding this comment.
It looks like we're not stopping/starting the Windows clients. Does this cut the Window clients out from having Consul workloads once they're activated on the servers? (That's probably ok for now but we'll want to fix that when we get the other workload tests added for Windows.)
| operatorToken := tc.createOperatorToken(policyID, f) | ||
| t.Log("created operator token:", operatorToken) | ||
|
|
||
| // === Register the Nomad job === |
There was a problem hiding this comment.
The logic you have here looks solid, but it looks like it could be generalized into e2eutil/utils.go to make what's already there more robust? It looks from your commit like what we have with require.Eventually there is brittle, so if we could reuse this that would be an across-the-board improvement.
| func (runner *LinuxRunner) Run(script string) error { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) |
There was a problem hiding this comment.
Nothing to do, just thinking out loud: We probably want to be able to pass a context to the provisioner or make this timeout configurable at some point.
| @@ -31,3 +63,22 @@ func (runner *LinuxRunner) Copy(local, remote string) error { | |||
| } | |||
|
|
|||
| func (runner *LinuxRunner) Close() {} | |||
|
|
|||
| func (runner *LinuxRunner) Logf(format string, args ...interface{}) { | |||
There was a problem hiding this comment.
Should we give LinuxRunner a testlog?
Having the NOMAD_TEST_STDOUT env var option might be nice since thats usually what we use to dump logs to stdout.
There was a problem hiding this comment.
The reason for the way I did the ssh runner was so that we'd get logs while the provisioning process was running, and not just when it was complete as one does for normal test logging in go (even with the -v flag, the logs only get dumped at the end of the run). But I'm definitely open to improvements on that!
shoenig
force-pushed
the
f-e2e-consul-acls
branch
2 times, most recently
from
b18e9c0
to
71e23f5
Compare
Provide script for managing Consul ACLs on a TF provisioned cluster for e2e testing. Script can be used to 'enable' or 'disable' Consul ACLs, and automatically takes care of the bootstrapping process if necessary. The bootstrapping process takes a long time, so we may need to extend the overall e2e timeout (20 minutes seems fine). Introduces basic tests for Consul Connect with ACLs.
This test is causing panics. Unlike the other similar tests, this one is using require.Eventually which is doing something bad, and this change replaces it with a for-loop like the other tests. Failure: === RUN TestE2E/Connect === RUN TestE2E/Connect/*connect.ConnectE2ETest === RUN TestE2E/Connect/*connect.ConnectE2ETest/TestConnectDemo === RUN TestE2E/Connect/*connect.ConnectE2ETest/TestMultiServiceConnect === RUN TestE2E/Connect/*connect.ConnectClientStateE2ETest panic: Fail in goroutine after TestE2E/Connect/*connect.ConnectE2ETest has completed goroutine 38 [running]: testing.(*common).Fail(0xc000656500) /opt/google/go/src/testing/testing.go:565 +0x11e testing.(*common).Fail(0xc000656100) /opt/google/go/src/testing/testing.go:559 +0x96 testing.(*common).FailNow(0xc000656100) /opt/google/go/src/testing/testing.go:587 +0x2b testing.(*common).Fatalf(0xc000656100, 0x1512f90, 0x10, 0xc000675f88, 0x1, 0x1) /opt/google/go/src/testing/testing.go:672 +0x91 github.com/hashicorp/nomad/e2e/connect.(*ConnectE2ETest).TestMultiServiceConnect.func1(0x0) /home/shoenig/go/src/github.com/hashicorp/nomad/e2e/connect/multi_service.go:72 +0x296 github.com/hashicorp/nomad/vendor/github.com/stretchr/testify/assert.Eventually.func1(0xc0004962a0, 0xc0002338f0) /home/shoenig/go/src/github.com/hashicorp/nomad/vendor/github.com/stretchr/testify/assert/assertions.go:1494 +0x27 created by github.com/hashicorp/nomad/vendor/github.com/stretchr/testify/assert.Eventually /home/shoenig/go/src/github.com/hashicorp/nomad/vendor/github.com/stretchr/testify/assert/assertions.go:1493 +0x272 FAIL github.com/hashicorp/nomad/e2e 21.427s
shoenig
force-pushed
the
f-e2e-consul-acls
branch
from
71e23f5
to
45920e9
Compare
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This PR introduces a way to enable Consul ACLs in our e2e environment (as provisioned with TF), and adds some basic tests for using Consul Connect from Nomad with Consul ACLs enabled.
The bootstrap script is meant to be re-usable inside a single e2e run, so future tests could also make use of it if they wish. It's very slow on the first run, and kind of slow on subsequent runs since agents need to be restarted to pick up ACL config changes. Hopefully this will all be made obsolete by
shipyardin the near future!The queue of branches to merge for connect w/ acls
master <- dev-connect-acls (tracking) <- f-connect-acl-cleanup (#6905) <- f-e2e-consul-acls (this)
Addresses #6917 (at least to the point needed for these Connect tests)