Merge 2.7 #11429
Merged
Merge 2.7 #11429
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
They're not needed after that section has passed, and on resource-constrained clouds like our vSphere it might make a difference to other vSphere-using tests (or even make this one more reliable).
…stroy-models acceptancetests: Destroy the extra models created in the migration CI test early
have them created when adding the default binding entry. this prevents a migration import panic for such scenarios.
juju#11421 ## Description of change We have seen that an old 2.5.1 model upgraded to 2.7.5 and then migrated is failing import with a panic. This is due to having an empty or absent endpoint bindings document, which when serialised by the description package (with `omitempty`) is nil, resulting in a panic when we attempt to populate the default binding. This patch simply creates the bindings map if it is nil, before setting the default binding. ## QA steps One could test with an old version of Juju, but the easiest way is to: - Bootstrap with this patch. - Deploy one application. - Delete the application's record from the `endpointbindings` collection directly. - Migrate the model and ensure success. ## Documentation changes None. ## Bug reference https://bugs.launchpad.net/juju/+bug/1871066
Part of uniter++ work.
Ensure that the start hook is run for CAAS units with storage. Add tests for CAAS units, wait for started to attach storage.
juju#11424 ## Description of change Backport of PR 11119: Enable install hook for k8s charms while skipping the install operation done by the uniter. It is not required due to the k8s pods. Do not block storage hooks based on the install hook not having been run for CAAS units, other wise it will hang if there is storage. ## QA steps Please see qa steps from juju#11119 ## Documentation changes https://discourse.juju.is/t/install-hook-now-called-for-kubernetes-charms/2585/2 ## Bug reference https://bugs.launchpad.net/juju/+bug/1854635
The serialised hooks have a new required field remote-application - set any hooks that don't have it set to be the application from the remote-unit field. Without this the uniter will refuse to start. See https://bugs.launchpad.net/juju/+bug/1869275
We can't necessarily rely on the uniter state being deserialisable into the current operation.State struct, since there could be subsequent upgrade steps. Read it into a map and just add remote-application to the hook instead.
We need to use the controller model state when getting api address info.
…grade-step juju#11426 ### Checklist - [ ] Checked if it requires a [pylibjuju](https://github.com/juju/python-libjuju) change? - [ ] Added [integration tests](https://github.com/juju/juju/tree/develop/tests) for the PR? - [ ] Added or updated [doc.go](https://discourse.jujucharms.com/t/readme-in-packages/451) related to packages changed? - [ ] Do comments answer the question of why design decisions were made? ---- ## Description of change If an upgrade happens while a hook is queued, the uniter state file will contain details about that hook execution so it can be run once the unit agent is running again. In 2.7 the hook was extended to include `remote-application`, and validation requires it. But if the hook was written by the 2.6 uniter and then the agent is upgraded to 2.7, the uniter won't start because the state file is invalid. Run an upgrade step when going to 2.7.6 to update any hooks that don't have `remote-application` set to be the application from the remote-unit field. (We're running it in the 2.7.6 upgrade because there could be agents stuck in this state since upgrading from 2.6.x.) ## QA steps Bootstrap a 2.6 controller, deploy dummy-source and set its token to a value. ```sh /snap/bin/juju bootstrap lxd n juju deploy ~/juju/acceptancetests/repository/charms/dummy-source juju config dummy-source token=hey ``` Modify the dummy-sink so that its `source-relation-changed` hook fails if `token` is set to "block". ```bash #!/bin/bash set -eux mkdir -p /var/run/dummy-sink token=$(relation-get token) echo "$token" > /var/run/dummy-sink/token if [[ "$token" == "block" ]]; then juju-log -l WARNING "Blocking: hook won't succeed while token is block" status-set blocked "Token is block, change it" || true exit 1 fi if [[ -z "$token" ]]; then juju-log -l INFO "Waiting for token" status-set waiting "Waiting for token" || true else juju-log -l INFO "Token is $token" status-set active "Token is $(echo $token | cut -c 1-20)" || true fi ``` Deploy that charm and relate it to the source. ```sh juju deploy ./blocker-sink && juju relate dummy-source blocker-sink ``` Update token to "block" - blocker-sink should get stuck. ```sh juju config dummy-source token=block ``` Check that the hook is in `/var/lib/juju/agents/unit-blocker-sink-0/state/uniter` without `remote-application`. Once the charm is blocked, upgrade to 2.7.6. ```sh juju upgrade-controller --build-agent juju upgrade-model ``` Watch the blocker-sink machine agent log to see the upgrade step report updating the file. See that the hook in the state file is updated with the application. Check that the uniter is started and hook executions are running. ```sh juju config dummy-source token=fine juju resolve blocker-sink/0 ``` ## Documentation changes None ## Bug reference https://bugs.launchpad.net/juju/+bug/1869275
juju#11427 ## Description of change juju#11407 introduced a regression when looking up controller API addresses. This PR ensures we use the controller state when needed to get the cloud service. ## QA steps bootstrap k8s controller deploy any k8s charm (would fail previously) ## Bug reference https://bugs.launchpad.net/juju/+bug/1871496
manadart
approved these changes
Apr 8, 2020
| controllerConfig, err := st.ControllerConfig() | ||
| // We are fetching info about the controller cloud service, | ||
| // so need the controller state. | ||
| ctrlSt, err := st.newStateNoWorkers(st.ControllerModelUUID()) |
There was a problem hiding this comment.
Oops. I missed that we were doing more than accessing the controllers collection, which is global.
|
|
|
@wallyworld this won't merge because there is a error in the code.
|
|
@wallyworld The missing code for reading the step can be emulated by cloning the code from here https://github.com/juju/juju/blob/develop/upgrades/steps_28.go#L102 |
|
@wallyworld this will resolve the issue: 316e68c Tested with upgrades from 2.6 and 2.7.5. 2.6 following the qa steps in #11426. |
|
|
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
Merge 2.7 with these PRs:
#11415 destroy extra models created in migration CI test
#11421 fix panic migrating application with no bindings
#11424 backport k8s install hook
#11426 add 2.7.6 upgrade step to fix uniter state files
#11427 fix regression deploying k8s charms
QA steps
run unit tests