-
Notifications
You must be signed in to change notification settings - Fork 115
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
Add replaceUnready annotation for Jobs #1575
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
// Copyright 2016-2018, Pulumi Corporation. | ||
// Copyright 2016-2021, Pulumi Corporation. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
|
@@ -35,6 +35,7 @@ import ( | |
structpb "github.com/golang/protobuf/ptypes/struct" | ||
pkgerrors "github.com/pkg/errors" | ||
"github.com/pulumi/pulumi-kubernetes/provider/v3/pkg/await" | ||
"github.com/pulumi/pulumi-kubernetes/provider/v3/pkg/await/states" | ||
"github.com/pulumi/pulumi-kubernetes/provider/v3/pkg/clients" | ||
"github.com/pulumi/pulumi-kubernetes/provider/v3/pkg/cluster" | ||
"github.com/pulumi/pulumi-kubernetes/provider/v3/pkg/gen" | ||
|
@@ -1368,6 +1369,23 @@ func (k *kubeProvider) Diff( | |
} | ||
} | ||
|
||
if metadata.ReplaceUnready(newInputs) { | ||
switch newInputs.GetKind() { | ||
case "Job": | ||
jobChecker := states.NewJobChecker() | ||
job, err := clients.FromUnstructured(newInputs) | ||
if err == nil { | ||
jobChecker.Update(job) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it okay to do this wait operation from Diff? Do we normally do that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not actually running the full await logic; this is a point in time check based on the current status of the Job. I'm not sure if there's precedent for that, but it shouldn't have a noticeable effect on performance, and Diff is the only place where we can indicate a replacement is required. |
||
if !jobChecker.Ready() { | ||
hasChanges = pulumirpc.DiffResponse_DIFF_SOME | ||
replaces = append(replaces, `.metadata.annotations["pulumi.com/replaceUnready"]`) | ||
} | ||
} | ||
default: | ||
_ = k.host.Log(ctx, diag.Warning, urn, "replaceUnready annotation is not supported for this resource") | ||
} | ||
lblackstone marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// Delete before replacement if we are forced to replace the old object, and the new version of | ||
// that object MUST have the same name. | ||
deleteBeforeReplace := | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional that the error is swallowed here? Shouldn't it be returned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this error would only occur from a malformed Job object and is unrelated to the diff, I thought it made sense to ignore. I can add a comment to that effect.