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

Optimize stack configuration step #650

Closed
EronWright opened this issue Sep 6, 2024 · 1 comment
Closed

Optimize stack configuration step #650

EronWright opened this issue Sep 6, 2024 · 1 comment
Assignees
Labels
kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed
Milestone

Comments

@EronWright
Copy link
Contributor

EronWright commented Sep 6, 2024

The Workspace controller is expected to configure the stack with the supplied configuration values. The current implementation calls the SetAllConfig RPC method for each config item, and this is proving to be slow. Would be nice to set in bulk, but be considerate about the ordering of the config elements.

Also, consider enhancing the Workspace config block to natively understand configmap/secret refs. Today, the Stack controller handles references via general extensibility, i.e. via the pod template. A first-class option would be better.

@pulumi-bot pulumi-bot added the needs-triage Needs attention from the triage team label Sep 6, 2024
@EronWright EronWright changed the title Optimize stack configuration Optimize stack configuration step Sep 6, 2024
@cleverguy25
Copy link

Added to epic #586

@blampe blampe added kind/engineering Work that is not visible to an external user and removed needs-triage Needs attention from the triage team labels Sep 9, 2024
@mjeffryes mjeffryes assigned EronWright and blampe and unassigned EronWright Sep 9, 2024
@mjeffryes mjeffryes added this to the 0.110 milestone Sep 12, 2024
@blampe blampe modified the milestones: 0.110, 0.111 Oct 1, 2024
blampe added a commit that referenced this issue Oct 17, 2024
We currently issue one `SetAllConfig` RPC for each user-specified
config. This is slow but it has important correctness guarantees:

1. The order we apply config matters -- if the user specifies `foo: foo`
followed by `foo: bar`, the net result must always be `foo: bar`.
2. The Pulumi CLI (and therefore Automation API) only allows specifying
`--path` on an all-or-nothing basis. This is bad for us because we
potentially have a blend of path and non-path keys.

Ideally we would be able to supply all of our configs to the automation
API in a single call, and in the case where _all_ of our config keys are
path-like (or all are not path-like) we actually can do that because we
no longer have limitation (2).

This PR makes that possible in the general case by transforming our
config keys in a way that allows us to treat them as if they are all
path-like.

In particular:
* The agent's `SetAllConfig` handler is modified to take a list of
configs instead of a map in order to preserve config order. The
top-level `path` param is also removed and handled on a per-key basis.
* While resolving configs, we escape any non-path keys so subsequent
path parsing treats them as verbatim. For example `foo.bar` gets escaped
as `["foo.bar"]`.
* We can then supply all of our keys at once to Automation API with
`Path: true`.
* If there are no configs to set then the operator doesn't invoke
`SetAllConfig`.

Fixes #650
@EronWright EronWright added the resolution/fixed This issue was fixed label Oct 17, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Pulumi Kubernetes Operator v2 Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants