Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[JUJU-1209] Added a bash-based version of the unregister-controller test. #14538

Merged
merged 4 commits into from
Sep 6, 2022
Merged

[JUJU-1209] Added a bash-based version of the unregister-controller test. #14538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fc4ff33
Added bash-based unregister-controller test.
anvial Aug 30, 2022
cda592d
Removed autogenerated comments from expect scripts.
anvial Aug 30, 2022
573b34b
Added accounts.yaml backup/restore, that helped to avoid using expect…
anvial Aug 31, 2022
b0e5a61
Added check that default controller is not equal to unregistered one …
anvial Aug 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions tests/suites/controller/task.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,7 @@ test_controller() {
# Leave this one last, as it can cause mongo to slowdown to a snails pace.
test_mongo_memory_profile

test_unregister

destroy_controller "test-controller"
}
49 changes: 49 additions & 0 deletions tests/suites/controller/unregister.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
run_unregister() {
echo

file="${TEST_DIR}/unregister.log"

ensure "unregister" "${file}"

echo "Get controller name"
controller_name=$(juju controllers --format=json | jq -r '."current-controller"')

echo "Check controller is known"
juju controllers --format=json | jq -r ".\"controllers\" | has(\"${controller_name}\")" | check true

echo "Backup controller info before unregister"
cp ~/.local/share/juju/controllers.yaml ~/.local/share/juju/controllers.yaml.bak
cp ~/.local/share/juju/accounts.yaml ~/.local/share/juju/accounts.yaml.bak

echo "Unregister controller"
juju unregister --yes "${controller_name}"

echo "Check controller is not known"
juju controllers --format=json | jq -r ".\"controllers\".\"${controller_name}\"" | check null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a comment here with:

# if you unregister the active controller, it should ensure that the current active controller is reset to empty

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it,

I've added:

  • a check that the named controller is in juju controllers output before the unregister
  • a check that the default controller is not equal to the unregistered one after juju unregister.


echo "Check the default controller is not equal to unregistered one"
check_not_contains "$(juju controllers --format=json | jq -r '."current-controller"')" "${controller_name}"

echo "Restore controller info after unregister"
mv ~/.local/share/juju/controllers.yaml.bak ~/.local/share/juju/controllers.yaml
mv ~/.local/share/juju/accounts.yaml.bak ~/.local/share/juju/accounts.yaml

juju controllers --format=json | jq -r '."current-controller"' | check "${controller_name}"

destroy_model "unregister"
}

test_unregister() {
if [ -n "$(skip 'test_unregister')" ]; then
echo "==> SKIP: Asked to skip controller unregister tests"
return
fi

(
set_verbosity

cd .. || exit

run "run_unregister"
)
}