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

Permadiff in pulumiLabels and effectiveLabels with empty labels for gcp:container:Cluster #2395

Closed
zbuchheit opened this issue Sep 19, 2024 · 11 comments · Fixed by #2441
Closed
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@zbuchheit
Copy link

Describe what happened

Similar behavior to #2372

A user reported seeing a diff in effectiveLabels/pulumiLabels under 8.2.0

gcp:container:Cluster gke-cluster  update [diff: ~effectiveLabels,pulumiLabels]

Sample program

TODO

Log output

No response

Affected Resource(s)

No response

Output of pulumi about

gcp:container:Cluster

Additional context

The previous issue for the other resources like bucket and kms key are fine now, seems only gcp:container:Cluster remains problematic

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

@zbuchheit zbuchheit added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Sep 19, 2024
@iwahbe
Copy link
Member

iwahbe commented Sep 20, 2024

Hmmm. @zbuchheit please let us know as soon as you have a repro.

@iwahbe iwahbe removed the needs-triage Needs attention from the triage team label Sep 20, 2024
@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Sep 20, 2024

The resource has an overloaded labels field: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster

labels is used as a Kubernetes property and resource_labels are the GCP labels.

I suspect that #2386 does not handle that case.

@iwahbe
Copy link
Member

iwahbe commented Sep 20, 2024

It does not. I'm guessing we will need to special case this resource.

@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Sep 20, 2024

Couldn't repro so far:

import * as pulumi from "@pulumi/pulumi";
import * as gcp from "@pulumi/gcp";

const _default = new gcp.serviceaccount.Account("default", {
  accountId: "service-account-id-2",
  displayName: "Service Account",
});

const primary = new gcp.container.Cluster("primary", {
  name: "marcellus-wallace",
  location: "us-central1-a",
  initialNodeCount: 3,
  deletionProtection: false,
  nodeConfig: {
    serviceAccount: _default.email,
    oauthScopes: ["https://www.googleapis.com/auth/cloud-platform"],
    resourceLabels: {
      foo: "bar",
      empty: "",
    },
  },
});

produces no diff, unlike with #2372

@VenelinMartinov
Copy link
Contributor

Ok, the repro is with labels actually:

import * as pulumi from "@pulumi/pulumi";
import * as gcp from "@pulumi/gcp";

const _default = new gcp.serviceaccount.Account("default", {
  accountId: "service-account-id-2",
  displayName: "Service Account",
});

const primary = new gcp.container.Cluster("primary", {
  name: "marcellus-wallace",
  location: "us-central1-a",
  initialNodeCount: 3,
  deletionProtection: false,
  nodeConfig: {
    serviceAccount: _default.email,
    oauthScopes: ["https://www.googleapis.com/auth/cloud-platform"],
    labels: {
      foo: "bar",
      empty: "",
    },
  },
});

@iwahbe iwahbe self-assigned this Sep 20, 2024
@iwahbe
Copy link
Member

iwahbe commented Sep 20, 2024

@VenelinMartinov I am unable to reproduce the problem on v8.2.0 with the above program.1

Footnotes

  1. I changed id-2 to id-3, but I don't think that will be material.

@iwahbe iwahbe added the needs-repro Needs repro steps before it can be triaged or fixed label Sep 20, 2024
@VenelinMartinov
Copy link
Contributor

Yeah, it looks like i was using 7.38.0 - pulumi new still gives v7.

@zbuchheit
Copy link
Author

Happening via Go MLC will attempt a repro shortly

@mikhailshilkov mikhailshilkov added the blocked The issue cannot be resolved without 3rd party action. label Sep 23, 2024
@zbuchheit
Copy link
Author

the following repros for me on 8.2.0, it appears the field causing it is resourceLabels

Code Repro
import * as gcp from "@pulumi/gcp";

const serviceAccount = new gcp.serviceaccount.Account(
  "zbuchheit-service-account",
  {
    accountId: "zbuchheit-service-account",
    displayName: "zbuchheit-service-account",
  }
);

const cluster = new gcp.container.Cluster("zbuchheit-cluster", {
  name: "zbuchheit-cluster",
  initialNodeCount: 1,
  location: "us-west2",
  nodeConfig: {
    machineType: "e2-medium",
    oauthScopes: ["https://www.googleapis.com/auth/cloud-platform"],
    serviceAccount: serviceAccount.email,
  },
  resourceLabels: {
    environment: "dev",
    test: "",
  },
  deletionProtection: false,
});

@zbuchheit zbuchheit added the needs-triage Needs attention from the triage team label Sep 24, 2024
@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Sep 25, 2024

I've confirmed @zbuchheit 's repro also works for me with 8.2.0 - will investigate.

My previous attempts at reproing this were with resourceLabels on the nodeConfig property - that seems to be the reason we couldn't repro before.

@VenelinMartinov VenelinMartinov removed blocked The issue cannot be resolved without 3rd party action. needs-repro Needs repro steps before it can be triaged or fixed needs-triage Needs attention from the triage team labels Sep 25, 2024
VenelinMartinov added a commit that referenced this issue Sep 25, 2024
This fixes the empty label handling in the GCP Cluster resource.

In the fix for #2372
(#2386 and
pulumi/pulumi-terraform-bridge#2417) we did not
know that the labels property in GCP is sometimes overloaded, ex GCP
Custer.

For the Cluster resource, the GCP labels are under `resource_labels`,
not `labels`

This PR adds the logic to the empty labels fix and adds a regression
test.

fixes #2395
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Sep 25, 2024
@pulumi-bot
Copy link
Contributor

This issue has been addressed in PR #2441 and shipped in release v8.3.1.

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.

5 participants