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

Lp 1927656 #13096

Merged
merged 4 commits into from
Jun 23, 2021
Merged

Lp 1927656 #13096

merged 4 commits into from
Jun 23, 2021

Conversation

ycliuhw
Copy link
Member

@ycliuhw ycliuhw commented Jun 22, 2021

Ensure the controllerUUID annotation of the migrated model's namespace gets updated to the new controller's UUID;

Checklist

  • Requires a pylibjuju change
  • Added integration tests for the PR
  • Added or updated doc.go related to packages changed
  • Comments answer the question of why design decisions were made

QA steps

$ juju bootstrap microk8s k1

$ juju bootstrap microk8s k2

$ juju add-model t1 -c k1 && juju deploy cs:~juju/mariadb-k8s-3 -m k1:t1

$ yml2json ~/.local/share/juju/controllers.yaml | jq '.controllers|to_entries|map("\(.key) => \(.value.uuid)")'
[
  "k2 => 16e28e79-9ccd-471f-8bd8-2ed35e53bb14",
  "k1 => e9f2ceb6-4d1e-44db-8c7d-5c5aaf34fe1f"
]

$ mkubectl get ns t1 -o json | jq .metadata.annotations
{
  "controller.juju.is/id": "e9f2ceb6-4d1e-44db-8c7d-5c5aaf34fe1f",
  "model.juju.is/id": "c688bb92-0587-4ad0-8209-431e9e5a65e7"
}

$ juju migrate k1:t1 k2

$ mkubectl get ns t1 -o json | jq .metadata.annotations
{
  "controller.juju.is/id": "16e28e79-9ccd-471f-8bd8-2ed35e53bb14",
  "model.juju.is/id": "c688bb92-0587-4ad0-8209-431e9e5a65e7"
}

$ juju kill-controller -t 0 -y k2 --debug

$ mkubectl get ns t1
Error from server (NotFound): namespaces "t1" not found

Documentation changes

No

Bug reference

https://bugs.launchpad.net/juju/+bug/1927656

@ycliuhw
Copy link
Member Author

ycliuhw commented Jun 22, 2021

@hpidcock hpidcock added the 2.9 label Jun 22, 2021
Copy link
Member

@hpidcock hpidcock left a comment

Choose a reason for hiding this comment

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

LGTM, couple of non-critical comments

@@ -202,14 +201,14 @@ func (s *BaseSuite) setupController(c *gc.C) *gomock.Controller {
return "appuuid", nil
}

s.mockNamespaces.EXPECT().Get(gomock.Any(), s.getNamespace(), v1.GetOptions{}).
s.mockNamespaces.EXPECT().Get(gomock.Any(), s.getNamespace(), v1.GetOptions{}).Times(2).
Copy link
Member

Choose a reason for hiding this comment

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

I think just making this .Any() would be fine.

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'd prefer asserting more explicitly. And also set to AnyTimes() here might fail the later namespace get assertion statements in other real tests.

caas/kubernetes/provider/k8s.go Outdated Show resolved Hide resolved
@ycliuhw
Copy link
Member Author

ycliuhw commented Jun 23, 2021

$$merge$$

@jujubot jujubot merged commit 4bb6b79 into juju:2.9 Jun 23, 2021
jujubot added a commit that referenced this pull request Jun 28, 2021
#13109

Merge from 2.9 to bring forward:
- #13103 from manadart/2.9-lxd-spec-assignment
- #13105 from ycliuhw/fix/lp-1929904
- #13106 from SimonRichardson/update-http-dep
- #13100 from tlm/model-operator-29-upgrade
- #13102 from manadart/2.9-lxd-with-proxy
- #13099 from SimonRichardson/updating-packaging-dep
- #13096 from ycliuhw/lp-1927656
- #13101 from hpidcock/aws-encrypted-ebs
- #13073 from tlm/LP1930798-juju-2.9-k8s-upgrade-2
- #13078 from hmlanigan/sidecar-charm-via-bundle
- #13098 from achilleasa/2.9-logsink-error-if-persisting-logs-to-db-fails
- #13091 from wallyworld/use-aws-sdk-v2
- #13095 from jujubot/increment-to-2.9.7

Conflicts:
- caas/kubernetes/provider/export_test.go
- cmd/juju/application/deployer/bundlehandler.go
- cmd/juju/application/deployer/deployer.go
- core/bundle/changes/handlers.go
- core/charm/computedseries.go
- go.mod
- go.sum
- 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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants