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

Revert "sdk/py/ComponentResource: Propagate provider to children" #12522

Merged
merged 1 commit into from Mar 27, 2023

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Mar 27, 2023

This reverts commit db4c071 (#12292).

This appears to have broken some customers.
Reverting to unblock.

Reopens #12161
Refs #12520

This reverts commit db4c071 (#12292).

This appears to have broken some customers.
Reverting to unblock.

Reopens #12161
Refs #12520
@pulumi-bot
Copy link
Contributor

Changelog

[uncommitted] (2023-03-27)

Bug Fixes

@abhinav
Copy link
Contributor Author

abhinav commented Mar 27, 2023

bors merge

abhinav added a commit that referenced this pull request Mar 27, 2023
We want to ship #12522 in a release ASAP.
There have been non-bugfix changes since last release
so we need to bump the minor version.

This bumps the version in preparation for the release.

Refs #12520
@bors
Copy link
Contributor

bors bot commented Mar 27, 2023

Build succeeded:

@bors bors bot merged commit 03f1067 into master Mar 27, 2023
42 checks passed
@bors bors bot deleted the abhinav/revert-py-provider-prop branch March 27, 2023 18:35
bors bot added a commit that referenced this pull request Mar 27, 2023
12525: Prepare release v3.60.0 r=abhinav a=abhinav

We want to ship #12522 in a release ASAP.
There have been non-bugfix changes since last release
so we need to bump the minor version.

This bumps the version in preparation for the release.

Refs #12520


Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
bors bot added a commit that referenced this pull request Apr 13, 2023
12639: sdk/py: Propagate provider from local and remote component resources r=abhinav a=abhinav

Each commit is individually reviewable.
The first commit just re-applies #12292.

---

This change brings back #12292 (fix for #12161),
which was reverted in #12522 because it broke MLCs (#12520)
and corrects the issue that caused that failure.

- #12161 manifests as local component resources
  not propagating the `provider` option to their children
- #12520 manifests as remote component resources (MLCs)
  not propagating the `provider` option to their children

Roughly, given:

    class MyComponentResource:
      def __init__(self, ...):
        # ...
        self.child = aws.s3.Bucket(..., opts=ResourceOptions(parent=self))

    MyComponentResource(opts=ResourceOptions(provider=my_aws_provider))

Both bugs took the form of the `aws.s3.Bucket` above not getting
`my_aws_provider`.

We previously fixed #12161 by keeping using the provider for the
resource only if its package matched the resource's type; otherwise, we
placed it in the providers bag for the children of that resource.

This had the effect of dropping the provider for component resources
because:

- `_get_providers` returns a provider (None for component resources)
  and a bag of providers for children of the resource.
  It is stored on the resource:

  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/resource.py#L866

- A request is made to `register_resource`, which calls
  `prepare_resource`:

  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/resource.py#L880-L882
  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/runtime/resource.py#L811

- `prepare_resource` re-interprets the `provider` and `providers`
  resource options, based on what's in the **original** `opts` struct
  completely ignoring the second result of `_get_providers`:

  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/runtime/resource.py#L210

So in short, the reason the original change broke was:
It turned `provider=x` into `providers=[x]` when appropriate,
but the consuming code did not consume the transformed option.

To fix this, we put the bag of providers produced by `_get_providers`
back into the `opts` struct so that `prepare_resource` and friends can
use it.

Testing:
An integration test is added that defines an MLC,
and uses it from Pulumi programs in Go, Python, and Node
that instantiate the MLC with no provider, provider option, and providers option,
and verifies that resources were instantiated with the correct providers.

Resolves #12593
Resolves #12161


Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
bors bot added a commit that referenced this pull request Apr 14, 2023
12639: sdk/py: Propagate provider from local and remote component resources r=abhinav a=abhinav

Each commit is individually reviewable.
The first commit just re-applies #12292.

---

This change brings back #12292 (fix for #12161),
which was reverted in #12522 because it broke MLCs (#12520)
and corrects the issue that caused that failure.

- #12161 manifests as local component resources
  not propagating the `provider` option to their children
- #12520 manifests as remote component resources (MLCs)
  not propagating the `provider` option to their children

Roughly, given:

    class MyComponentResource:
      def __init__(self, ...):
        # ...
        self.child = aws.s3.Bucket(..., opts=ResourceOptions(parent=self))

    MyComponentResource(opts=ResourceOptions(provider=my_aws_provider))

Both bugs took the form of the `aws.s3.Bucket` above not getting
`my_aws_provider`.

We previously fixed #12161 by keeping using the provider for the
resource only if its package matched the resource's type; otherwise, we
placed it in the providers bag for the children of that resource.

This had the effect of dropping the provider for component resources
because:

- `_get_providers` returns a provider (None for component resources)
  and a bag of providers for children of the resource.
  It is stored on the resource:

  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/resource.py#L866

- A request is made to `register_resource`, which calls
  `prepare_resource`:

  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/resource.py#L880-L882
  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/runtime/resource.py#L811

- `prepare_resource` re-interprets the `provider` and `providers`
  resource options, based on what's in the **original** `opts` struct
  completely ignoring the second result of `_get_providers`:

  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/runtime/resource.py#L210

So in short, the reason the original change broke was:
It turned `provider=x` into `providers=[x]` when appropriate,
but the consuming code did not consume the transformed option.

To fix this, we put the bag of providers produced by `_get_providers`
back into the `opts` struct so that `prepare_resource` and friends can
use it.

Testing:
An integration test is added that defines an MLC,
and uses it from Pulumi programs in Go, Python, and Node
that instantiate the MLC with no provider, provider option, and providers option,
and verifies that resources were instantiated with the correct providers.

Resolves #12593
Resolves #12161


Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
bors bot added a commit that referenced this pull request Apr 14, 2023
12639: sdk/py: Propagate provider from local and remote component resources r=abhinav a=abhinav

Each commit is individually reviewable.
The first commit just re-applies #12292.

---

This change brings back #12292 (fix for #12161),
which was reverted in #12522 because it broke MLCs (#12520)
and corrects the issue that caused that failure.

- #12161 manifests as local component resources
  not propagating the `provider` option to their children
- #12520 manifests as remote component resources (MLCs)
  not propagating the `provider` option to their children

Roughly, given:

    class MyComponentResource:
      def __init__(self, ...):
        # ...
        self.child = aws.s3.Bucket(..., opts=ResourceOptions(parent=self))

    MyComponentResource(opts=ResourceOptions(provider=my_aws_provider))

Both bugs took the form of the `aws.s3.Bucket` above not getting
`my_aws_provider`.

We previously fixed #12161 by keeping using the provider for the
resource only if its package matched the resource's type; otherwise, we
placed it in the providers bag for the children of that resource.

This had the effect of dropping the provider for component resources
because:

- `_get_providers` returns a provider (None for component resources)
  and a bag of providers for children of the resource.
  It is stored on the resource:

  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/resource.py#L866

- A request is made to `register_resource`, which calls
  `prepare_resource`:

  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/resource.py#L880-L882
  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/runtime/resource.py#L811

- `prepare_resource` re-interprets the `provider` and `providers`
  resource options, based on what's in the **original** `opts` struct
  completely ignoring the second result of `_get_providers`:

  https://github.com/pulumi/pulumi/blob/f76bd5463b512222b62d9e60191d93ab2a66290e/sdk/python/lib/pulumi/runtime/resource.py#L210

So in short, the reason the original change broke was:
It turned `provider=x` into `providers=[x]` when appropriate,
but the consuming code did not consume the transformed option.

To fix this, we put the bag of providers produced by `_get_providers`
back into the `opts` struct so that `prepare_resource` and friends can
use it.

Testing:
An integration test is added that defines an MLC,
and uses it from Pulumi programs in Go, Python, and Node
that instantiate the MLC with no provider, provider option, and providers option,
and verifies that resources were instantiated with the correct providers.

Resolves #12593
Resolves #12161


Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants