[JUJU-1209] Added a bash-based version of the unregister-controller test. #14538
Conversation
tests/suites/controller/expect-scripts/juju-change-user-password.exp
Outdated
Show resolved
Hide resolved
|
|
||
| set timeout 10 | ||
| set controller_name [lindex $argv 1] | ||
| spawn juju login -u admin -c {*}$argv |
There was a problem hiding this comment.
this syntax seems a bit hard to understand, but I have the feeling we'll get used to it.
There was a problem hiding this comment.
Backup/restore acoounts.yaml file helped to avoid using expect so we can postpone this tool introduction
|
|
||
| controller_name=$(juju controllers --format=json | jq -r '."current-controller"') | ||
|
|
||
| echo "Change admin password" |
There was a problem hiding this comment.
At a high level, why is our "unregister test suite" worrying about change user password and login? (IOW, while this may be a faithful representation of our existing test suite, and it is great to port across, we should take a critical view and consider if we should be testing this behavior as part of this test.)
Maybe it should, as you test that you can login again to a controller that you have unregistered. (presumably this is testing that the controller hasn't actually been destroyed, just removed from local info?)
There was a problem hiding this comment.
Backup/restore acoounts.yaml file helped to avoid using juju login (and expect tool)
| echo "Unregister controller" | ||
| juju unregister --yes "${controller_name}" | ||
|
|
||
| juju controllers --format=json | jq -r ".\"controllers\".\"${controller_name}\"" | check null |
There was a problem hiding this comment.
I think a comment here with:
# if you unregister the active controller, it should ensure that the current active controller is reset to empty
There was a problem hiding this comment.
I do have a concern that this check doesn't really check what I think the behavior of unregister is meant to be.
The meaning of juju unregister is not 'I have unset the default controller'. The meaning is that "I no longer know who $controller is", whether they were the default or not.
So I'd rather see a test of:
juju controllers and that before the unregister it lists that the named controller is there, and after the unregister it no longer does.
I think it is fine to then also include "and if the controller that was removed was the default controller, we don't leave a default value that points to something that is now unknown".
But we are missing the bit that says "this is no longer a known controller".
There was a problem hiding this comment.
Got it,
I've added:
- a check that the named controller is in
juju controllersoutput before the unregister - a check that the default controller is not equal to the unregistered one after
juju unregister.
tests/main.sh
Outdated
| @@ -227,7 +227,7 @@ fi | |||
| echo "" | |||
|
|
|||
| echo "==> Checking for dependencies" | |||
| check_dependencies curl jq yq shellcheck | |||
| check_dependencies curl jq yq shellcheck expect | |||
There was a problem hiding this comment.
README.md also needs to be updated:
## Getting started
Before running tests, you'll need to install `jq`, `yq`, `shellcheck` and `golangci-lint`:
sudo snap install jq
sudo snap install yq
sudo snap install shellcheck
go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.44.2There was a problem hiding this comment.
I wonder if that exact version is still even correct anymore. I feel like this README entry should probably be redirected to something that is actually used by the CI suite as well, to ensure dependencies, so that we get them lined up, and can avoid future skew.
There was a problem hiding this comment.
Backup/restore acoounts.yaml file helped to avoid using expect so we can postpone this tool introduction
|
/merge |
#14595 ``` Merge branch '2.9' into 2.9-into-develop # Conflicts: # acceptancetests/assess_cloud.py # acceptancetests/assess_endpoint_bindings.py # acceptancetests/assess_heterogeneous_control.py # state/migration_import_test.go # apiserver/facades/client/client/status.go # scripts/win-installer/setup.iss # snap/snapcraft.yaml # version/version.go ``` - #14551 - #14557 - #14560 - #14558 - #14554 - #14570 - #14577 - #14506 - #14538 - #14559 - #14574 - #14573 - #14567 - #14545 - #14541 - #14588
anvial
deleted the
JUJU-1209-rewrite-to-bash-based-nw-unregister-controller
branch
This PR adds a bash-based version of the unregister-controller test.
Checklist
QA steps
cd tests ./main.sh -v -p aws -c aws controller test_unregister