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.

Sign up for GitHub

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

Jump to bottom

Use output properties for await logic #2790

Merged
merged 19 commits into from
Feb 2, 2024
Merged

Use output properties for await logic #2790

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
cdccea2
Use output properties for await logic
EronWright Jan 29, 2024
06a868c
changelog
EronWright Jan 29, 2024
ecd723a
consistent client versioning
EronWright Feb 1, 2024
cc4e672
deletion should use outputs
EronWright Feb 1, 2024
8dbffef
simplify
EronWright Feb 1, 2024
784f48f
use outputs for awaiter lookup
EronWright Feb 1, 2024
1da5f6a
Merge remote-tracking branch 'origin/master' into eronwright/issue-25…
EronWright Feb 1, 2024
f24566a
remove inputs from awaiters
EronWright Feb 1, 2024
132b8e5
linting
EronWright Feb 1, 2024
c2a01fb
assertions for safety
EronWright Feb 1, 2024
37be2d6
more assertions
EronWright Feb 1, 2024
489416d
Merge remote-tracking branch 'origin/master' into eronwright/issue-25…
EronWright Feb 1, 2024
db44f8c
Merge remote-tracking branch 'origin/master' into eronwright/issue-25…
EronWright Feb 1, 2024
188400a
update tests
EronWright Feb 1, 2024
e1aaa81
Uae Input for skipAwait
EronWright Feb 2, 2024
460f195
use Inputs for timeout
EronWright Feb 2, 2024
d3783c5
tweak
EronWright Feb 2, 2024
688ef1b
Merge remote-tracking branch 'origin/master' into eronwright/issue-25…
EronWright Feb 2, 2024
556e701
linting
EronWright Feb 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## Unreleased

- Fix DiffConfig issue when when provider's kubeconfig is set to file path (https://github.com/pulumi/pulumi-kubernetes/pull/2771)
- Use output properties for await logic (https://github.com/pulumi/pulumi-kubernetes/pull/2790)

## 4.7.1 (January 17, 2024)

Expand Down
55 changes: 29 additions & 26 deletions provider/pkg/await/await.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,19 @@ type ReadConfig struct {

type UpdateConfig struct {
ProviderConfig
Previous *unstructured.Unstructured
Inputs *unstructured.Unstructured
Timeout float64
Preview bool
OldInputs *unstructured.Unstructured
Copy link
Contributor Author

@EronWright EronWright Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

OldOutputs *unstructured.Unstructured
Inputs *unstructured.Unstructured
Timeout float64
Preview bool
// 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
Copy link
Contributor Author

@EronWright EronWright Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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).

Name string
Timeout float64
}
Expand Down Expand Up @@ -172,7 +174,7 @@ func Creation(c CreateConfig) (*unstructured.Unstructured, error) {
// server API, at which point they should hopefully succeed.
var err error
if client == nil {
client, err = c.ClientSet.ResourceClient(c.Inputs.GroupVersionKind(), c.Inputs.GetNamespace())
client, err = c.ClientSet.ResourceClientForObject(c.Inputs)
if err != nil {
if skip, version := skipRetry(c.Inputs.GroupVersionKind(), c.ClusterVersion, err); skip {
return &kinds.RemovedAPIError{
Expand Down Expand Up @@ -248,10 +250,10 @@ func Creation(c CreateConfig) (*unstructured.Unstructured, error) {
// Wait until create resolves as success or error. Note that the conditional is set up to log
// only if we don't have an entry for the resource type; in the event that we do, but the await
// logic is blank, simply do nothing instead of logging.
id := fmt.Sprintf("%s/%s", c.Inputs.GetAPIVersion(), c.Inputs.GetKind())
id := fmt.Sprintf("%s/%s", outputs.GetAPIVersion(), outputs.GetKind())
if awaiter, exists := awaiters[id]; exists {
if metadata.SkipAwaitLogic(c.Inputs) {
logger.V(1).Infof("Skipping await logic for %v", c.Inputs.GetName())
logger.V(1).Infof("Skipping await logic for %v", outputs.GetName())
} else {
if awaiter.awaitCreation != nil {
conf := createAwaitConfig{
Expand Down Expand Up @@ -280,7 +282,7 @@ func Creation(c CreateConfig) (*unstructured.Unstructured, error) {
// If the client fails to get the live object for some reason, DO NOT return the error. This
// will leak the fact that the object was successfully created. Instead, fall back to the
// last-seen live object.
live, err := client.Get(c.Context, c.Inputs.GetName(), metav1.GetOptions{})
live, err := client.Get(c.Context, outputs.GetName(), metav1.GetOptions{})
if err != nil {
return outputs, nil
}
Expand All @@ -289,7 +291,7 @@ func Creation(c CreateConfig) (*unstructured.Unstructured, error) {

// Read checks a resource, returning the object if it was created and initialized successfully.
func Read(c ReadConfig) (*unstructured.Unstructured, error) {
client, err := c.ClientSet.ResourceClient(c.Inputs.GroupVersionKind(), c.Inputs.GetNamespace())
client, err := c.ClientSet.ResourceClientForObject(c.Inputs)
if err != nil {
return nil, err
}
Expand All @@ -306,7 +308,7 @@ func Read(c ReadConfig) (*unstructured.Unstructured, error) {
id := fmt.Sprintf("%s/%s", outputs.GetAPIVersion(), outputs.GetKind())
if awaiter, exists := awaiters[id]; exists {
if metadata.SkipAwaitLogic(c.Inputs) {
logger.V(1).Infof("Skipping await logic for %v", c.Inputs.GetName())
logger.V(1).Infof("Skipping await logic for %v", c.Name)
} else {
if awaiter.awaitRead != nil {
conf := createAwaitConfig{
Expand Down Expand Up @@ -360,7 +362,7 @@ func Update(c UpdateConfig) (*unstructured.Unstructured, error) {

// Get the "live" version of the last submitted object. This is necessary because the server may
// have populated some fields automatically, updated status fields, and so on.
liveOldObj, err := client.Get(c.Context, c.Previous.GetName(), metav1.GetOptions{})
liveOldObj, err := client.Get(c.Context, c.OldOutputs.GetName(), metav1.GetOptions{})
if err != nil {
return nil, err
}
Expand All @@ -377,10 +379,10 @@ func Update(c UpdateConfig) (*unstructured.Unstructured, error) {
// Wait until patch resolves as success or error. Note that the conditional is set up to log only
// if we don't have an entry for the resource type; in the event that we do, but the await logic
// is blank, simply do nothing instead of logging.
id := fmt.Sprintf("%s/%s", c.Inputs.GetAPIVersion(), c.Inputs.GetKind())
id := fmt.Sprintf("%s/%s", currentOutputs.GetAPIVersion(), currentOutputs.GetKind())
if awaiter, exists := awaiters[id]; exists {
if metadata.SkipAwaitLogic(c.Inputs) {
logger.V(1).Infof("Skipping await logic for %v", c.Inputs.GetName())
logger.V(1).Infof("Skipping await logic for %v", currentOutputs.GetName())
} else {
if awaiter.awaitUpdate != nil {
conf := updateAwaitConfig{
Expand All @@ -396,7 +398,7 @@ func Update(c UpdateConfig) (*unstructured.Unstructured, error) {
timeout: c.Timeout,
clusterVersion: c.ClusterVersion,
},
lastInputs: c.Previous,
lastInputs: c.OldInputs,
lastOutputs: liveOldObj,
}
waitErr := awaiter.awaitUpdate(conf)
Expand All @@ -411,12 +413,12 @@ func Update(c UpdateConfig) (*unstructured.Unstructured, error) {

gvk := c.Inputs.GroupVersionKind()
logger.V(3).Infof("Resource %s/%s/%s '%s.%s' patched and updated", gvk.Group, gvk.Version,
gvk.Kind, c.Inputs.GetNamespace(), c.Inputs.GetName())
gvk.Kind, c.Inputs.GetNamespace(), currentOutputs.GetName())

// If the client fails to get the live object for some reason, DO NOT return the error. This
// will leak the fact that the object was successfully created. Instead, fall back to the
// last-seen live object.
live, err := client.Get(c.Context, c.Inputs.GetName(), metav1.GetOptions{})
live, err := client.Get(c.Context, currentOutputs.GetName(), metav1.GetOptions{})
if err != nil {
return currentOutputs, nil
}
Expand Down Expand Up @@ -450,7 +452,7 @@ func csaUpdate(c *UpdateConfig, liveOldObj *unstructured.Unstructured, client dy
// optimistically rather than failing the update.
_ = handleCSAIgnoreFields(c, liveOldObj)
// Create merge patch (prefer strategic merge patch, fall back to JSON merge patch).
patch, patchType, _, err := openapi.PatchForResourceUpdate(c.Resources, c.Previous, c.Inputs, liveOldObj)
patch, patchType, _, err := openapi.PatchForResourceUpdate(c.Resources, c.OldInputs, c.Inputs, liveOldObj)
if err != nil {
return nil, err
}
Expand All @@ -462,7 +464,7 @@ func csaUpdate(c *UpdateConfig, liveOldObj *unstructured.Unstructured, client dy
options.DryRun = []string{metav1.DryRunAll}
}

return client.Patch(c.Context, c.Inputs.GetName(), patchType, patch, options)
return client.Patch(c.Context, liveOldObj.GetName(), patchType, patch, options)
}

// ssaUpdate handles the logic for updating a resource using server-side apply.
Expand Down Expand Up @@ -490,7 +492,7 @@ func ssaUpdate(c *UpdateConfig, liveOldObj *unstructured.Unstructured, client dy
options.DryRun = []string{metav1.DryRunAll}
}

currentOutputs, err := client.Patch(c.Context, c.Inputs.GetName(), types.ApplyPatchType, objYAML, options)
currentOutputs, err := client.Patch(c.Context, liveOldObj.GetName(), types.ApplyPatchType, objYAML, options)
if err != nil {
if errors.IsConflict(err) {
err = fmt.Errorf("Server-Side Apply field conflict detected. See %s for troubleshooting help\n: %w",
Expand Down Expand Up @@ -543,7 +545,7 @@ func handleSSAIgnoreFields(c *UpdateConfig, liveOldObj *unstructured.Unstructure
for _, f := range managedFields {
s, err := fluxssa.FieldsToSet(*f.FieldsV1)
if err != nil {
return fmt.Errorf("unable to parse managed fields from resource %q into fieldpath.Set: %w", c.Inputs.GetName(), err)
return fmt.Errorf("unable to parse managed fields from resource %q into fieldpath.Set: %w", liveOldObj.GetName(), err)
}

switch f.Manager {
Expand Down Expand Up @@ -706,18 +708,18 @@ 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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

if err != nil {
return nilIfGVKDeleted(err)
}

patchResource := kinds.IsPatchURN(c.URN)
if c.ServerSideApply && patchResource {
err = ssa.Relinquish(c.Context, client, c.Inputs, c.FieldManager)
err = ssa.Relinquish(c.Context, client, c.Outputs, c.FieldManager)
return err
}

timeout := metadata.TimeoutDuration(c.Timeout, c.Inputs, 300)
timeout := metadata.TimeoutDuration(c.Timeout, c.Outputs, 300)
timeoutSeconds := int64(timeout.Seconds())
listOpts := metav1.ListOptions{
FieldSelector: fields.OneTermEqualSelector("metadata.name", c.Name).String(),
Expand All @@ -739,10 +741,10 @@ func Deletion(c DeleteConfig) error {
// if we don't have an entry for the resource type; in the event that we do, but the await logic
// is blank, simply do nothing instead of logging.
var waitErr error
id := fmt.Sprintf("%s/%s", c.Inputs.GetAPIVersion(), c.Inputs.GetKind())
id := fmt.Sprintf("%s/%s", c.Outputs.GetAPIVersion(), c.Outputs.GetKind())
if awaiter, exists := awaiters[id]; exists && awaiter.awaitDeletion != nil {
if metadata.SkipAwaitLogic(c.Inputs) {
logger.V(1).Infof("Skipping await logic for %v", c.Inputs.GetName())
if metadata.SkipAwaitLogic(c.Outputs) {
logger.V(1).Infof("Skipping await logic for %v", c.Name)
} else {
waitErr = awaiter.awaitDeletion(deleteAwaitConfig{
createAwaitConfig: createAwaitConfig{
Expand All @@ -752,6 +754,7 @@ func Deletion(c DeleteConfig) error {
initialAPIVersion: c.InitialAPIVersion,
clientSet: c.ClientSet,
currentInputs: c.Inputs,
currentOutputs: c.Outputs,
logger: c.DedupLogger,
timeout: c.Timeout,
clusterVersion: c.ClusterVersion,
Expand Down
Loading
Loading