pulumi / pulumi

Pulumi - Infrastructure as Code in any programming language 🚀
https://www.pulumi.com
Apache License 2.0
21.8k stars 1.12k forks source link

Python dynamic provider needs better error message when accessing missing key in props #10040

Open juicemia opened 2 years ago

juicemia commented 2 years ago

What happened?

I have the following dynamic provider:

class IstioMeshProvider(ResourceProvider):
    def create(self, props):
        version = props['version']
        revision = version.replace('.', '-')

        proc = subprocess.run([f"istioctl-{version}", 'install', '-y', '--set', f"revision={revision}"])
        if proc.returncode != 0:
            raise Exception(f"Unable to install Istio service mesh. \n\nSTDOUT:\n\n{proc.stdout}\n\nSTDERR:\n\n{proc.stderr}")

        return CreateResult("istioctl-"+binascii.b2a_hex(os.urandom(16)).decode("utf-8"), outs={
            version: version,
            revision: revision
        })

    def delete(self, id, props):
        version = props['version']
        revision = version.replace('.', '-')

        proc = subprocess.run([f"istioctl-{version}", 'x', 'uninstall', '-y', '--set', f"revision={revision}"])
        if proc.returncode != 0:
            raise Exception(f"Unable to uninstall Istio service mesh. \n\nSTDOUT:\n\n{proc.stdout}\n\nSTDERR:\n\n{proc.stderr}")

    def diff(self, id, old_inputs, new_inputs):
        return DiffResult(
            changes=old_inputs != new_inputs,
            replaces=[],
            stables=None,
            delete_before_replace=False
        )

    def update(self, id, old_inputs, new_inputs):
        version = new_inputs['version']
        revision = version.replace('.', '-')

        proc = subprocess.run([f"istioctl-{version}", 'install', '-y', '--set', f"revision={revision}"])
        if proc.returncode != 0:
            raise Exception(f"Unable to uninstall Istio service mesh. \n\nSTDOUT:\n\n{proc.stdout}\n\nSTDERR:\n\n{proc.stderr}")

        return UpdateResult(
            outs={
                version: version,
                revision: revision
            }
        )

class IstioMeshArgs(object):
    # TODO: add args for operator manifest, kubeconfig
    version: Input[str]

    def __init__(self, version):
        self.version = version

class IstioMesh(Resource):
    version: Output[str]
    revision: Output[str]

    def __init__(self, name: str, args: IstioMeshArgs, opts = None):
        full_args = {**vars(args)}
        super().__init__(IstioMeshProvider(), name, full_args, opts)

I'm calling it like this:

IstioMesh('mesh', IstioMeshArgs(version='1.13.5'))

When I run pulumi up to create the mesh it works. If I then destroy it it works. I destroy it like this:

pulumi destroy -t urn:pulumi:staging::quickstart::pulumi-python:dynamic:Resource::mesh

If I create the mesh with version set to '1.13.5', then change the version of the mesh to '1.14.1', the update works. However, If I run pulumi destroy -t urn:pulumi:staging::quickstart::pulumi-python:dynamic:Resource::mesh it fails with the following output:

Previewing destroy (staging):
     Type                               Name                Plan
     pulumi:pulumi:Stack                quickstart-staging
 -   └─ pulumi-python:dynamic:Resource  mesh                delete

Resources:
    - 1 to delete

Do you want to perform this destroy? yes
Destroying (staging):
     Type                               Name                Status                  Info
     pulumi:pulumi:Stack                quickstart-staging  **failed**              1 error; 1 message
 -   └─ pulumi-python:dynamic:Resource  mesh                **deleting failed**     1 error

Diagnostics:
  pulumi-python:dynamic:Resource (mesh):
    error: Exception calling application: 'version'

  pulumi:pulumi:Stack (quickstart-staging):
    E0703 18:22:20.039514000 6142881792 fork_posix.cc:76]                  Other threads are currently calling into gRPC, skipping fork() handlers

    error: update failed

Resources:

Duration: 2s

I would expect the destroy to work the same way it works if I don't trigger an update first.

Steps to reproduce

  1. Have istioctl-1.13.5 and istioctl-1.14.1 in your $PATH.
  2. Apply the following to a bare Kubernetes cluster that supports Istio:
class IstioMeshProvider(ResourceProvider):
    def create(self, props):
        version = props['version']
        revision = version.replace('.', '-')

        proc = subprocess.run([f"istioctl-{version}", 'install', '-y', '--set', f"revision={revision}"])
        if proc.returncode != 0:
            raise Exception(f"Unable to install Istio service mesh. \n\nSTDOUT:\n\n{proc.stdout}\n\nSTDERR:\n\n{proc.stderr}")

        return CreateResult("istioctl-"+binascii.b2a_hex(os.urandom(16)).decode("utf-8"), outs={
            version: version,
            revision: revision
        })

    def delete(self, id, props):
        version = props['version']
        revision = version.replace('.', '-')

        proc = subprocess.run([f"istioctl-{version}", 'x', 'uninstall', '-y', '--set', f"revision={revision}"])
        if proc.returncode != 0:
            raise Exception(f"Unable to uninstall Istio service mesh. \n\nSTDOUT:\n\n{proc.stdout}\n\nSTDERR:\n\n{proc.stderr}")

    def diff(self, id, old_inputs, new_inputs):
        return DiffResult(
            changes=old_inputs != new_inputs,
            replaces=[],
            stables=None,
            delete_before_replace=False
        )

    def update(self, id, old_inputs, new_inputs):
        version = new_inputs['version']
        revision = version.replace('.', '-')

        proc = subprocess.run([f"istioctl-{version}", 'install', '-y', '--set', f"revision={revision}"])
        if proc.returncode != 0:
            raise Exception(f"Unable to uninstall Istio service mesh. \n\nSTDOUT:\n\n{proc.stdout}\n\nSTDERR:\n\n{proc.stderr}")

        return UpdateResult(
            outs={
                version: version,
                revision: revision
            }
        )

class IstioMeshArgs(object):
    # TODO: add args for operator manifest, kubeconfig
    version: Input[str]

    def __init__(self, version):
        self.version = version

class IstioMesh(Resource):
    version: Output[str]
    revision: Output[str]

    def __init__(self, name: str, args: IstioMeshArgs, opts = None):
        full_args = {**vars(args)}
        super().__init__(IstioMeshProvider(), name, full_args, opts)

IstioMesh('mesh', IstioMeshArgs(version='1.13.5'))
# IstioMesh('mesh', IstioMeshArgs(version='1.14.1'))
  1. Uncomment the line for version 1.14.1 and comment the line for 1.13.5, then run pulumi up to update the Istio version on the Kubernetes cluster.
  2. Run pulumi destroy -t $MESH_URN to destroy the mesh.

Expected Behavior

The mesh resource is deleted along with the underlying Istio installation.

Actual Behavior

The destroy operation fails with this output:

Previewing destroy (staging):
     Type                               Name                Plan
     pulumi:pulumi:Stack                quickstart-staging
 -   └─ pulumi-python:dynamic:Resource  mesh                delete

Resources:
    - 1 to delete

Do you want to perform this destroy? yes
Destroying (staging):
     Type                               Name                Status                  Info
     pulumi:pulumi:Stack                quickstart-staging  **failed**              1 error; 1 message
 -   └─ pulumi-python:dynamic:Resource  mesh                **deleting failed**     1 error

Diagnostics:
  pulumi-python:dynamic:Resource (mesh):
    error: Exception calling application: 'version'

  pulumi:pulumi:Stack (quickstart-staging):
    E0703 18:22:20.039514000 6142881792 fork_posix.cc:76]                  Other threads are currently calling into gRPC, skipping fork() handlers

    error: update failed

Resources:

Duration: 2s

Versions used

CLI          
Version      3.35.3
Go Version   go1.17.11
Go Compiler  gc

Plugins
NAME    VERSION
gcp     6.29.0
python  unknown

Host     
OS       darwin
Version  12.3
Arch     arm64

This project is written in python: executable='/Users/hugotorres/.pyenv/shims/python3' version='3.10.4'

Current Stack: staging

TYPE                               URN
pulumi:pulumi:Stack                urn:pulumi:staging::quickstart::pulumi:pulumi:Stack::quickstart-staging
pulumi:providers:gcp               urn:pulumi:staging::quickstart::pulumi:providers:gcp::default_6_29_0
gcp:compute/address:Address        urn:pulumi:staging::quickstart::gcp:compute/address:Address::wl-us-nat-0
gcp:compute/network:Network        urn:pulumi:staging::quickstart::gcp:compute/network:Network::wl-us-vpc
gcp:compute/address:Address        urn:pulumi:staging::quickstart::gcp:compute/address:Address::wl-us-nat-1
pulumi:providers:pulumi-python     urn:pulumi:staging::quickstart::pulumi:providers:pulumi-python::default
gcp:compute/firewall:Firewall      urn:pulumi:staging::quickstart::gcp:compute/firewall:Firewall::wl-us-admin-whitelist
gcp:compute/firewall:Firewall      urn:pulumi:staging::quickstart::gcp:compute/firewall:Firewall::wl-us-deny-default
gcp:compute/router:Router          urn:pulumi:staging::quickstart::gcp:compute/router:Router::wl-us-router
gcp:compute/subnetwork:Subnetwork  urn:pulumi:staging::quickstart::gcp:compute/subnetwork:Subnetwork::primary-subnet
gcp:compute/firewall:Firewall      urn:pulumi:staging::quickstart::gcp:compute/firewall:Firewall::primary-us-central1-internal-any-any
gcp:container/cluster:Cluster      urn:pulumi:staging::quickstart::gcp:container/cluster:Cluster::wl-us
gcp:compute/routerNat:RouterNat    urn:pulumi:staging::quickstart::gcp:compute/routerNat:RouterNat::wl-us-router-nat
gcp:container/nodePool:NodePool    urn:pulumi:staging::quickstart::gcp:container/nodePool:NodePool::primary
pulumi-python:dynamic:Resource     urn:pulumi:staging::quickstart::pulumi-python:dynamic:Resource::mesh

Found no pending operations associated with staging

Backend        
Name           hugos-mbp.lan
URL            gs://ht-test-bucket
User           hugo
Organizations  

Dependencies:
NAME        VERSION
autopep8    1.6.0
pip         22.1.2
pulumi-gcp  6.29.0
setuptools  62.6.0
wheel       0.37.1

Pulumi locates its logs in /var/folders/v_/7cfnsw_n7cbgl0vwrt48jl5w0000gn/T/ by default

Additional context

I just started playing with Pulumi over the July 4th weekend. This isn't production code but I'm exploring moving off of Terraform for managing our Kubernetes cluster because managing Istio is a pain.

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

juicemia commented 2 years ago

Just an update, I tried running it again today and this time I couldn't delete the IstioMesh even before upgrading, so I doubt delete ever worked at all.

juicemia commented 2 years ago

Another update: I tried changing the name of the version input just to see if I could isolate the issue to that, and sure enough, it seems like that's the problem.

Changed the code to the following:

class IstioMeshProvider(ResourceProvider):
    def create(self, props):
        version = props['foo']
        revision = version.replace('.', '-')

        proc = subprocess.run([f"istioctl-{version}", 'install', '-y', '--set', f"revision={revision}"])
        if proc.returncode != 0:
            raise Exception(f"Unable to install Istio service mesh. \n\nSTDOUT:\n\n{proc.stdout}\n\nSTDERR:\n\n{proc.stderr}")

        return CreateResult("istioctl-"+binascii.b2a_hex(os.urandom(16)).decode("utf-8"), outs={
            version: version,
            revision: revision
        })

    def delete(self, id, props):
        version = props['foo']
        revision = version.replace('.', '-')

        proc = subprocess.run([f"istioctl-{version}", 'x', 'uninstall', '-y', '--revision', revision])
        if proc.returncode != 0:
            raise Exception(f"Unable to uninstall Istio service mesh. \n\nSTDOUT:\n\n{proc.stdout}\n\nSTDERR:\n\n{proc.stderr}")

    def diff(self, id, old_inputs, new_inputs):
        return DiffResult(
            changes=old_inputs != new_inputs,
            # If we want to do a completely automated upgrade of Istio to a new revision, which includes cleaning up
            # the old revision by deleting it completely, then we should set `version` in the `replaces` so that Pulumi
            # knows to run the provider's `delete` implementation. However, having the process completely automated can
            # be dangerous because there's no verification by a human operator that the new mesh is actually working. If
            # we don't want to do a completely automated upgrade, but instead just install a new revision and then manually
            # roll over the deployments once the new revision is manually verified to be working, we can do this. This
            # `DiffResult` will tell Pulumi to run the `update`, which can then install the new Istio revision and stop there.
            replaces=[],
            stables=None,
            delete_before_replace=False
        )

    def update(self, id, old_inputs, new_inputs):
        version = new_inputs['foo']
        revision = version.replace('.', '-')

        proc = subprocess.run([f"istioctl-{version}", 'install', '-y', '--set', f"revision={revision}"])
        if proc.returncode != 0:
            raise Exception(f"Unable to uninstall Istio service mesh. \n\nSTDOUT:\n\n{proc.stdout}\n\nSTDERR:\n\n{proc.stderr}")

        return UpdateResult(
            outs={
                version: version,
                revision: revision
            }
        )

class IstioMeshArgs(object):
    # TODO: add args for operator manifest, kubeconfig
    foo: Input[str]

    def __init__(self, foo):
        self.foo = foo

class IstioMesh(Resource):
    version: Output[str]
    revision: Output[str]

    def __init__(self, name: str, args: IstioMeshArgs, opts = None):
        full_args = {**vars(args)}
        super().__init__(IstioMeshProvider(), name, full_args, opts)

That got me this error:

Diagnostics:
  pulumi-python:dynamic:Resource (mesh):
    error: Exception calling application: 'foo'

  pulumi:pulumi:Stack (quickstart-staging):
    E0705 16:33:23.975474000 6119305216 fork_posix.cc:76]                  Other threads are currently calling into gRPC, skipping fork() handlers

    error: update failed
juicemia commented 2 years ago

I figured out my issue.

This:


        return UpdateResult(
            outs={
                version: version,
                revision: revision
            }
        )

should be this:


        return UpdateResult(
            outs={
                'version': version,
                'revision': revision
            }
        )

I think there's an opportunity for a better error message here.

Exception calling application: 'version' doesn't tell the user much.

after-ephemera commented 1 year ago

I started seeing this today with a bad dictionary access as well. It would be nice to have a clearer error message here.