-
Notifications
You must be signed in to change notification settings - Fork 505
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
Ensure relation members in uniter data model are not nil #11634
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
howbazaar
approved these changes
May 28, 2020
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.
Awesome, thanks.
stCopy := &State{ | ||
RelationId: s.RelationId, | ||
ChangedPending: s.ChangedPending, | ||
stCopy := NewState(s.RelationId) |
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.
Much cleaner and nicer. Thanks.
|
Merged
jujubot
added a commit
that referenced
this pull request
May 29, 2020
#11640 ## Description of change Merge 2.8-rc branch with these commits: #11641 unpin machine applications on destroy #11610 Fix CAAS Operator tests race #11616 Test speedup - add insecure KeyProfile for testing #11628 Life leader pinning API to be a member #11612 Introduce centralised SNI TLS getter #11620 remove machine series upgrade lock #11630 migrating an old controller with incomplete endpoint bindings confused model import #11633 Add 2.7.7 upgrade step to re-run ReplaceSpaceNameWithIDEndpointBindings #11634 Ensure relation members in uniter data model are not nil #11627 CMR relation remove ## QA steps See individual PRs
jujubot
added a commit
that referenced
this pull request
May 29, 2020
#11642 Forward Port: - Increment juju to 2.8-rc3 #11603 - Version updated to 2.8.1 #11605 - 2.8 - Enhance state.Space -> network.SpaceInfo Conversion Performance #11606 - Add an acceptance test for CharmRevisionUpdater instead of flakey "unit" test #11609 - 2.8 - Add GetByHardwareAddress to InterfaceInfos #11607 - CAASOperator tests race condition #11610 - Test speedup - fast insecure KeyProfile for testing #11616 - Remove JujuConnSuite from some api tests #11615 - [2.8] Enhance manual machine cleanup script #11618 - [2.8] All space infos backport #11622 - Multi-arch docker image build support #11623 - 2.8 - Backport Uniter Test Race Fix #11625 - 2.8 - Use Up-To-Date Charm for Branches Integration Test #11626 - Lift leader pinning API to be a member #11628 - [2.8] Bindings constraints for space topology #11621 - Introduces centralised SNI tls getter. #11612 - Muxhttp server tests for worker #11629 - 2.7 - Remove Machine Series Upgrade Lock #11620 - Migrating a model from an old controller with incomplete endpoint bin… #11630 - Add support for bootstrapping to EKS; #11614 - Add 2.7.7 upgrade step to re-run ReplaceSpaceNameWithIDEndpointBindings #11633 - Ensure relation members in uniter data model are not nil #11634 - CMR relation remove #11627 - 2.8-rc Performance Enhancements Ported from 2.8 #11637 - Refactor leadership pinning API #11631 - Merge 2.7 2020528 #11636 - Port stickupkid/unpin machine applications on destroy #11641 - Merge 2.8-rc branch #11640 - Clean up more package level loggers. #11635 Conflicts: - core/network/nic.go - core/network/nic_test.go - scripts/win-installer/setup.iss - snap/snapcraft.yaml - version/version.go
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
The relation state struct maintained by the uniter agent is initialised with empty (not nil) maps for Members and ApplicationMembers. YAML serialisation / deserialisation however results in these becoming nil if they were empty when serialised. There's a subsequent validation check before the next hook invocation which fails as it checks for nil. This wasn't an issue previously when state was stored to disk and not the controller as reading state off disk would ensure the maps were empty not nil.
QA steps
I tested with a modified build which added a wrench but not the patch.
Use the wrench to set up an error in the leader-elected hook for mariadb and create a relation to mediawiki. Remove mariadb once things appear settled and observe the logs
unit-mariadb-2: 12:42:34 ERROR juju.worker.dependency "uniter" manifold worker returned unexpected error: preparing operation "run relation-broken (4; app: ) hook": inappropriate "relation-broken" for "": relation is broken and cannot be changed further
upgrade-controller --build-agent with the patch and see that the relation-broken hook is now run and there are no errors.
Bug reference
https://bugs.launchpad.net/juju/+bug/1881021