Allow attaching to running providers #8979
Conversation
|
|
||
| // Attach triggers an attach for a currently running provider to the engine | ||
| // TODO It would be nice if this was a HostClient rather than the string address but due to dependency ordering we don't have access to declare that here. | ||
| Attach(address string) error |
There was a problem hiding this comment.
Can this be an implementation detail of the gRPC implementation of plugins rather than part of the Plugin type?
There was a problem hiding this comment.
Maybe? I thought about maybe having a GrpcProvider that was just Provider + Attach? Is there a more "go" way to do it?
There was a problem hiding this comment.
Yeah, I'm wondering whether or not we need to expose Attach at all. I think we could just invoke the gRPC method directly here: https://github.com/pulumi/pulumi/pull/8979/files#diff-4372cd77587542c3e09139fb236d0bce7d0ff31c3c81864950974b1e33e5104eR146
|
Thinking aloud a bit: Another way to achieve this w/o touching the core plugin host would be to implement this as a proxy host, like we do for the I also wonder if the envvar might be more ergonomic if it we a list of (name, version?, port) tuples. Something like One other thing: have you tried this when running an update where multiple instances of a particular provider are needed? I'm not sure how this fits into that scenario, as today each instance of a provider implies a new process. |
Yeh happy to change to that style. I'm not sure versions matter here, I see it similar to how if you have like "pulumi-resource-aws" on path we just ignore the version and use the one you've got on path. Which I guess also answers your other question of what happens if you have multiple instances, well you don't. Same as if it was on path. |
Codecov Report
@@ Coverage Diff @@
## master #8979 +/- ##
==========================================
+ Coverage 59.11% 59.30% +0.18%
==========================================
Files 646 643 -3
Lines 100254 100073 -181
Branches 1420 1427 +7
==========================================
+ Hits 59262 59345 +83
+ Misses 37411 37333 -78
+ Partials 3581 3395 -186
Continue to review full report at Codecov.
|
| @@ -2171,6 +2172,8 @@ type ResourceProviderClient interface { | |||
| Cancel(ctx context.Context, in *empty.Empty, opts ...grpc.CallOption) (*empty.Empty, error) | |||
| // GetPluginInfo returns generic information about this plugin, like its version. | |||
| GetPluginInfo(ctx context.Context, in *empty.Empty, opts ...grpc.CallOption) (*PluginInfo, error) | |||
| // Attach sends the engine address to an already running plugin. | |||
| Attach(ctx context.Context, in *PluginAttach, opts ...grpc.CallOption) (*empty.Empty, error) | |||
There was a problem hiding this comment.
This is going to break implementers of ResourceProviderClient. Which if we're going to have to push out a break change we may as well change NewProviderServer to take a GrpcProvider as well.
Still curious about this. |
I'll have a look. I thought we cached processes per provider name, but I'll dig into this more. |
|
So with attach it will redial the grpc connection each time and call Attach (with the same engine address). So it would also call Configure multiple times. |
Description
This adds an envvar check to attach to a currently running provider rather than spawning a new provider binary. This lets you start for example the aws provider in a debugger let it start listening in on a port and then running pulumi up with an env var set to use that running provider for aws
PULUMI_PROVIDER_AWS=<port>Engine side part of a fix for #2396
Checklist