Always set a field manager name #2271
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
Recent versions of Kubernetes have started to create a managedFields entry for every resource, even those that use Client-Side Apply (CSA). The name of the field manager is inferred from the name of the executable performing the update. This can lead to cross-platform conflicts, because the executable name can be different (e.g., myprogram vs. myprogram.exe). This leads to multiple field managers owning the same fields. To avoid auto-naming conflicts like this, always set a field manager name. In the CSA mode, this name will always be "pulumi-kubernetes". In Server-Side Apply (SSA) mode, the name will be "pulumi-kubernetes-<suffix>", with a randomly-generated suffix.
lblackstone
force-pushed
the
lblackstone/csa-field-manager
branch
from
92858e5
to
69a6f94
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
CHANGELOG.md
Outdated
| @@ -2,6 +2,7 @@ | |||
|
|
|||
| - Add `PULUMI_K8S_ENABLE_PATCH_FORCE` env var support (https://github.com/pulumi/pulumi-kubernetes/pulls/2260) | |||
| - Add link to resolution guide for SSA conflicts (https://github.com/pulumi/pulumi-kubernetes/pulls/2265) | |||
| - Always set a field manager name (https://github.com/pulumi/pulumi-kubernetes/pulls/2271) | |||
There was a problem hiding this comment.
I'd say it's not that obvious why this helps, so perhaps it would be better to phrase this in terms of what it fixes for people, in the changelog.
provider/pkg/provider/provider.go
Outdated
| @@ -2903,7 +2903,12 @@ func initialAPIVersion(state resource.PropertyMap, oldConfig *unstructured.Unstr | |||
| // 1. Resource annotation (this will likely change to a typed option field in the next major release) | |||
| // 2. Value from the Pulumi state | |||
| // 3. Randomly generated name | |||
| func fieldManagerName(randomSeed []byte, state resource.PropertyMap, inputs *unstructured.Unstructured) string { | |||
| func fieldManagerName( | |||
There was a problem hiding this comment.
Is it the case that for each invocation of fieldManagerName, you already know something about where the result should come from? For instance, in Create there's no existing state, so the field manager name must come from the inputs (or be a const).
If so, then ignoring that information and evaluating it again means it relies on coincidence -- it assumes that this procedure will make the same decisions as the calling code. If the field manager name must be in the state, for example, then it shouldn't fall back on cons'ing a new random name, it should return an error.
I have not internalised how this provider code all works, or even read all of it, so I may be way off beam!
There was a problem hiding this comment.
A less dramatic observation: serverSideApplyMode is always just k.serverSideApplyMode; perhaps this should be a method of the provider, so this goes through a kind of internal dispatch rather than relying on the caller to provide the (right) value.
There was a problem hiding this comment.
I'm not sure if I completely follow your question. The intent of this method is to ensure that a fieldManager name exists according to the precedence indicated in the docstring. If the value isn't specified, then one will be automatically assigned during resource creation.
Good call about changing this to be a provider method. I'll update that.
provider/pkg/provider/provider.go
Outdated
| @@ -2243,8 +2243,8 @@ func (k *kubeProvider) Update( | |||
| return nil, err | |||
| } | |||
|
|
|||
| fieldManagerOld := fieldManagerName(nil, oldState, oldInputs) | |||
| fieldManager := fieldManagerName(nil, oldState, newInputs) | |||
| fieldManagerOld := fieldManagerName(nil, oldState, oldInputs, k.serverSideApplyMode) | |||
There was a problem hiding this comment.
Does k.serverSideApply also have an old value and a new value, in the sense of it being switched on since the last run? Would that matter?
There was a problem hiding this comment.
The value can change, but the provider logic should be able to cope with it. Generally speaking, the provider will leave values alone that are already set, but would use a different code path for computing diffs.
Use a method instead of a function to ensure that the serverSideApplyMode flag is used consistently
Does the PR have any schema changes?Looking good! No breaking changes found. |
Proposed changes
Recent versions of Kubernetes have started to create a managedFields entry for every resource, even those that use Client-Side Apply (CSA). The name of the field manager is inferred from the name of the executable performing the update. This can lead to cross-platform conflicts, because the executable name can be different (e.g., myprogram vs. myprogram.exe). This leads to multiple field managers owning the same fields.
To avoid auto-naming conflicts like this, always set a field manager name. In the CSA mode, this name will always be "pulumi-kubernetes". In Server-Side Apply (SSA) mode, the name will be "pulumi-kubernetes-", with a randomly-generated suffix.
Related issues (optional)
Fix #2218