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.

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

[JUJU-1307] Rewrote to bash based nw-resolve #14573

Merged
merged 4 commits into from
Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions testcharms/charms/simple-resolve/README
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Simple charm that errors once on the install hook, but works the next time run.

Used to test juju resolve command.
1 change: 1 addition & 0 deletions testcharms/charms/simple-resolve/copyright
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Copyright 2018 Canonical Ltd.
26 changes: 26 additions & 0 deletions testcharms/charms/simple-resolve/hooks/install
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#!/bin/bash

# Simple install hook that errors the first time it is run, on any subsequent
# run the hook will succeed (unless the canary file is removed).

canary_file=/tmp/resolver.canary
success_file=/tmp/resolver.success

function exit_error() {
status-set blocked "Install hook failed on: $1 attempts." || true
exit 1
}

if [ ! -e ${canary_file} ]; then
echo "No canary file, exiting error."
echo 1 > ${canary_file}
exit_error 1
fi

# Need to error for x amount of times before juju stops trying automatically.
run_count=$(cat ${canary_file})

# For now juju is always re-trying. Need to just exit regardless.
echo $((${run_count}+1)) > ${canary_file}
exit_error ${run_count}

11 changes: 11 additions & 0 deletions testcharms/charms/simple-resolve/hooks/start
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash

install_success_file=/tmp/resolver.success

status_message="No install hook"
if [ -e ${install_success_file} ]; then
status_message="Install hook succeeded"
fi

echo "${status_message}"
status-set active "${status_message}" || true
11 changes: 11 additions & 0 deletions testcharms/charms/simple-resolve/metadata.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: simple-resolve
maintainer: Christopher Lee <chris.mclachlan-lee@canonical.com>
summary: Purposfully error on install hook for testing resolve command.
description: |
A testing charm that errors once on install hook then succeeds on the next run.
categories:
- misc
series:
Copy link
Member

Choose a reason for hiding this comment

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

Juju is soon to only support jammy and focal, so the rest of these should probably be dropped

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed

- jammy
- focal
- bionic
24 changes: 24 additions & 0 deletions tests/suites/deploy/deploy_charms.sh
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,28 @@ run_deploy_lxd_to_container() {
destroy_model "${model_name}"
}

# Checks the install hook resolving with --no-retry flag
run_resolve_charm() {
echo

model_name="test-resolve-charm"
file="${TEST_DIR}/${model_name}.log"

ensure "${model_name}" "${file}"

charm=./testcharms/charms/simple-resolve
juju deploy "${charm}"

wait_for "error" '.applications["simple-resolve"] | ."application-status".current'

juju resolve --no-retry simple-resolve/0

wait_for "No install hook" '.applications["simple-resolve"] | ."application-status".message'
wait_for "active" '.applications["simple-resolve"] | ."application-status".current'

destroy_model "${model_name}"
}
Copy link
Member

Choose a reason for hiding this comment

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

I see that you have copied over --no-retry from the original test. function names in the test seem to imply that it is asserted in the test that the failed hook is not re-run, but this is never checked.

Worth evaluating the purpose of this test, since the original seems broken

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean to add a check that there was only ONE hook re-run (after the juju resolve --no-retry execution)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a message wait_for ("No install hook") to check the --no-retry flag behavior.


test_deploy_charms() {
if [ "$(skip 'test_deploy_charms')" ]; then
echo "==> TEST SKIPPED: deploy charms"
Expand All @@ -243,11 +265,13 @@ test_deploy_charms() {
run "run_deploy_lxd_to_machine"
run "run_deploy_lxd_profile_charm"
run "run_deploy_local_lxd_profile_charm"
run "run_resolve_charm"
;;
*)
echo "==> TEST SKIPPED: deploy_lxd_to_machine - tests for LXD only"
echo "==> TEST SKIPPED: deploy_lxd_profile_charm - tests for LXD only"
echo "==> TEST SKIPPED: deploy_local_lxd_profile_charm - tests for LXD only"
echo "==> TEST SKIPPED: resolve_charm - tests for LXD only"
;;
esac
)
Expand Down