Overhaul logic for resource diffing #2445
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
lblackstone
force-pushed
the
lblackstone/last-applied-config
branch
2 times, most recently
from
d91155e
to
7dee3c7
Compare
Does the PR have any schema changes?Found 2 breaking changes: |
lblackstone
force-pushed
the
lblackstone/last-applied-config
branch
3 times, most recently
from
3451180
to
949561d
Compare
lblackstone
force-pushed
the
lblackstone/last-applied-config
branch
6 times, most recently
from
91c69ce
to
d813693
Compare
lblackstone
changed the title
lblackstone
force-pushed
the
lblackstone/last-applied-config
branch
3 times, most recently
from
71cbadb
to
6ae907c
Compare
Previous versions of the provider used the "kubectl.kubernetes.io/last-applied-configuration" annotation to store a copy of the configuration as part of the Kubernetes resource. This annotation was used for input diff computation in client-side apply mode. Specifically, input diffs were computed between the new inputs and the last-applied-configuration value from the previous live state. Using this annotation resulted in a large number of reported issues (30+) for the provider, and it will no longer be used. 14916d3 added logic to prune a map based on a target map. This logic is used to prune live state to match the previous inputs before performing an input diff computation. This avoids the need to store the last-applied-configuration.
lblackstone
force-pushed
the
lblackstone/last-applied-config
branch
from
6ae907c
to
a1916a8
Compare
lblackstone
force-pushed
the
lblackstone/last-applied-config
branch
from
59a6e40
to
ccf4e41
Compare
rquitales
approved these changes
Jun 23, 2023
|
Fantastic to see this merged. Is there a rough timeframe for when you guys expect v4 to be released? |
We have the first release candidate out as of last Friday. We still need to write a migration guide and do a bit more testing, but it should be about feature complete at this time. I'm guessing that it will be ready for the official release by the end of July. |
This was referenced Jul 12, 2023
lblackstone
added a commit
that referenced
this pull request
Jul 14, 2023
This commit incorporates several significant changes to the resource diff logic. These changes are summarized below, followed by detailed explanations of each:
1. Drop usage of the "kubectl.kubernetes.io/last-applied-configuration" annotation.
2. Compute preview diffs using resource inputs rather than making a dry-run API call.
3. Automatically update `.metadata.managedFields` to work with resources that were managed with client-side apply, and later upgraded to use server-side apply.
4. Fix a bug with the diff calculation so that resource drift is detected accurately after a refresh.
---
1. Previous versions of the provider used the "kubectl.kubernetes.io/last-applied-configuration" annotation to store a copy of the configuration as part of the Kubernetes resource. This annotation was used for input diff computation in client-side apply mode. Specifically, input diffs were computed between the new inputs and the last-applied-configuration value from the previous live state. Using this annotation resulted in a large number of reported issues (30+) for the provider, and it will no longer be used.
14916d3 added logic to prune a map based on a target map. This logic is used to prune live state to match the previous inputs before performing an input diff computation. This avoids the need to store the last-applied-configuration.
2. Server-side apply mode in 3.x versions of the provider performed several network-bound API calls as part of the resource diff computation. This led to poor performance and diverged from the way that other Pulumi providers compute resource diffs. This behavior was brought in line with other providers, and now previews resource changes using the client-side inputs. Accepted changes are still updated using server-side apply. Note that this now requires the stack to be refreshed to detect resource drift in both client-side apply and server-side apply modes.
3. In server-side apply (SSA) mode, Kubernetes uses information in the `.metadata.managedFields` property to determine ownership of each field in a resource. Kubernetes v1.18+ sets the managedFields property in both client-side and server-side apply modes. Operations performed using client-side mode set a field manager with an operation type of "Update", while server-side operations use "Apply". Kubernetes treats these as separate field managers, which can lead to problems during server-side updates. To avoid this class of problems, the provider automatically takes ownership of fields managed by known client-side field managers during the first SSA-enabled update. This change should be transparent to users. See kubernetes/kubernetes#99003 for additional details.
4. Previous versions of the provider contained a bug where previous inputs rather than live state were used to compute a diff. This prevented the provider from detecting resource drift caused by other controllers. This bug was fixed so that drift will be corrected on update after a refresh is performed.
---------
Co-authored-by: Ramon Quitales <ramon@pulumi.com>
lblackstone
added a commit
that referenced
this pull request
Jul 14, 2023
This commit incorporates several significant changes to the resource diff logic. These changes are summarized below, followed by detailed explanations of each:
1. Drop usage of the "kubectl.kubernetes.io/last-applied-configuration" annotation.
2. Compute preview diffs using resource inputs rather than making a dry-run API call.
3. Automatically update `.metadata.managedFields` to work with resources that were managed with client-side apply, and later upgraded to use server-side apply.
4. Fix a bug with the diff calculation so that resource drift is detected accurately after a refresh.
---
1. Previous versions of the provider used the "kubectl.kubernetes.io/last-applied-configuration" annotation to store a copy of the configuration as part of the Kubernetes resource. This annotation was used for input diff computation in client-side apply mode. Specifically, input diffs were computed between the new inputs and the last-applied-configuration value from the previous live state. Using this annotation resulted in a large number of reported issues (30+) for the provider, and it will no longer be used.
14916d3 added logic to prune a map based on a target map. This logic is used to prune live state to match the previous inputs before performing an input diff computation. This avoids the need to store the last-applied-configuration.
2. Server-side apply mode in 3.x versions of the provider performed several network-bound API calls as part of the resource diff computation. This led to poor performance and diverged from the way that other Pulumi providers compute resource diffs. This behavior was brought in line with other providers, and now previews resource changes using the client-side inputs. Accepted changes are still updated using server-side apply. Note that this now requires the stack to be refreshed to detect resource drift in both client-side apply and server-side apply modes.
3. In server-side apply (SSA) mode, Kubernetes uses information in the `.metadata.managedFields` property to determine ownership of each field in a resource. Kubernetes v1.18+ sets the managedFields property in both client-side and server-side apply modes. Operations performed using client-side mode set a field manager with an operation type of "Update", while server-side operations use "Apply". Kubernetes treats these as separate field managers, which can lead to problems during server-side updates. To avoid this class of problems, the provider automatically takes ownership of fields managed by known client-side field managers during the first SSA-enabled update. This change should be transparent to users. See kubernetes/kubernetes#99003 for additional details.
4. Previous versions of the provider contained a bug where previous inputs rather than live state were used to compute a diff. This prevented the provider from detecting resource drift caused by other controllers. This bug was fixed so that drift will be corrected on update after a refresh is performed.
---------
Co-authored-by: Ramon Quitales <ramon@pulumi.com>
This was referenced Jul 14, 2023
lblackstone
added a commit
that referenced
this pull request
Jul 17, 2023
This commit incorporates several significant changes to the resource diff logic. These changes are summarized below, followed by detailed explanations of each:
1. Drop usage of the "kubectl.kubernetes.io/last-applied-configuration" annotation.
2. Compute preview diffs using resource inputs rather than making a dry-run API call.
3. Automatically update `.metadata.managedFields` to work with resources that were managed with client-side apply, and later upgraded to use server-side apply.
4. Fix a bug with the diff calculation so that resource drift is detected accurately after a refresh.
---
1. Previous versions of the provider used the "kubectl.kubernetes.io/last-applied-configuration" annotation to store a copy of the configuration as part of the Kubernetes resource. This annotation was used for input diff computation in client-side apply mode. Specifically, input diffs were computed between the new inputs and the last-applied-configuration value from the previous live state. Using this annotation resulted in a large number of reported issues (30+) for the provider, and it will no longer be used.
14916d3 added logic to prune a map based on a target map. This logic is used to prune live state to match the previous inputs before performing an input diff computation. This avoids the need to store the last-applied-configuration.
2. Server-side apply mode in 3.x versions of the provider performed several network-bound API calls as part of the resource diff computation. This led to poor performance and diverged from the way that other Pulumi providers compute resource diffs. This behavior was brought in line with other providers, and now previews resource changes using the client-side inputs. Accepted changes are still updated using server-side apply. Note that this now requires the stack to be refreshed to detect resource drift in both client-side apply and server-side apply modes.
3. In server-side apply (SSA) mode, Kubernetes uses information in the `.metadata.managedFields` property to determine ownership of each field in a resource. Kubernetes v1.18+ sets the managedFields property in both client-side and server-side apply modes. Operations performed using client-side mode set a field manager with an operation type of "Update", while server-side operations use "Apply". Kubernetes treats these as separate field managers, which can lead to problems during server-side updates. To avoid this class of problems, the provider automatically takes ownership of fields managed by known client-side field managers during the first SSA-enabled update. This change should be transparent to users. See kubernetes/kubernetes#99003 for additional details.
4. Previous versions of the provider contained a bug where previous inputs rather than live state were used to compute a diff. This prevented the provider from detecting resource drift caused by other controllers. This bug was fixed so that drift will be corrected on update after a refresh is performed.
---------
Co-authored-by: Ramon Quitales <ramon@pulumi.com>
EronWright
added a commit
that referenced
this pull request
Sep 21, 2023
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
for Pulumi's contribution guidelines.
Help us merge your changes more quickly by adding more details such
as labels, milestones, and reviewers.-->
### Proposed changes
<!--Give us a brief description of what you've done and what it solves.
-->
This PR improves the handling of the read-only metadata fields of
Kubernetes objects, such as `creationTimestamp` and `resourceVersion`.
The provider now ignores these fields when they appear as resource
inputs, to address a few quirks:
1. Unparseable values do not cause an API Server error during preview.
2. A subsequent refresh does not show a difference to the `__inputs`
property.
3. The behavior is now uniform for standard resource types and for
custom resource types.
An integration test is provided to show that such fields are ignored as
inputs, and are still available as outputs. Manual tests confirmed
identical behavior for custom resource objects.
### Related issues (optional)
Closes #2351
Related:
- #2511
- #2445
<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->
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.
Proposed changes
This commit incorporates several significant changes to the resource diff logic. These changes are summarized below, followed by detailed explanations of each:
.metadata.managedFieldsto work with resources that were managed with client-side apply, and later upgraded to use server-side apply.Previous versions of the provider used the "kubectl.kubernetes.io/last-applied-configuration" annotation to store a copy of the configuration as part of the Kubernetes resource. This annotation was used for input diff computation in client-side apply mode. Specifically, input diffs were computed between the new inputs and the last-applied-configuration value from the previous live state. Using this annotation resulted in a large number of reported issues (30+) for the provider, and it will no longer be used.
14916d3 added logic to prune a map based on a target map. This logic is used to prune live state to match the previous inputs before performing an input diff computation. This avoids the need to store the last-applied-configuration.
Server-side apply mode in 3.x versions of the provider performed several network-bound API calls as part of the resource diff computation. This led to poor performance and diverged from the way that other Pulumi providers compute resource diffs. This behavior was brought in line with other providers, and now previews resource changes using the client-side inputs. Accepted changes are still updated using server-side apply. Note that this now requires the stack to be refreshed to detect resource drift in both client-side apply and server-side apply modes.
In server-side apply (SSA) mode, Kubernetes uses information in the
.metadata.managedFieldsproperty to determine ownership of each field in a resource. Kubernetes v1.18+ sets the managedFields property in both client-side and server-side apply modes. Operations performed using client-side mode set a field manager with an operation type of "Update", while server-side operations use "Apply". Kubernetes treats these as separate field managers, which can lead to problems during server-side updates. To avoid this class of problems, the provider automatically takes ownership of fields managed by known client-side field managers during the first SSA-enabled update. This change should be transparent to users. See Server-side apply: migration from client-side apply leaves stuck fields in the object kubernetes/kubernetes#99003 for additional details.Previous versions of the provider contained a bug where previous inputs rather than live state were used to compute a diff. This prevented the provider from detecting resource drift caused by other controllers. This bug was fixed so that drift will be corrected on update after a refresh is performed.
Related issues (optional)
Fix #2446
Fix #2444
Fix #2427
Fix #2404
Fix #1576
Fix #1464
Fix #1118
Fix #787