Support for metadata.generateName (CSA) #2808

EronWright · 2024-02-01T20:30:11Z

Proposed changes

This PR implements support for .metadata.generateName in CSA mode, based on #2790.

Adjust the auto-naming logic to consider .metadata.generateName to be a variant of auto-naming. Pulumi will not assign an auto-name to a new resource that has generateName, and upon delete will use the replace-then-delete technique.
Use outputs rather than inputs to fetch the live object or to obtain the live object's name.
Fix an autonaming bug where the old auto-name would erroneously be adopted, overwriting a new computed name.

Tests

A new integration test is provided to test the use of metadata.generateName. It tests creation, update, replacement, and promotion from .generateName to .name. (ref)
New test cases for the await logic.
The existing autonaming test is enhanced to test how autonaming takes precedence over generateName, in the update case. This is to ensure backwards compatibility, e.g. in the edge case that an existing object has a generateName field (which Pulumi ignored until now).

Related issues (optional)

Closes #2539

github-actions · 2024-02-01T20:33:36Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

codecov · 2024-02-01T20:36:19Z

Codecov Report

Attention: 198 lines in your changes are missing coverage. Please review.

Comparison is base (ef6b07b) 24.72% compared to head (f4e4d13) 24.64%.
Report is 1 commits behind head on master.

Files	Patch %	Lines
provider/pkg/provider/provider.go	0.00%	60 Missing ⚠️
provider/pkg/await/awaiters.go	0.00%	48 Missing ⚠️
provider/pkg/await/await.go	18.60%	35 Missing ⚠️
provider/pkg/openapi/openapi.go	0.00%	11 Missing ⚠️
provider/pkg/await/deployment.go	44.44%	10 Missing ⚠️
provider/pkg/await/ingress.go	10.00%	9 Missing ⚠️
provider/pkg/await/job.go	0.00%	7 Missing ⚠️
provider/pkg/await/statefulset.go	12.50%	7 Missing ⚠️
provider/pkg/await/pod.go	0.00%	6 Missing ⚠️
provider/pkg/metadata/naming.go	84.21%	2 Missing and 1 partial ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2808      +/-   ##
==========================================
- Coverage   24.72%   24.64%   -0.08%     
==========================================
  Files          48       48              
  Lines        9651     9710      +59     
==========================================
+ Hits         2386     2393       +7     
- Misses       7109     7160      +51     
- Partials      156      157       +1

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

rquitales

LGTM

rquitales · 2024-02-03T01:04:35Z

provider/pkg/provider/provider.go

+	// This makes it possible to update the program to set `.metadata.name` to the name that was
+	// made by `.metadata.generateName` without triggering replacement.
+	if newInputs.GetName() != "" {
+		oldLivePruned.SetName(oldLive.GetName())


Admittedly, this took me a couple of passes to understand what's happening. Maybe it's worthwhile to expand the comment a bit? Something like, generateName will create and store a name, but the oldLivePruned object drops the name field to match the input, which we now need for diffing.

Though this could just be an artifact of code review only showing small chunks of code without context.

rquitales · 2024-02-03T01:17:20Z

tests/sdk/nodejs/autonaming/step4/index.ts


 //
-// User has now specified `.metadata.name`, so Pulumi should replace the resource, and NOT allocate
-// a name to it.
+// User has now specified `.metadata.generateName`, which Pulumi ignores because autonaming has already occurred,


In this scenario, could we emit a warning to the user that we won't honor generateName? I believe we just silently continue right?

rquitales · 2024-02-03T01:18:54Z

tests/sdk/nodejs/generatename/step2/index.ts

+const namespace = new k8s.core.v1.Namespace("test-namespace");
+
+//
+// The `.metadata.generateName` field has changed, but Pulumi does NOT automatically replace in that situation.


Maybe a warning here too?

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@pulumi/kubernetes](https://pulumi.com) ([source](https://togithub.com/pulumi/pulumi-kubernetes)) | dependencies | minor | [`4.7.1` -> `4.8.0`](https://renovatebot.com/diffs/npm/@pulumi%2fkubernetes/4.7.1/4.8.0) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>pulumi/pulumi-kubernetes (@pulumi/kubernetes)</summary> ### [`v4.8.0`](https://togithub.com/pulumi/pulumi-kubernetes/blob/HEAD/CHANGELOG.md#480-February-22-2024) [Compare Source](https://togithub.com/pulumi/pulumi-kubernetes/compare/v4.7.1...v4.8.0) - Fix DiffConfig issue when when provider's kubeconfig is set to file path ([pulumi/pulumi-kubernetes#2771) - Fix for replacement having incorrect status messages ([pulumi/pulumi-kubernetes#2810) - Use output properties for await logic ([pulumi/pulumi-kubernetes#2790) - Support for metadata.generateName (CSA) ([pulumi/pulumi-kubernetes#2808) - Fix unmarshalling of Helm values yaml file ([pulumi/pulumi-kubernetes#2815) - Handle unknowns in Helm Release resource ([pulumi/pulumi-kubernetes#2822) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate).  Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>

vavsab · 2024-03-07T17:09:46Z

@EronWright did you test it for ConfigMap?
Is had generateName working for years in ConfigMap before this change. Today I get an error

the `.metadata.generateName` field is not supported in Server-Side Apply mode

Maybe I need to change anything in my provider?

EronWright · 2024-04-10T20:47:05Z

@vavsab I would guess that your provider is configured to use server-side apply mode, which you can disable:
https://www.pulumi.com/registry/packages/kubernetes/how-to-guides/managing-resources-with-server-side-apply/

gunzy83 · 2024-05-27T03:00:17Z

@vavsab I would guess that your provider is configured to use server-side apply mode, which you can disable: https://www.pulumi.com/registry/packages/kubernetes/how-to-guides/managing-resources-with-server-side-apply/

Is there a reason this is not supported with SSA? Will it be supported in the future?

EronWright mentioned this pull request Feb 1, 2024

Support for metadata.generateName (CSA) #2594

Closed

EronWright changed the base branch from master to eronwright/issue-2539-a February 1, 2024 22:00

EronWright force-pushed the eronwright/issue-2539 branch from bcbc7b0 to 5fa3713 Compare February 1, 2024 22:00

EronWright marked this pull request as ready for review February 1, 2024 23:43

EronWright requested review from rquitales and lblackstone February 1, 2024 23:43

Base automatically changed from eronwright/issue-2539-a to master February 2, 2024 20:43

EronWright added 6 commits February 2, 2024 12:46

Implement GenerateName

db4834a

changelog

8dd11a3

Update await tests

54e73b7

Fix naming test and consolidate some logic

2b6390b

fake generateName improvement

f4e64f1

comment

1f93e88

EronWright force-pushed the eronwright/issue-2539 branch from deb295e to 1f93e88 Compare February 2, 2024 20:52

EronWright added 3 commits February 2, 2024 13:07

assertion message tweak

33f8f1a

tweak error messages to use resource URN

8581366

clearify the deleteBeforeReplace logic

f4e4d13

EronWright enabled auto-merge (squash) February 2, 2024 22:48

mjeffryes assigned EronWright Feb 3, 2024

rquitales approved these changes Feb 3, 2024

View reviewed changes

EronWright merged commit cc3c25d into master Feb 3, 2024
20 checks passed

EronWright deleted the eronwright/issue-2539 branch February 3, 2024 01:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for metadata.generateName (CSA) #2808

Support for metadata.generateName (CSA) #2808

EronWright commented Feb 1, 2024 •

edited

github-actions bot commented Feb 1, 2024

codecov bot commented Feb 1, 2024 •

edited

rquitales left a comment

rquitales Feb 3, 2024

rquitales Feb 3, 2024

rquitales Feb 3, 2024

vavsab commented Mar 7, 2024

EronWright commented Apr 10, 2024

gunzy83 commented May 27, 2024

Support for metadata.generateName (CSA) #2808

Support for metadata.generateName (CSA) #2808

Conversation

EronWright commented Feb 1, 2024 • edited

Proposed changes

Tests

Related issues (optional)

github-actions bot commented Feb 1, 2024

Does the PR have any schema changes?

codecov bot commented Feb 1, 2024 • edited

Codecov Report

rquitales left a comment

Choose a reason for hiding this comment

rquitales Feb 3, 2024

Choose a reason for hiding this comment

rquitales Feb 3, 2024

Choose a reason for hiding this comment

rquitales Feb 3, 2024

Choose a reason for hiding this comment

vavsab commented Mar 7, 2024

EronWright commented Apr 10, 2024

gunzy83 commented May 27, 2024

EronWright commented Feb 1, 2024 •

edited

codecov bot commented Feb 1, 2024 •

edited