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
Port stickupkid/unpin machine applications on destroy #11641
Merged
jujubot
merged 7 commits into
juju:2.8-rc
from
wallyworld:port-stickupkid/unpin-machine-applications-on-destroy
May 29, 2020
Merged
Port stickupkid/unpin machine applications on destroy #11641
jujubot
merged 7 commits into
juju:2.8-rc
from
wallyworld:port-stickupkid/unpin-machine-applications-on-destroy
May 29, 2020
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
The following re-works the common leadership pinning API. It removes the idea that the API is part of a facade and instead defines it's own type, one that can move independantly of a facade version. This has been an ongoing discussion for sometime, but has concretely formalised to a discourse post[1]. The changes are relatively simple, adding new methods for taking a application name slice, so calling Pin/Unpin doesn't require a machine to talk to. This should give us a lot of flexibility about when and how we can call these methods. 1. https://discourse.juju.is/t/common-facade-apis/3081
The tag is required to be higher up the stack in-order to make it accessible for external calls to change it depending on their requirements.
The following updates the methods to be a lot more generic as we no-longer care about if it's a machine we're dealing with. All we care about is the application names, so with this in mind I've updated the type correctly, along with tests...
…p-pinning-type juju#11631 ## Description of change The following re-works the common leadership pinning API. It removes the idea that the API is part of a facade and instead defines its own type, one that can move independently of a facade version. This has been an ongoing discussion for some time, but has concretely formalised to a discourse post[1]. The changes are relatively simple, adding new methods for taking an application name slice, so calling Pin/Unpin doesn't require a machine to talk to. This should give us a lot of flexibility about when and how we can call these methods. 1. https://discourse.juju.is/t/common-facade-apis/3081 ## QA steps Test pass, this is a mechanical refactoring change ## Bug reference https://bugs.launchpad.net/juju/+bug/1879663
During a destruction of a machine application, we should attempt to remove any last references to ties for leadership pinning. The normal destroy flow will attempt to remove the leadership pinnings, but will fail if there exists pinnings. Using force will allow this to proceed. The call for removing pinnings is idempotent, so calling it multiple times is fine.
…n-destroy' into port-stickupkid/unpin-machine-applications-on-destroy
ycliuhw
approved these changes
May 29, 2020
|
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
Just note that I made a comment on the un-merged original patch, regarding returning an unpin error after successfully calling invoking destruction. |
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
Forward port of #11639 with legacy lease structs removed.
QA steps
See original PR
Bug reference
https://bugs.launchpad.net/juju/+bug/1879663