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

TestRegress170 fails under latest bridge changes #528

Closed
VenelinMartinov opened this issue Jun 11, 2024 · 6 comments
Closed

TestRegress170 fails under latest bridge changes #528

VenelinMartinov opened this issue Jun 11, 2024 · 6 comments
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Jun 11, 2024

Describe what happened

https://github.com/pulumi/pulumi-hcloud/actions/runs/9451894942/job/26034396438?pr=525

Seems to expect no diff but gets a diff.

Looks like a regression on #170

Sample program

.

Log output

  "response": {
    "stables": "*",
    "changes": "DIFF_SOME",
    "hasDetailedDiff": true,
    "diffs": ["networks"],
    "detailedDiff": {
      "networks[0].networkId": {
          "kind": "UPDATE"
    }
  }
  },

Affected Resource(s)

No response

Output of pulumi about

.

Additional context

No response

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).

@VenelinMartinov VenelinMartinov added needs-triage Needs attention from the triage team kind/bug Some behavior is incorrect or out of spec labels Jun 11, 2024
@VenelinMartinov VenelinMartinov self-assigned this Jun 11, 2024
@VenelinMartinov VenelinMartinov removed the needs-triage Needs attention from the triage team label Jun 11, 2024
@VenelinMartinov VenelinMartinov changed the title TestRegress170 fails under latest bridge TestRegress170 fails under latest bridge changes Jun 11, 2024
@VenelinMartinov VenelinMartinov removed their assignment Jun 11, 2024
@VenelinMartinov VenelinMartinov added the needs-triage Needs attention from the triage team label Jun 11, 2024
@VenelinMartinov
Copy link
Contributor Author

Reproes with bridge master

@iwahbe iwahbe self-assigned this Jun 11, 2024
@iwahbe iwahbe added p1 A bug severe enough to be the next item assigned to an engineer and removed needs-triage Needs attention from the triage team labels Jun 11, 2024
@iwahbe
Copy link
Member

iwahbe commented Jun 11, 2024

P1 because it's blocking the next bridge release.

@iwahbe
Copy link
Member

iwahbe commented Jun 11, 2024

I have confirmed that the regression is genuine (and not overfitting from the test).

@iwahbe
Copy link
Member

iwahbe commented Jun 12, 2024

A binary search has shown that "ba48631b Enable DiffStrategy PlanState by default (#1976)" is the last good commit on the bridge, "b012fa8e Fix proposed new unknown blocks (#2060)" introduced the regression.

@iwahbe iwahbe removed the p1 A bug severe enough to be the next item assigned to an engineer label Jun 12, 2024
@iwahbe
Copy link
Member

iwahbe commented Jun 12, 2024

This looks like a bug in upstream: hetznercloud/terraform-provider-hcloud#650. I have confirmed that Pulumi can use the same workaround as TF suggests. I'll bake the work-around into a transform so we don't expose the change to our users. I believe that we are safe to release the bridge.

iwahbe added a commit that referenced this issue Jun 12, 2024
Fixes #528

I have removed the diff replay test since it doesn't capture the issue anymore. I have
replaced this with an integration test since we need to ensure that "Check" behaves such
that diff doesn't present an issue.
iwahbe added a commit that referenced this issue Jun 12, 2024
Fixes #528

I have removed the diff replay test since it doesn't capture the issue anymore. I have
replaced this with an integration test since we need to ensure that "Check" behaves such
that diff doesn't present an issue.
@pulumi-bot pulumi-bot reopened this Jul 1, 2024
@pulumi-bot
Copy link
Contributor

Cannot close issue:

  • does not have required labels: resolution/

Please fix these problems and try again.

@VenelinMartinov VenelinMartinov added the resolution/fixed This issue was fixed label Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants