https://pulumi.com logo
#contribute
Title
# contribute
l

lemon-agent-27707

12/02/2020, 5:39 PM
👋 @bored-oyster-3147 another community member is interested in contributing to the C# Automation API effort. Meet @tall-needle-56640 👋 Are there any areas where you feel like there are opportunities to collaborate or parallelize? Perhaps looking into inline programs and implementation of the gRPC language server as I know you said you might not get to that part of the implementation in your first pass.
b

bored-oyster-3147

12/02/2020, 6:39 PM
Hi @tall-needle-56640. You can definitely start the language server implementation if you want. You can see what I've done so far here. After that is done it's just a matter of wiring it up with the inline entry point in
LocalWorkspace
. @lemon-agent-27707 I think I'm still awaiting a response on some questions I had in my most recent comment on that PR. But I haven't returned to this yet since before thanksgiving as I have been busy. I'm hoping to get back to this this week, either tomorrow or friday. I will be working on the deserialization of project settings when I return, both YAML & JSON. I have been putting it off since it is a little more complex without the duck typing the other platforms have, so I'll need to write some custom converters I think. Shouldn't be too bad though but should be fairly easy for @tall-needle-56640 to work without colliding. How do we want to deal with merging? Is my PR into
pulumi/auto/dotnet
fine as-is?
t

tall-needle-56640

12/02/2020, 6:45 PM
If we are both raising PRs against the same branch, and are not working on the same feature, merges shouldn't really be a problem.
And what is meant by "language server"? Are there any architecture/design docs?
b

bored-oyster-3147

12/02/2020, 6:56 PM
The bullet about adding
InlineProgram
support here
The language server implementation is necessary for a portion of what I've done, so if we merge separately into the same target branch there will need to be a 3rd PR wiring them together. That's fine with me but that's why I was asking
l

lemon-agent-27707

12/02/2020, 7:10 PM
Ah, my apologies! I missed that while I was out last week. I'll take a look at get a response to you today. @tall-needle-56640 the language server is a gRPC service that needs to be implemented in C# to enable inline programs. The typescript reference is here: https://github.com/pulumi/pulumi/blob/master/sdk/nodejs/x/automation/server.ts If an inline program exists in the workspace, then
up
and
preview
spin up this server and invert control with the engine https://github.com/pulumi/pulumi/blob/master/sdk/nodejs/x/automation/stack.ts#L147-L169
t

tall-needle-56640

12/02/2020, 9:18 PM
@bored-oyster-3147 Or, if I'm understanding correctly, I could create a pull request to your branch when it's ready.
b

bored-oyster-3147

12/02/2020, 9:32 PM
that would work too.
t

tall-needle-56640

12/07/2020, 9:26 PM
Is there a C# equivalent to
runtime.runInPulumiStack
? Or does that still need to be developed?
Here its equivalent in Go
👀 1
l

lemon-agent-27707

12/07/2020, 9:44 PM
The equivalent is
Deployment.RunAsync
My understanding is that the guts of the setup is in this private implementation of the deployment class https://github.com/pulumi/pulumi/blob/3d2e31289aba2065853d06024a3da9adfc013202/sdk%2Fdotnet%2FPulumi%2FDeployment%2FDeployment.cs#L72
So the equivalent would be the methods in the Deployment.Runner class: https://github.com/pulumi/pulumi/blob/0bdf73341b83a1437ce061bc62418e80e365ea6c/sdk%2Fdotnet%2FPulumi%2FDeployment%2FDeployment.Runner.cs#L12 Which supports deploying a
Stack
class, or a lambda function.
t

tall-needle-56640

12/07/2020, 10:29 PM
OK Thanks. Now I need to set the config. You mentioned C# having a global variable problem there, but I don't see that. The real problem, I think, is that the config is parsed from env. vars. and cannot otherwise be set. So my solution would be to add an instance method
SetAllConfig(IDictionary)
, so that it can be changed for each deployment instance. Something like:
Copy code
public void SetAllConfig(IDictionary<string, string> config)
{
    AllConfig = config.ToImmutableDictionary();
}
What do you think?
l

lemon-agent-27707

12/07/2020, 10:30 PM
I believe something similar was done for nodejs, this sounds like a reasonable approach to me. cc @tall-librarian-49374 who has greater familiarity with the C# runtime
t

tall-needle-56640

12/08/2020, 5:34 AM
I've done a lot of back and forth between reading NodeJS and Go to try to figure out this config stuff. Go's Run method accepts a custom-defined context parameter. In C#, I get
ServiceCallContext
which is generic. I couldn't find any auto-generated contexts either. Do I need to create my own? NodeJS's Run method sets the config through
runtime.setAllConfig
, but all that does is set a global config variable. But I can't figure out where it is actually used in the context of pulumi.runInPulumiStack. I need direction of which approach to choose and where the config is inserted into the stack/deployment/whatever.
l

lemon-agent-27707

12/08/2020, 6:01 AM
So both in the case of node, and Go the config get to the RPC lang server via the request. In traditional CLI driven pulumi, the language runtime server (pulumi-language-go, pulumi-language-nodejs) sets an environment variable PULUMI_CONFIG so that when the user code executes the SDKs can parse that environment variable to read the config. CLI invokes language runtime (a go program) -> language runtime sets env vars -> language runtime runs user code -> user code parses env var and stores config in the inline program case it's a little different We set up language-native runtime server written in dotnet with a pointer to the user code we want to run -> CLI makes an RPC run request to that server (req contains the config) -> RPC server needs to set the runtime config before invoking the user code. This is the point where config is parsed in the traditional CLI driven flow: https://github.com/pulumi/pulumi/blob/394c91d7f6ab7a4096f4454827690a460f665433/sdk%2Fdotnet%2FPulumi%2FDeployment%2FDeployment_Config.cs#L19 If you look at the nodejs impl, you'll see that it does the same thing with a global: https://github.com/pulumi/pulumi/blob/7d171917ead72d73531927b01e236523a1f7b276/sdk%2Fnodejs%2Fruntime%2Fconfig.ts#L20 The nodejs impl was extended to allow overwriting that global config instead of just parsing it from an env var: https://github.com/pulumi/pulumi/blob/7d171917ead72d73531927b01e236523a1f7b276/sdk%2Fnodejs%2Fruntime%2Fconfig.ts#L32 We'll probably need to extend the internal instance of deployment to allow setting all config. It looks like there is already a method there for setting a single value.
t

tall-needle-56640

12/08/2020, 8:35 PM
Hmmm... I'm thinking about config wrong. What I really need to know is how to inject RunInfo/Options.
l

lemon-agent-27707

12/08/2020, 9:17 PM
You may need to refactor deployment. Right now all these values are set in a private constructor: https://github.com/pulumi/pulumi/blob/3d2e31289aba2065853d06024a3da9adfc013202/sdk%2Fdotnet%2FPulumi%2FDeployment%2FDeployment.cs#L70 Perhaps you can create a new constructor that accepts a set of overrides? Might be able to take some inspiration from how we mock things out for unit testing: https://github.com/pulumi/pulumi/blob/0bdf73341b83a1437ce061bc62418e80e365ea6c/sdk%2Fdotnet%2FPulumi%2FDeployment%2FDeployment_Run.cs#L183 Looks like this is the code that actually creates the deployment in the current CLI workflow: https://github.com/pulumi/pulumi/blob/0bdf73341b83a1437ce061bc62418e80e365ea6c/sdk%2Fdotnet%2FPulumi%2FDeployment%2FDeployment_Run.cs#L217 Feels like some of the context is coming back, I believe @tall-librarian-49374 suggested creating another implementation of Deployment for the inline program use case. Yes looking back at the C# issue there are notes in here about that: https://github.com/pulumi/pulumi/issues/5596
One option here is to create another implementation of 
Deployment
 that is used for 
InlinePrograms
 that has the 
Up
 
Preview
 etc functions exposed. You should only ever be using one form of deployment in a program, either 
Deployment.runAsync
 or 
Deployment.Up
 so this would probably work out fine.
cc @microscopic-pilot-97530 who may have some thoughts on approach here.
b

bored-oyster-3147

12/09/2020, 2:44 PM
do we have an example of what a stack settings file looks like as JSON?
b

bored-oyster-3147

12/09/2020, 5:19 PM
perfect, thank you!
hmmm ok so my dilemma is deserializing stack settings from JSON. since the CLI doesn't seem to support
pulumi config set
as JSON I can't myself get an idea of what the different shapes look like in JSON in order to write deserialization. like the example you linked gives me plain text, and secure text - but what does a plain object and a secure object look like in JSON? since structured config is supported
l

lemon-agent-27707

12/09/2020, 8:00 PM
Current nodejs and go implementations don't support reading/writing structured config (
pulumi config set --path
). It just works with KVPs and requires the user to marshal/unmarshal.
b

bored-oyster-3147

12/09/2020, 8:04 PM
hmmmm ok. What's KVPs? & can you clarify what you mean by marshal/unmarshal? I have seen that referenced in the code but am not familiar with the term. Do I even need to represent a config object in .NET then? Like is this boolean here necessary?
l

lemon-agent-27707

12/09/2020, 8:08 PM
key value pairs. The typescript implementation tries to keep this fairly simple: https://github.com/pulumi/pulumi/blob/9f4461db375cc2cb2785cb8bf4d4ee5cbf8e8e87/sdk%2Fnodejs%2Fx%2Fautomation%2FstackSettings.ts#L20 The idea behind stack settings config is not to be an interface for manipulation, but for importing/exporting.
(a part of keeping it simple is keeping the typing fairly loose, but not lossy if that makes sense).
b

bored-oyster-3147

12/09/2020, 8:12 PM
that makes sense to me, but in .NET the problem with:
export type StackSettingsConfigValue = string | StackSettingsSecureConfigValue | any;
is that
any
case at the end. I would like the .NET implementation to have parity with the TS implementation, but I need some way to represent that final dynamic type case that isn't too ugly.
l

lemon-agent-27707

12/09/2020, 8:17 PM
Hm, I wonder if it would be appropriate to treat it as opaque data. For instance go has a json.RawMessage type: https://golang.org/pkg/encoding/json/#RawMessage
b

bored-oyster-3147

12/09/2020, 8:25 PM
that could work, the
System.Text.Json
equivalent is
JsonElement
I think, so if it is an object we could just deserialize it as that and let the consumer figure it out. It means that our
StackSettingsConfigValue
would now be aware of whether the underlying config was JSON or YAML, because it would also need whatever the
Yaml.Dotnet
equivalent of
JsonElement
is. So our config value type would be something like:
Copy code
{
    string? Value,
    JsonElement? ValueJson,
    YamlElement? ValueYaml,
    bool IsSecure,
    bool IsObject,
}
side note i now realize that marshal/unmarshal is GO speak for serialize/deserialize? haha everyone needs their own words I guess
l

lemon-agent-27707

12/09/2020, 9:17 PM
I don't think we should leak json/yaml into the type. Is there something one level removed?
Alternatively, if we're dealing with YAML just translate YAMLElement -> JSONElement
That is effectively what the typescript implementation does, parses YAML into an "any" which is just an untyped javascript object.
Maybe deserializing to
Dictionary<string, Object>
?
b

bored-oyster-3147

12/09/2020, 10:26 PM
I think that
Dictionary<string, object>
might work for JSON deserialization. Will need to experiment with that. I'll let you know
t

tall-needle-56640

12/10/2020, 11:28 PM
I'm walking through this example and the automation code. And I don't ever see the language server used in any meaningful way. I am very confused.
l

lemon-agent-27707

12/11/2020, 1:28 AM
The presence of
program
in the arguments (
InlineProgramArguments
) causes the stack and the workspace to operate in "inline" program mode: https://github.com/pulumi/automation-api-examples/blob/main/nodejs/inlineProgram-ts/index.ts#L69 When
stack.up
is called, we check to see a program was specified (rather than relying on a pulumi project on disk): https://github.com/pulumi/pulumi/blob/master/sdk/nodejs/x/automation/stack.ts#L147 When we do have a program (a function pointer), we start the language server: https://github.com/pulumi/pulumi/blob/master/sdk/nodejs/x/automation/stack.ts#L152
The package level overview from the Go docs offers a nice mental model: https://pkg.go.dev/github.com/pulumi/pulumi/sdk/v2/go/x/auto
t

tall-needle-56640

12/11/2020, 8:39 PM
Right but in that same block
languageServer.Run
is not being used. Is something else calling it? If so, from where?
l

lemon-agent-27707

12/11/2020, 8:41 PM
The port is passed to the pulumi CLI to establish an RPC connection: https://github.com/pulumi/pulumi/blob/master/sdk/nodejs/x/automation/stack.ts#L168
Then the pulumi engine can make the rpc Run request.
t

tall-needle-56640

12/11/2020, 8:44 PM
Oh OK. I get it now. Sorry for all the questions, I'm learning many things at the same time. lol But I appreciate the help (and patience).
l

lemon-agent-27707

12/11/2020, 8:45 PM
You are learning the guts of the pulumi architecture! I think it's awesome partypus 8bit
t

tall-needle-56640

12/11/2020, 8:56 PM
👍
@lemon-agent-27707 Can I get some feedback on my pull request when you get a chance? The PR is to @bored-oyster-3147’s repo since I rely on his PR, but it would be good to get feedback. I think it's close.
b

bored-oyster-3147

12/15/2020, 8:29 PM
I have some thoughts / questions if that's OK: 1. I think the new
Deployment
partial should reside up by the other
Deployment
partials if we're going to keep it a partial. Rather than in the Automation directory. So maybe
X/Deployment/Deployment_InlineProgram.cs
? 2. I think the LanguageServer file should be called
LanguageServer.cs
rather than
Server.cs
3. Why is
LanguageServer<T>
generic? It doesn't look like we're using the type argument anywhere. those are largely styling thoughts I had on first glance.. I will need to figure out the functionality to remark on anything else
I'm going to start putting together the Up/Preview/Destroy methods on
XStack
later today, and then maybe you can merge with mine again and we can wire it up as part of the same PR? It's all coming together though!
t

tall-needle-56640

12/15/2020, 8:43 PM
Cool. 1. I wanted to get some more feedback before deciding on a location. 2. It's named Server.cs only because the other languages named it that. But I completely agree with you, LanguageServer.cs makes more sense. 3. Thanks. That was a relic from the nodejs code.
b

bored-oyster-3147

12/15/2020, 8:43 PM
gotcha I didn't realize the others called it Server.cs - you were right to name it that then
l

lemon-agent-27707

12/15/2020, 9:34 PM
@tall-needle-56640 Took a brief look and looks structurally in the right direction. There doesn't appear to be any concurrency control over Run() as there is in the go impl. I think in terms of giving a deeper review, it would be helpful to see the implementation of Stack.Up(), how the lifecycle of the grpc server is managed, and a sample program using an inline deployment to see everything end to end.
t

tall-needle-56640

12/15/2020, 9:52 PM
@lemon-agent-27707 Is the scenario that
Stack.Up()
is called concurrently?
l

lemon-agent-27707

12/15/2020, 9:56 PM
I believe it's that the CLI makes duplicate requests to the Run() gRPC endpoint.
The CLI could** make duplicate requests. You may also need to implement the C# equivalent of close (this might be shutdown?)
b

bored-oyster-3147

12/23/2020, 3:36 PM
Hi @tall-needle-56640 I merged your language server implementation and I'm trying to plot out how to wire it up but I think I'm missing something. Did you have a working example? If we look here in this
Stack.Up
implementation they are starting a GRPC Server & the LanguageServer side-by-side. They then wire them together and handle the closing / shutdown of the server. It looks like though that our LanguageServer implementation is instantiating an instance of
Deployment
with your new
Deployment_Inline
constructor, and within that a new GRPC Server is instantiated as well. Is that intended? What am I missing?
t

tall-needle-56640

12/23/2020, 8:37 PM
This is my first gRPC project, but, from what I can tell, GrpcEngine (in C#) is not providing the same functionality as
grpc
(in TS). It seems that GrpcEngine has very limited functionaility, and doesn't have a way to add the LanguageServer to it, like in TS (i.e.
server.addService()
).
@bored-oyster-3147 I never had a working example because I didn't think my piece was testable in isolation. I created a sample app that simply ran
Copy code
var stack = await LocalWorkspace.SelectStackAsync(new InlineProgramArgs("projectName", "dev", () => Console.WriteLine("ProgramArgs")) { WorkDir = @"C:\Pulumi\functionapp" });
But I get the error
Pulumi.X.Automation.Commands.Exceptions.CommandException:
could not get cloud url: could not load current project: project is missing a 'name' attribute
So it's trying to execute the pulumi CLI before I could possibly call an
Up()
method (if it existed). So where would the gPRC service even get a chance to spin up? I seem to be missing something as well. lol
l

lemon-agent-27707

12/24/2020, 11:38 AM
Does that workdir have a pulumi.yaml?
b

bored-oyster-3147

12/24/2020, 2:22 PM
I see - I think I’ll need to try to get a deeper understanding of the gRPC/languageserver bits before I make this connection. Thanks for the info. It does look like the error you were getting was due to not having a project file or stack setup before selecting? Could try the CreateOrSelect method? But you are correct that the methods that would actually spin up the server don’t exist yet, that’s the final bit I gotta work on.
t

tall-needle-56640

12/24/2020, 2:32 PM
@lemon-agent-27707 @bored-oyster-3147 Ah... something altered my Pulumi.yaml file:
Copy code
-name: PulumiPort
-runtime: dotnet
-description: A minimal Azure C# Pulumi program
+Name: asd
+Runtime:
+  Name: NodeJS
+  Options:
+Main:
+Description:
+Author:
+Website:
+License:
+Config:
+Template:
+Backend:
@bored-oyster-3147 The capitalization seems to be making it fail. Any idea what is updating 'Pulumi.yaml'?
b

bored-oyster-3147

12/24/2020, 3:10 PM
Hmmm yea I have an idea of what may be doing that. Serialization should be ignoring null values and the property name serialization needs a lowercase naming policy. But, we decided to defer serialization til later so I gotta pull all that out and I don’t think that will be happening anymore.
But after the holiday, merry Christmas y’all
t

tall-needle-56640

12/24/2020, 3:19 PM
Merry Christmas!
@bored-oyster-3147 What won't be happening anymore?
b

bored-oyster-3147

12/28/2020, 10:28 PM
well it shouldn't be overwriting an existing yaml file, it shouldn't be serializing null properties, & it shouldn't be serializing property names capitalized
I think what you see is a side-effect of the half-started serialization that existed when you forked
@tall-needle-56640 @lemon-agent-27707 sorry I've been busy so this hasn't been as fast going as I'd lile. But I enlisted a friend to help kickstart the GRPC server bit. We need to spin up a temporary web host for that part. The very bare bones of that is in my draft PR. I'm currently working on the Stack action APIs, so I'm putting together the various
Options
objects for the actions as well as the
Result
objects. Then I will be wiring it together.
So, more progress tomorrow now that I am moving and have good direction. I stripped out the serialization public APIs, all that remains is what was going on internally.
t

tall-needle-56640

01/04/2021, 10:17 PM
@bored-oyster-3147 Did you see my PR with the gRPC server implementation?
b

bored-oyster-3147

01/04/2021, 10:17 PM
The need for a web host for the GRPC bit brings in a AspNetCore reference at the bare minimum. So that leads back to questions about maybe splitting automation API into a separate class library? Will need thoughts from pulumi team on that
@tall-needle-56640 oh wow I'm sorry I did not see that. my bad! But it is very similar to the beginning of what I have going on here https://github.com/orionstudt/pulumi/blob/8f5919c022e48117c57c232159e90eaec351caaf/sdk/dotnet/Pulumi/X/Automation/XStack.cs#L189
We needed to do something special to hook in and retrieve the dynamic port that was assigned but otherwise it is basically what you are doing there. I will add some further abstraction so we are not repeating ourselves in Up/Preview/Destroy etc
t

tall-needle-56640

01/04/2021, 10:22 PM
@bored-oyster-3147 Sounds good!
Anything I can work on?
l

lemon-agent-27707

01/04/2021, 10:25 PM
Heads up that programs are only run during up/preview, so the gRPC server won't be needed for refresh/destroy.
b

bored-oyster-3147

01/04/2021, 10:27 PM
@lemon-agent-27707 noted, I think I remember that being the case. @tall-needle-56640 I will be fleshing out the stack actions some more tomorrow so we will be colliding there. But things that remain from the original PR that you can maybe look at are: • Workspace cancel/export/import implementation • RemoteProgram implementation