Fixes in how the provider handles CRDs and CRs #271
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hausdorff
force-pushed
the
hausdorff/istio
branch
from
b759d1e
to
c91572b
Compare
lukehoban
approved these changes
Nov 14, 2018
pkg/await/await.go
Outdated
| deleteOpts := metav1.DeleteOptions{} | ||
| if version.Compare(1, 6) < 0 { | ||
| // 1.5.x option. | ||
| boolFalse := false | ||
| // nolint | ||
| deleteOpts.OrphanDependents = &boolFalse | ||
| } else { | ||
| } else if version.Compare(1, 7) < 0 { | ||
| // 1.6.x option. (NOTE: Background delete propagation is broken in k8s v1.6, and maybe later.) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
lblackstone
requested changes
Nov 14, 2018
Currently, if a CRD is created after the CRs that depend on it, the CRs will fail. The main reason for this is that our caching discovery client does not refresh after a CRD is created, so when asked about a G/V/K, it will fail because it thinks the server does not know about it. To fix this, this commit will cause us to mark the cache as invalid any time a CRD is created. This means that the CR, when a create operation is retried, will update its view of the server before creation.
Currently, if a CRD is deleted before we have a chance to delete dependent CRs, we could fail because the the G/V/K is no longer registered with the API server. This commit corrects this by considering `NotFound` to be equivalent to a successful delete. The rationale behind this is that we must have previously successfully created the resource; hence, if the G/V/K is missing, it was successfully deleted. Importantly, if the user accidentally switches active contexts, a `pulumi up -r` will register all of these as deletes. We will be working on this scenario in the future.
Background deletion was broken for much of Kubernetes 1.6, and our code accordingly manually sets deletion to "foreground" after 1.6. This causes pain when deleting CRDs with finalizers, which will hang until the GC has a chance to delete them and all their dependencies, which is quite likely to be a time longer than our timeout period. This assumption is outdated for newer verions of Kubernetes, so this commit will default to whatever the user has set for all versions > 1.7.
hausdorff
force-pushed
the
hausdorff/istio
branch
from
c91572b
to
8bebb90
Compare
|
Ok I think we're ready for another spin on this @lblackstone. I refactored the |
lblackstone
approved these changes
Nov 14, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commit messages for a more detailed discussion.