-
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
Merge 3.2 into 3.3 #16476
Merged
Merged
Merge 3.2 into 3.3 #16476
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 model config `authorized-keys` should not be settable directly by doing $ juju model-config authorized-keys=foo. This patch ensures that the model config set does not contain the authorized-keys key or it returns an error.
exists there. Old versions used region from the credential, but subsequent versions correctly use the model's cloud region.
…an error is returned anyways
juju#16459 The model config `authorized-keys` should not be settable directly by doing $ juju model-config authorized-keys=foo. This patch ensures that the model config set does not contain the authorized-keys key or it returns an error. ## Checklist *If an item is not applicable, use `~strikethrough~`.* - [X] Code style: imports ordered, good names, simple structure, etc - [X] Comments saying why design decisions were made - [X] Go unit tests, with comments saying what you're testing - [ ] ~[Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~ - [ ] ~[doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~ ## QA steps After bootstrapping (on any cloud) and creating a model, try updating the authorized-keys using `model-config`: ``` juju model-config authorized-keys=foo ERROR authorized-keys cannot be set ``` ## Links **Launchpad bug:**https://bugs.launchpad.net/juju/+bug/2039615 **Jira card:** JUJU-4813 *Insert other relevant links here.*
juju#16467 The Oracle provider on 2.9 uses the region from credential attributes as the model region. This breaks when migrating models to 3.3 where this behaviour has been fixed to use region stored against the model itself. Here we modify migrated OCI clouds to set the region from credential attributes if it exists. ## QA steps - Bootstrap an OCI cloud with this patch and add a model. - Bootstrap a 3.3 OCI, then migrate the model to it. ## Links **Jira card:** [JUJU-4809](https://warthogs.atlassian.net/browse/JUJU-4809) [JUJU-4809]: https://warthogs.atlassian.net/browse/JUJU-4809?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
# Conflicts: # state/migration_export_test.go
juju#16474 5c86ef2 (origin/2.9, 2.9) Merge pull request juju#16467 from manadart/2.9 No conflicts
juju#16475 Merge from 3.1 to bring forward: - juju#16474 from anvial/merge-2.9-3.1-20231020 - juju#16459 from nvinuesa/juju-4813 Trivially resolved conflict in _state/migration_export_test.go_.
manadart
approved these changes
Oct 20, 2023
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.
Thanks.
anvial
force-pushed
the
merge-3.2-3.3-20231020
branch
from
October 20, 2023 10:23
c13b9fa
to
197eb77
Compare
/merge |
anvial
force-pushed
the
merge-3.2-3.3-20231020
branch
from
October 20, 2023 10:42
197eb77
to
e1879df
Compare
anvial
force-pushed
the
merge-3.2-3.3-20231020
branch
from
October 20, 2023 11:36
e1879df
to
3cc3070
Compare
/merge |
Merged
jujubot
added a commit
that referenced
this pull request
Oct 25, 2023
#16498 Forward merges: - #16459 - #16467 - #16473 - #16474 - #16475 - #16476 - #16393 - #16480 - #16481 - #16287 - #16477 - #16486 - #16434 - #16487 - #16488 - #16491 - #16492 - #16494 - #16489 - #16497 Conflicts: - api/client/machinemanager/machinemanager.go - api/client/machinemanager/machinemanagernew_test.go - apiserver/facades/client/modelconfig/modelconfig.go - apiserver/facades/client/modelconfig/modelconfig_test.go - apiserver/stateauthenticator/context.go - cmd/juju/application/refresh_test.go - scripts/win-installer/setup.iss - snap/snapcraft.yaml - version/version.go Mostly trivially solved. `cmd/juju/application/refrest_test.go` was the only one that required any original code. Verify by ensuring unit tests pass
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.
Moving forward:
No conflicts.
Note: Drops fix for OCI cloud region in separate commit