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

Make Kafka Cluster compatible with XSkipDetailedDiffForChanges #264

Open
t0yv0 opened this issue Nov 8, 2023 · 0 comments
Open

Make Kafka Cluster compatible with XSkipDetailedDiffForChanges #264

t0yv0 opened this issue Nov 8, 2023 · 0 comments
Labels
bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. kind/bug Some behavior is incorrect or out of spec

Comments

@t0yv0
Copy link
Member

t0yv0 commented Nov 8, 2023

What happened?

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

The Kafka Cluster resource needs tweaking to operate correctly with XSkipDetailedDiffForChanges.

Context: bridge is introducing a change to the diffing algorithm that is currently flagged under
XSkipDetailedDiffForChanges but is on the fast track to become turned on unconditionally as it
solves important issues for GCP and AWS provider. This change was introduced in:
pulumi/pulumi-terraform-bridge#1502

When pre-testing this change through Azure tests, TestBasicClusterPy started failing. It is expected
that after a successful pulumi up, the subsequent pulumi preview generates a no-changes plan.
With XSkipDetailedDiffForChanges it started to generate an Update plan instead, based on these
attributes:

     CHANGING FROM DIFF_NONE to DIFF_SOME
     ~ "basic.#" => {0 1 false false <nil> false false 0}

Investigating this further, the root cause is still unclear, but note that the "basic" parameter is
very strange/peculiar, it insists on having an empty object with no schema:

Which then the users have to supply in the code:
https://github.com/pulumi/pulumi-confluentcloud/blob/main/examples/basic-kafka-acls/py/__main__.py#L16

Something is not computing the diff for this attribute correctly. Possible ways forward here can be
a bridge fix for the diff bug, or working around and avoiding the empty-attribute basic structure.

Affected area/feature

Example

N/A

Output of pulumi about

N/A

Additional context

N/A

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@t0yv0 t0yv0 added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Nov 8, 2023
@mikhailshilkov mikhailshilkov added bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. and removed needs-triage Needs attention from the triage team labels Nov 8, 2023
VenelinMartinov added a commit to pulumi/pulumi-terraform-bridge that referenced this issue May 6, 2024
This reverts commit c57799f, reversing
changes made to e71c1a6.

reverts #1893

We have some downstream failures:

https://github.com/pulumi/pulumi-azure/actions/runs/8945276805/job/24576271881?pr=2001

https://github.com/pulumi/pulumi-github/actions/runs/8945161556/job/24576285007?pr=652

Some opened issues which look like prerequisites:
pulumi/pulumi-azure#1421 (related to the
failure above)
pulumi/pulumi-confluentcloud#264 (we don't
have credentials here, so likely why it did not produce a downstream
failure)

Anton also noted [some
issues](#1927)
with SkipDetailedDiff in AWS, so this change seems much less risky than
initially though.
VenelinMartinov added a commit to pulumi/pulumi-terraform-bridge that referenced this issue May 8, 2024
Fixes #1934

This is already in GCP and reverting it causes a bunch of test failures.

We need to push it to other providers.

Will address:
- [x]
https://github.com/pulumi/pulumi-azure/actions/runs/8945276805/job/24576271881?pr=2001
- [x]
https://github.com/pulumi/pulumi-github/actions/runs/8945161556/job/24576285007?pr=652
- [x] pulumi/pulumi-azure#1421
- [X] pulumi/pulumi-confluentcloud#264 - We do
not have credentials on this one. We can't test here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

2 participants