mirror of https://github.com/pulumi/pulumi.git
6 Commits
Author | SHA1 | Message | Date |
---|---|---|---|
Thomas Gummerer |
fc10da33d9
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com> |
|
Thomas Gummerer |
477a54b3de
|
deploytest/RegisterResource: return struct instead of values (#15988)
This method already returns 4 different values, and we want to add more. Refactor it so it returns a struct, to make adding additional return values easier in the future. |
|
Fraser Waters |
ce04d43792
|
Save component resources as parents (#15846)
<!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/15843. This tracks parent information separate from Goal information (as that was only generated and saved for custom, not component, resources). We also need to keep track of the transform list between the first RegisterResource starting a remote component construction and the remote component _actually_ registering itself. This means we save the transform list correctly once the component is registered inside the remote process allowing other resource registrations in the remote process to see that saved transform list. If we just wait till the Construct call is done then none of the inner children pick up the transform list. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. --> |
|
Fraser Waters |
d5b189f81b
|
Add a test to show transforms can error (#15848)
<!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Just adding a sanity test that this workflow behaves correctly in the engine. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. --> |
|
Fraser Waters |
f749c2cb24
|
Send output values to transforms for dependency tracking (#15637)
<!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This engine change is necessary for correct dependency tracking of properties through transform functions. Unlike other parts of the runtime/provider/engine interface transform functions do not have a property dependencies map sent or returned. Transforms rely entirely on the dependency arrays in output values for their dependency tracking. This change takes the property dependency map and upgrades all the input properties to be output values tracking those same dependencies (if a property doesn't have any dependencies it doesn't need to be upgraded to an output value). This upgrade is only done if we're using transform functions, there's currently no need to do this unless we're using transforms. After running the transforms we downgrade the output values back to plain/secret/computed values if we're registering a custom resource. If we're registering a component resource we just leave all the properties upgraded as output values. We also rebuild the resources dependencies slice and propertyDependency map with the dependencies recorded in the transformed properties. If the transform didn't change the properties this will just be the same data we encoded into the output values in the properties structure. There are a couple of things to keep in mind with this change. Firstly using a transform can cause a property that was being sent to a component provider as just a `resource.NumberProperty` to start being sent as an `OutputProperty` with the `NumberProperty` as its element. Component providers _should_ handle that, but it's feasible that it could break some component code. This is a fairly limited blast radius as it will only happen when that user starts using a transform function. Secondly, we can't know in the engine if a property dependencies are from a top level output value or a nested output. That is SDKs send both of the following property shapes the same way to the engine for custom resources: ``` A) Output(Array([Number(1)]), ["dep"]) --- B) Array([Output(Number(1), ["dep"]) ``` Currently both would get upgraded to `Output(Array([Number(1)]), ["dep"])`. For a component resource the SDK will send output values, and as long as they define a superset of the dependencies in the property map then we don't add the top-level output tracking the same dependency set. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [ ] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. --> |
|
Fraser Waters |
7422c44ca4
|
Engine support for remote transforms (#15290)
<!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This adds support to the engine for "remote transformations". A transform is "remote" because it is being invoked via the engine on receiving a resource registration, rather than being ran locally in process before sending a resource registration. These transforms can also span multiple process boundaries, e.g. a transform function in a user program, then a transform function in a component library, both running for a resource registered by another component library. The underlying new feature here is the idea of a `Callback`. The expectation is we're going to use callbacks for multiple features so these are _not_ defined in terms of transformations. A callback is an untyped byte array (usually will be a protobuf message), plus an address to define which server should be invoked to do the callback, and a token to identify it. A language sdk can start up and serve a `Callbacks` service, keep a mapping of tokens to in-process functions (currently just using UUID's for this), and then pass that service address and token to the engine to be invoked later on. The engine uses these callbacks to track transformations callbacks per resource, and on a new resource registrations invokes each relevant callback with the resource properties and options, having new properties and options returned that are then passed to the next relevant transform callback until all have been called and the engine has the final resource state and options to use. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. --> |