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.

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

Remove diff calculation in observe-only reconciliation #461

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mergenci
Copy link
Member

@mergenci mergenci commented Jan 16, 2025

Description of your changes

Fixes crossplane-contrib/provider-upjet-aws#1597.

While importing a DynamoDB Table, in the linked issue, Terraform executes CustomizeDiff functions when Upjet calculates the diff in Observe(). One of the CustomizeDiff functions is a validation function that checks whether the configuration that is subject to diff is valid. Because the configuration is practically empty — it only contains the resource name in the import path — validation fails and returns an error, which in turn causes our Observe() to fail.

I've checked how the behavior of managed reconciler changes for observe-only reconciliation. When Upjet always returns ResourceUpToDate = true, this block executes. Otherwise, when Upjet calculates the diff and returns ResourceUpToDate accordingly, this block executes in case ResourceUpToDate = false. The difference between those two blocks are (1) debug messages and (2) metric records.

The difference doesn't matter for the import path, in which ResourceUpToDate doesn't make much sense. We can consider ResourceUpToDate = true, by definition, which is the case in this PR. The difference might matter in observe-only reconciliation that is not an import operation.

TODO:

  • Consider updating Terraform Plugin Framework external client as well.

I have:

  • Read and followed Upjet's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Manually, using the resource manifest given in the linked issue above.

@mergenci mergenci force-pushed the remove-diff-in-observe-only branch from 61c3d4d to 54e83a3 Compare January 16, 2025 20:24
@mergenci mergenci force-pushed the remove-diff-in-observe-only branch from 54e83a3 to 10d0d54 Compare January 16, 2025 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Observing DynamoDB Table rangeKey throws Unused attributes error
1 participant