-
Notifications
You must be signed in to change notification settings - Fork 9
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 tool for e2e test cluster setup and cleanup #313
Conversation
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
f652a1f
to
59edf00
Compare
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, left just a few cosmetic comments
cmd/e2e-cluster/commands/setup.go
Outdated
cmd.Flags().StringArrayVarP(&images, "image", "i", cluster.DefaultE2eClusterConfig().Images, | ||
"additional image to pre-load in the cluster") |
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.
Does cobra handle array args as comma-separated list, or as specify the flag multiple times?
I never remember which is which, so I would suggest to specify this in the help message.
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 point. It supports both. I will add this to the comment.
|
||
> Note: if you are testing changes in the agent, be sure you generate the image and [upload it to the cluster](#building-the-xk6-disruptor-agent-image) using kind | ||
|
||
When you don't longer needs the cluster, you can delete it using kind: | ||
When you don't longer needs the cluster, you can delete it using the [e2e-cluster tool](#e2e-cluster-tool) or `kind`: |
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.
Double space
When you don't longer needs the cluster, you can delete it using the [e2e-cluster tool](#e2e-cluster-tool) or `kind`: | |
When you don't longer needs the cluster, you can delete it using the [e2e-cluster tool](#e2e-cluster-tool) or `kind`: |
This behavior is controlled by passing the `WithKeepOnFail` option when creating the namespace or by setting `E2E_KEEPONFAIL=true` in the environment when running an e2e test. |
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.
Other env vars are set to 0
or 1
, but this one we set it to true
. IIRC both are okay as we use strconv.ParseBool
which accepts both, but I think it would be good to stick to one or the other in the examples.
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 catch. It should also be 1
.
97f4e36
to
e32ca86
Compare
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
e32ca86
to
1af2329
Compare
Description
Fixes #311
Checklist:
make lint
) and all checks pass.make test
) and all tests pass.make e2e-xxx
foragent
,disruptors
,kubernetes
orcluster
related changes)