Use output properties for await logic #2790

EronWright · 2024-01-29T18:17:22Z

Proposed changes

This PR changes the provider's await logic to get the object name from the output properties rather than the input properties, since the inputs may or may not contain an object name, to prepare to support generateName field (see #2539). Note that auto-naming populates the inputs, whereas generateName doesn't.

The awaiters continue to look to the inputs (not the live object) for the skipAwait and timeout annotations.

Related issues (optional)

Related: #2539

github-actions · 2024-01-29T18:21:00Z

Does the PR have any schema changes?

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

codecov · 2024-01-29T18:22:53Z

Codecov Report

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

Comparison is base (ef6b07b) 24.72% compared to head (556e701) 24.67%.

Files	Patch %	Lines
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/provider/provider.go	0.00%	4 Missing ⚠️
provider/pkg/await/service.go	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2790      +/-   ##
==========================================
- Coverage   24.72%   24.67%   -0.05%     
==========================================
  Files          48       48              
  Lines        9651     9669      +18     
==========================================
  Hits         2386     2386              
- Misses       7109     7127      +18     
  Partials      156      156

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

provider/pkg/await/await.go

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

provider/pkg/await/await.go

 	// IgnoreChanges is a list of fields to ignore when diffing the old and new objects.
 	IgnoreChanges []string
 }

 type DeleteConfig struct {
 	ProviderConfig
 	Inputs  *unstructured.Unstructured
+	Outputs *unstructured.Unstructured


Important to note here that the Inputs field contained the old outputs (current). The deletion logic should therefore use Outputs to maintain the status-quo. Inputs is used only for meta-annotations (skipAwait, timeout).

EronWright · 2024-02-01T02:09:02Z

provider/pkg/openapi/openapi.go

@@ -105,19 +105,19 @@ func PatchForResourceUpdate(
 	if knownGV := kinds.KnownGroupVersions.Has(lastSubmitted.GetAPIVersion()); !knownGV {
 		// Use a JSON merge patch for CRD Kinds.
 		patch, patchType, err = MergePatch(
-			lastSubmitted, lastSubmittedJSON, currentSubmittedJSON, liveOldJSON,
+			liveOldObj, lastSubmittedJSON, currentSubmittedJSON, liveOldJSON,


The MergePatch and StrategicMergePatch methods take a liveOld parameter to obtain the object's GVK (and object name) with which to get schema information for patch calculation. Here we see that the code was using the lastSubmitted, which is the old input (which may be an older API version?). The PR changes it to use the live object, which has the GVK of the new input. There's still the potential for a mismatch but it seems out of scope (see #2807).

EronWright · 2024-02-01T02:12:07Z

provider/pkg/provider/provider.go

+		Inputs:  oldInputs,
+		Outputs: current,


Again note how Inputs was the old outputs.

…39-a

EronWright · 2024-02-01T17:04:45Z

provider/pkg/await/await.go

-	Inputs   *unstructured.Unstructured
-	Timeout  float64
-	Preview  bool
+	OldInputs  *unstructured.Unstructured


Previous is renamed to OldInputs for clarity, given there's a new property OldOutputs.

provider/pkg/await/deployment.go

EronWright · 2024-02-01T18:17:24Z

provider/pkg/await/awaiters.go

@@ -51,7 +51,6 @@ type createAwaitConfig struct {
 	initialAPIVersion string
 	logger            *logging.DedupLogger
 	clientSet         *clients.DynamicClientSet
-	currentInputs     *unstructured.Unstructured


I see no reason for the awaiters to have or to use the inputs, since they're used strictly after any desired changes have been applied. The awaiters often compare spec and status, and the output object is a reasonable basis for that. Similarly, the deployment awaiter compares the old/new specs to anticipate whether a rollout will occur, now using the last/current outputs.

The inputs are still used to get the timeout annotation, but more eagerly in the core await logic.

…39-a

provider/pkg/await/await.go

EronWright · 2024-02-02T19:27:21Z

provider/pkg/await/await.go

@@ -718,19 +733,22 @@ func Deletion(c DeleteConfig) error {
 	}

 	// Obtain client for the resource being deleted.
-	client, err := c.ClientSet.ResourceClientForObject(c.Inputs)
+	client, err := c.ClientSet.ResourceClientForObject(c.Outputs)


As tempting as it is to use c.Inputs here, it would change the status quo and create some code smell because the client is used with the c.Outputs in many places below.

…39-a

rquitales

LGTM

### Proposed changes  This PR implements support for `.metadata.generateName` in CSA mode, based on #2790. 1. 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. 2. Use outputs rather than inputs to fetch the live object or to obtain the live object's name. 3. Fix an autonaming bug where the old auto-name would erroneously be adopted, overwriting a new computed name. ### Tests 1. 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](https://github.com/pulumi/pulumi-kubernetes/pull/2808/files#diff-7e85ce707283b1d281d971a8e5b0c49b959b0803ba3437dc9dbd422552326835R269)) 2. New test cases for the await logic. 3. 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

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>

Use output properties for await logic

cdccea2

EronWright requested a review from rquitales January 29, 2024 18:17

changelog

06a868c

EronWright marked this pull request as ready for review January 29, 2024 19:11

EronWright mentioned this pull request Jan 29, 2024

Support for metadata.generateName (CSA) #2594

Closed

mjeffryes assigned EronWright Jan 29, 2024

EronWright requested a review from lblackstone January 29, 2024 21:04

EronWright commented Jan 29, 2024

View reviewed changes

provider/pkg/await/await.go Outdated Show resolved Hide resolved

EronWright added 3 commits January 31, 2024 16:26

consistent client versioning

ecd723a

deletion should use outputs

cc4e672

simplify

8dbffef

EronWright commented Feb 1, 2024

View reviewed changes

use outputs for awaiter lookup

784f48f

EronWright commented Feb 1, 2024

View reviewed changes

EronWright added 3 commits January 31, 2024 18:13

Merge remote-tracking branch 'origin/master' into eronwright/issue-25…

1da5f6a

…39-a

remove inputs from awaiters

f24566a

linting

132b8e5

EronWright commented Feb 1, 2024

View reviewed changes

provider/pkg/await/deployment.go Show resolved Hide resolved

assertions for safety

c2a01fb

EronWright commented Feb 1, 2024

View reviewed changes

EronWright added 2 commits February 1, 2024 11:13

more assertions

37be2d6

Merge remote-tracking branch 'origin/master' into eronwright/issue-25…

489416d

…39-a

EronWright mentioned this pull request Feb 1, 2024

Support for metadata.generateName (CSA) #2808

Merged

EronWright added 2 commits February 1, 2024 13:44

Merge remote-tracking branch 'origin/master' into eronwright/issue-25…

db44f8c

…39-a

update tests

188400a

rquitales reviewed Feb 1, 2024

View reviewed changes

provider/pkg/await/await.go Outdated Show resolved Hide resolved

EronWright added 3 commits February 2, 2024 10:21

Uae Input for skipAwait

e1aaa81

use Inputs for timeout

460f195

tweak

d3783c5

EronWright commented Feb 2, 2024

View reviewed changes

EronWright added 2 commits February 2, 2024 11:38

Merge remote-tracking branch 'origin/master' into eronwright/issue-25…

688ef1b

…39-a

linting

556e701

rquitales approved these changes Feb 2, 2024

View reviewed changes

EronWright merged commit 33c84f9 into master Feb 2, 2024
20 checks passed

EronWright deleted the eronwright/issue-2539-a branch February 2, 2024 20:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use output properties for await logic #2790

Use output properties for await logic #2790

EronWright commented Jan 29, 2024 •

edited

Loading

github-actions bot commented Jan 29, 2024

codecov bot commented Jan 29, 2024 •

edited

Loading

EronWright Feb 1, 2024 •

edited

Loading

EronWright Feb 1, 2024 •

edited

Loading

EronWright Feb 1, 2024

EronWright Feb 1, 2024 •

edited

Loading

EronWright Feb 1, 2024 •

edited

Loading

EronWright Feb 2, 2024

rquitales left a comment

Use output properties for await logic #2790

Use output properties for await logic #2790

Conversation

EronWright commented Jan 29, 2024 • edited Loading

Proposed changes

Related issues (optional)

github-actions bot commented Jan 29, 2024

Does the PR have any schema changes?

codecov bot commented Jan 29, 2024 • edited Loading

Codecov Report

EronWright Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

EronWright Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

EronWright Feb 1, 2024

Choose a reason for hiding this comment

EronWright Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

EronWright Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

EronWright Feb 2, 2024

Choose a reason for hiding this comment

rquitales left a comment

Choose a reason for hiding this comment

EronWright commented Jan 29, 2024 •

edited

Loading

codecov bot commented Jan 29, 2024 •

edited

Loading

EronWright Feb 1, 2024 •

edited

Loading

EronWright Feb 1, 2024 •

edited

Loading

EronWright Feb 1, 2024 •

edited

Loading

EronWright Feb 1, 2024 •

edited

Loading