-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Simple upgrade test using kubetest2 framework #10523
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cc @rifelpet I think this is the right way to do things? /assign rifelpet |
tests/e2e/scenarios/upgrade/run-test
Outdated
if [[ "${CLOUD_PROVIDER}" == "aws" ]]; then | ||
ZONES=`kops get cluster ${CLUSTER_NAME} -ojson | jq -r .spec.subnets[].zone` | ||
CLUSTER_TAG="${CLUSTER_NAME}" | ||
TEST_ARGS="${TEST_ARGS} --provider=aws --gce-zone=${ZONES[0]} --cluster-tag=${CLUSTER_TAG}" |
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.
Might be worthy of a comment that --gce-zone is actually provider agnostic (this tripped me up for a minute)
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 call - added
tests/e2e/scenarios/upgrade/run-test
Outdated
|
||
kops validate cluster | ||
|
||
kubetest2 kops ${KUBETEST2_COMMON_ARGS} --down |
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.
any reason not to just let the trap function run every time? You could remove these last 3 lines and simplify the two functions above.
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.
My thought was that we wanted to preserve the error, but actually I think that you're right and it's just over-complicating things - we want to know if down failed and there's still a lingering cluster!
2e2a4b5
to
63cf5ff
Compare
Starting very simple and hard-coded! Co-authored-by: Peter Rifel <rifelpet@users.noreply.github.com>
Thanks @rifelpet - I incorporated your change! |
👍🏻 |
Starting very simple and hard-coded!