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

Conversation

anvial
Copy link
Member

@anvial anvial commented Sep 5, 2022

This PR added a bash-based version of the nw-resolve test to the deploy suite. To implement the test, the simple-resolve charm was copied to the testcharms subfolder.

Checklist

  • Code style: imports ordered, good names, simple structure, etc
  • Comments saying why design decisions were made

QA steps

cd tests
./main.sh -v -p lxd deploy run_resolve_charm

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

# touch ${success_file}
# status-set maintenance "Install hook succeeded." || true
# exit 0
# fi
Copy link
Member

Choose a reason for hiding this comment

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

Should these comments be dropped?

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.

Copy link
Member

@jack-w-shaw jack-w-shaw left a comment

Choose a reason for hiding this comment

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

LGMT

@anvial
Copy link
Member Author

anvial commented Sep 6, 2022

/merge

@jujubot jujubot merged commit 0c289c1 into juju:2.9 Sep 6, 2022
@ycliuhw ycliuhw mentioned this pull request Sep 9, 2022
jujubot added a commit that referenced this pull request Sep 10, 2022
#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 anvial deleted the JUJU-1307-rewrite-to-bash-based-nw-resolve branch November 2, 2022 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants