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

[pkov2] agent RPC server #624

Merged
merged 22 commits into from
Aug 8, 2024
Merged

[pkov2] agent RPC server #624

merged 22 commits into from
Aug 8, 2024

Conversation

EronWright
Copy link
Contributor

@EronWright EronWright commented Aug 2, 2024

Proposed changes

Epic Link: #606

Demo Video Link: https://pulumi.slack.com/archives/C07DQSV84DC/p1722636430008649

Implements an agent consisting of two commands:

  • init - fetches a flux source into the given directory, intended for use in an init container.
  • serve - starts an RPC server providing an automation API to perform stack updates over the given workspace.

Overview

The RPC server assumes that the project source code has been checked out to a local working directory, called the "workspace" directory. This generally corresponds to a sub-directory within a git repository, e.g. examples/random-yaml.

At startup, the server opens the workspace using auto.NewLocalWorkspace. All RPC operations are applied to this same workspace, usually one-at-a-time. Some operations cause state changes, e.g. stack selection, that may affect subsequent operations. Some operations produce PreconditionFailed if a stack hasn't been selected.

At startup, the server optionally runs pulumi install to install dependencies and plugins for the project, based on pulumi/pulumi#16782. Note that PKOv1 has some code to install plugins, now believed to be obsolete (see discussion).

The supported operations are:

  • WhoAmI - returns current user info.
  • Install - runs pulumi install in the workspace.
  • SelectStack - select (and optionally create) a stack, for use in subsequent operations.
  • Info - a summary of the current stack.
  • SetAllConfig - set multiple configuration values on the current stack, based on literals, environment variables, and file references. It is expected that the server's pod would have ConfigMaps and Secrets mounted accordingly.
  • Preview runs the preview operation for the current stack.
  • Up runs the up operation for the current stack.
  • Destroy runs the destroy operation for the current stack.
  • Refresh runs the refresh operation for the current stack.

The deployment operations have streaming responses, consisting of a series of engine events and a final result.

The agent uses zap for logging, because it supports structured logging, implements io.Writer to capture Pulumi console output, and integrates well with grpc-go.

Follow-ups

  • Write RPC server tests
  • Rename 'init' to 'fetch' for clarity
  • lock the workspace during an operation? Or rely on locking within the Pulumi CLI?

Related issues (optional)

Closes #610 #611

@EronWright EronWright requested a review from a team August 2, 2024 17:44
@EronWright EronWright added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Aug 2, 2024
@EronWright EronWright marked this pull request as ready for review August 6, 2024 15:30
Copy link
Contributor

@blampe blampe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an awesome start! Most of my feedback is nits or structural suggestions that don't need to block.

@@ -0,0 +1,54 @@
# Build the agent binary
FROM golang:1.22 AS builder
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FROM golang:1.22 AS builder
FROM --platform=$BUILDPLATFORM golang:1.22 AS builder

This will build in a native image instead of an emulated one when cross-compiling.

agent/version/version.go Show resolved Hide resolved
go.mod Show resolved Hide resolved
@@ -0,0 +1,114 @@
module github.com/pulumi/pulumi-kubernetes-operator/agent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Structure nits:

  • We should put the new code under a v2 subdirectory.
  • We should try as hard as possible to keep one go.mod at the root of the repo, unless there's a legitimate version incompatibility that's non-trivial to resolve between v1 and v2. (Is there?)
  • If we absolutely need a new module, it should be under v2 so the agent and server both belong to it since they're versioned together.

Unnecessary modules are a big source of accidental complexity and lead to all sorts of headaches and gotchas down the line (example). Trust me on this one :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to tackle the larger structural stuff in a follow-up.

agent/go.mod Show resolved Hide resolved
agent/pkg/proto/agent.proto Outdated Show resolved Hide resolved
agent/pkg/proto/agent.proto Outdated Show resolved Hide resolved
agent/pkg/proto/agent.proto Show resolved Hide resolved
Comment on lines +94 to +95
optional bool path =
1; // Parse the keys as paths in a map or list rather than raw strings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right in thinking this basically resolves the structure config problem by letting you mount any config map you want into the pod and use that as your stack config directly? Does that mean SetAllConfig would only get invoked once per operation?

Copy link
Contributor Author

@EronWright EronWright Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This SetAllConfig RPC method closely mirrors the underlying auto api, that takes a path bool to indicate that the keys in the provided map are "paths" as opposed to literal config keys. What's special here is that the RPC method is able to resolve "valueFrom" refs that draw values from environment variables and local files.

The Stack controller calls SetAllConfig today after extracting values from configmaps and secrets. In the v2 design, the controller doesn't see the values. The configmaps/secrets are mounted onto the workspace pod (as envs and/or files) and then read via these "valueFrom" refs.

I'm not seeking to solve structured config beyond what auto api can do today.

agent/pkg/server/server_test.go Show resolved Hide resolved
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 17.14286% with 1624 lines in your changes missing coverage. Please review.

Project coverage is 17.14%. Comparing base (13b81b8) to head (6c55ee2).

Files Patch % Lines
agent/pkg/proto/agent.pb.go 7.43% 1082 Missing and 14 partials ⚠️
agent/pkg/proto/agent_grpc.pb.go 0.00% 210 Missing ⚠️
agent/pkg/server/server.go 60.34% 96 Missing and 67 partials ⚠️
agent/cmd/serve.go 0.00% 101 Missing ⚠️
agent/cmd/init.go 0.00% 26 Missing ⚠️
agent/cmd/root.go 0.00% 21 Missing ⚠️
agent/cmd/version.go 0.00% 5 Missing ⚠️
agent/main.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              v2     #624      +/-   ##
=========================================
+ Coverage   8.76%   17.14%   +8.37%     
=========================================
  Files          4        8       +4     
  Lines       1164     1960     +796     
=========================================
+ Hits         102      336     +234     
- Misses      1055     1543     +488     
- Partials       7       81      +74     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@EronWright EronWright merged commit 5b5d8a7 into v2 Aug 8, 2024
6 checks passed
@EronWright EronWright deleted the v2-agent-serve branch August 8, 2024 03:11
@mjeffryes mjeffryes added this to the 0.108 milestone Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants