Embed a context.Context in the environ ProviderCallContext #12953
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
wallyworld
force-pushed
the
provider-context-refactor
branch
3 times, most recently
from
85933a7 to
99aeb3c
Compare
manadart
approved these changes
May 7, 2021
| @@ -14,7 +16,8 @@ type ModelCredentialInvalidator interface { | |||
| // CallContext creates a CloudCallContext for use when calling environ methods | |||
| // that may require invalidate a cloud credential. | |||
| func CallContext(ctx ModelCredentialInvalidator) *CloudCallContext { | |||
| callCtx := NewCloudCallContext() | |||
| // TODO(wallyworld) - pass in the stdcontext | |||
There was a problem hiding this comment.
Not yet. This is called from various facade methods, just once in the method. It would be a lot more churn to pass in a context and unlike the workers where we were setting up a static context and reusing it, it's ok to leave this for now and follow up later to tidy up.
wallyworld
force-pushed
the
provider-context-refactor
branch
from
99aeb3c to
fddd976
Compare
|
|
jujubot
added a commit
that referenced
this pull request
May 28, 2021
#13000 Merge from 2.9 to bring forward: - #12998 from wallyworld/merge-2.8-20210520 - #12996 from wallyworld/caas-remove-storage - #12991 from ycliuhw/fix/lp-1927721 - #12995 from jnsgruk/2.9 - #12994 from SimonRichardson/upgrade-series-containers-regression - #12992 from manadart/2.9-upgrades - #12982 from manadart/2.9-net-get-sort - #12990 from jujubot/increment-to-2.9.3 - #12988 from tlm/lp1928486-proxy-vars - #12989 from wallyworld/ss-service-name - #12987 from wallyworld/embeddedcli-macaroon-error - #12985 from wallyworld/export-bundle-defaults - #12984 from wallyworld/operator-charm-upgrade - #12981 from wallyworld/remove-unit-panic - #12979 from wallyworld/k8s-lxd-cloud - #12978 from wallyworld/sidecar-charm-storage-fix - #12980 from tlm/2.8-to-2.9-merge - #12972 from SimonRichardson/add-relation-error - #12970 from manadart/2.9-address-refactoring - #12976 from jujubot/increment-to-2.9.2 - #12975 from wallyworld/merge-2.8-20210513 - #12968 from ycliuhw/fix/lp-1927311 - #12973 from achilleasa/2.9-fix-misc-refresh-with-switch-issues - #12961 from manadart/2.9-store-is-secondary - #12969 from SimonRichardson/fix-static-analysis - #12964 from tlm/k8s-proxy-ssh - #12951 from ycliuhw/fix/lp-1925734 - #12962 from tlm/k8s-proxying-python-rbac - #12959 from SimonRichardson/bundle-scale-overlays - #12960 from SimonRichardson/users-cli-help-directive - #12954 from manadart/2.9-machine-config-per-nic - #12958 from wallyworld/improve-add-relation-message - #12956 from tlm/lp-1926595-libjuju - #12955 from wallyworld/destroy-model-timeout - #12957 from tlm/lp-1892255-env-vars - #12953 from wallyworld/provider-context-refactor - #12952 from manadart/2.9-transport-address-config - #12944 from wallyworld/extra-destroymodel-info Conficts: - apiserver/facades/client/action/operation.go - apiserver/facades/client/application/application.go - apiserver/facades/client/application/application_unit_test.go - apiserver/facades/controller/caasoperatorprovisioner/provisioner.go - apiserver/params/network.go - caas/kubernetes/provider/application/application_test.go - cmd/juju/application/refresher/refresher_mock_test.go - cmd/juju/application/utils/utils.go - feature/flags.go - go.mod - go.sum - provider/azure/environ.go - provider/azure/storage.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.
We use a
environs/context.ProviderCallContextto pass to API calls in the Environ interface. This context instance provides a callback which can be used to report invalid credentials.This PR embeds a
context.Contextin the above context. This is immediately useful for the Azure provider where the Azure SDK takes acontext.Contextand we were having to create a separate one of those ourselves. Now, we can just use a single context instance.Most of the churn here is mechanical - a common helper method used by tests was renamed.
NewCloudCallContext -> NewEmptyCloudCallContext
Also, the workers which use a context got some refactoring to create the context on demand for each API call instead of creating one up front and using that same instance each time.
A followup PR will use this work to provide a context with deadline to the environ.Destroy() call to allow model destroy operations to timeout cleanly.
QA steps
This is just a refactor. QA can be to bootstrap on a few substrates (including Azure) and deploy a few charms.