Helm Release shows spurious diff when inputs are unknown

[ffMathy]

What happened?

Getting the following for my Helm release:

~ kubernetes:helm.sh/v3:Release chart update [diff: +compat-allowNullValues,apiVersion,createNamespace,dependencyUpdate,description,devel,disableCRDHooks,disableOpenapiValidation,disableWebhooks,forceUpdate,keyring,kind,lint,postrender,recreatePods,renderSubchartNotes,replace,resetValues,resourceNames,reuseValues,skipAwait,skipCrds,verify,version,waitForJobs~values]

It always happens after doing a refresh and then an up. For those exact values.

Example

No example. Under NDA.

Output of pulumi about

CLI
Version 3.92.0
Go Version go1.21.3
Go Compiler gc

Host
OS debian
Version 12.2
Arch aarch64

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

[mikhailshilkov]

Hi @ffMathy

I'm still going to ask you to provide a repro example. Issues without a code snippet are not actionable for us. Maybe you can isolate a small program from your NDA-covered code and remove all proprietary details. Thank you for you understanding.

[dirien]

Hi @ffMathy,

will catch up on you with this and will create a example without the need of snippets of code from you!

[ffMathy]

Wow, that sounds great! Thank you.

[ffMathy]

@mjeffryes which commit was this fixed in? Or what release?

[mjeffryes]

My apologies @ffMathy, was just grooming tickets that have been awaiting-feedback for more than 2 weeks; missed that the ball is actually in our court for this one!

[dirien]

@mjeffryes we meet tomorrow with @ffMathy to have a deeper look into this!

[EronWright]

I have a possible explanation, it is that some of the inputs - maybe one of the chart values - contain unknowns. In this case, the provider behaves differently and in a way that would produce the above diff. I would advocate for a fix to the Check logic to improve upon this.

To explain further, here's the Check logic that is evidently being skipped and producing a noisy diff:

pulumi-kubernetes/provider/pkg/provider/helm_release.go

Lines 357 to 375 in 1944a52

	if !news.ContainsUnknowns() {
	logger.V(9).Infof("Decoding new release.")
	new, err := decodeRelease(news, fmt.Sprintf("%s.news", label))
	if err != nil {
	return nil, err
	}
	err = r.setComputedInputs(ctx, urn, new)
	if err != nil {
	// setComputedInputs fails when the chart cannot be rendered, and we report it as a problem with the `chart` input.
	failures = append(failures, &pulumirpc.CheckFailure{
	Property: "chart",
	Reason: fmt.Sprintf("%v; check the chart name and repository configuration.", err),
	})
	}
	logger.V(9).Infof("New: %+v", new)
	news = resource.NewPropertyMap(new)
	}

Simply put, Check normally uses decodeRelease to transform the program inputs into planned resource state (as represented by the Release struct and in variable new), which Diff then compares to the old resource state. In the special case of news.ContainsUnknowns(), the transformation doesn't take place, and Diff then (wrongly) compares raw program inputs to old resource state. The clue for me was that the diff says +compat which refers to an obsolete resource property that exists in the SDK code (and is always true) but doesn't exist in the Release struct.

[ffMathy]

That sounds very plausible! None of the values that are in the diff are specified by us.

It be great if we could also seal this off with a unit test.

[dirien]

@EronWright had the session with @ffMathy and this a code you can use, which is very very close to the setup of @ffMathy:

import * as pulumi from "@pulumi/pulumi";
import * as kubernetes from "@pulumi/kubernetes";
import * as command from "@pulumi/command";

const someNamespace = new kubernetes.core.v1.Namespace('some-namespace', {
    metadata: {
        name: 'some-namespace'
    }
});

const x = new command.local.Command('some-command', {
    update: "ls -la",
    create: "ls -la",
});

export const commandResult = x.stdout

new kubernetes.helm.v3.Release('some-chart', {
    chart: 'oci://ghcr.io/dirien/charts/minecraft-exporter',
    version: ' 0.11.1',
    name: 'some-stuff',
    timeout: 60 * 60 * 3,
    namespace: someNamespace.metadata.name,
    atomic: true,
    cleanupOnFail: true,
    description: x.stdout,
    values: {
        replicaCount: 1,
        dsd: 1,
        sds: 12,
    },
}, {
    customTimeouts: {
        create: '30m',
        update: '6h',
    },
});

This results in the pulumi preview to:

➜ pulumi preview
Previewing update (dev)

View in Browser (Ctrl+O): https://app.pulumi.com/dirien/lego-helm/dev/previews/26dabdc2-8f56-4736-b405-fdec73ce526c

     Type                              Name           Plan       Info
     pulumi:pulumi:Stack               lego-helm-dev             
 +   ├─ command:local:Command          some-command   create     
 ~   └─ kubernetes:helm.sh/v3:Release  some-chart     update     [diff: +compat-allowNullValues,apiVersion,checksum,createNamespace,dependencyUpdate,devel,disableCRDHooks,disableOpenapiValidation,disableWebhooks,forceUpdate,keyring,kind,lint,pos

Outputs:
  + commandResult: output<string>

Resources:
    + 1 to create
    ~ 1 to update
    2 changes. 2 unchanged

So using a resource output from a computed field as input in the Release object results in the situation @ffMathy reported!

[ffMathy]

Is there an ETA on this?

Right now, it causes our Pulumi to reprovision the full Helm release in production every time, leading to around 3 - 5 minutes of downtime per deploy.

[ffMathy]

CC @dirien. This is quite critical to us.

[IdoOzeri]

CC @dirien. This is quite critical to us.

I second that. I'm currently encountering the same behavior.

[ffMathy]

Maybe we need to tag @mjeffryes to get an ETA instead. Not sure if this has been forgotten. At some point it seemed to be progressing, but now it seems to have stagnated.

[EronWright]

To provide an update, this is my current task and I expect to deliver a fix next week.

[EronWright]

@ffMathy would you clarify what you think the expected behavior should be? In the repro case, the description property's value seems to be varying, and that would naturally cause a helm upgrade. Of course, there's a bug with the diff output because it implicates unrelated properties, and I intend to fix that. Imagine that was fixed; if the input is varying then you'd still see an upgrade. Thanks!

[ffMathy]

I'd just like to know which of the values are varying. Because right now it shouldn't deploy every time. At first glance, these values shouldn't change.

So fixing the diff could be enough. Then at least I'll understand the cause of it.

Could also be that logging some warnings could help. But that might create more confusion than clarity.

[EronWright]

Brief update, this is still my main work task. Some framework code was first needed to make this issue be practical to solve. The root cause is that the handling of unknown inputs is very coarse-grained and further aggravated by a bug.

[ffMathy]

Interesting. Thanks for the updates. Please keep that coming! 😍

[EronWright]

I posted a PR to solve the issue: #2822

[ffMathy]

Awesome work! How long does it usually take for it to be released after merge?

[EronWright]

We plan to cut a new release on Tuesday.

[EronWright]

The fix is now available in v4.8.0, enjoy!

[ffMathy]

Yay, thanks! Great work.

[IdoOzeri]

I'd hate to spoil the party, but for me, this issue persists even in version 4.9.1:

args.autoscaler.values = pulumi.Output.all(
                autoscaler_role.role_arn,
                args.autoscaler.values
            ).apply(
                lambda output_args: populate_values_with_placeholders(
                    values=output_args[1],
                    placeholders={
                        '<role-arn>': output_args[0]
                    }
                )
            )

First time I'm running this, I'm getting the expected plan to install the cluster autoscaler.
Once applied, it's installed successfully.

Then, I change nothing and run pulumi up again, and this is constantly what I'm getting:

     pulumi:pulumi:Stack                  base-infra-dev                    
     └─ xm_k8s_installations:HelmRelease  cluster-installations             
 ~      └─ kubernetes:helm.sh/v3:Release  autoscaler             update     [diff: ~values]

Resources:
    ~ 1 to update
    102 unchanged

Do you want to perform this update? details
  pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:dev::base-infra::pulumi:pulumi:Stack::base-infra-dev]
            > kubernetes:core/v1:Service: (read)
                [id=ingress-nginx/ingress-nginx-controller]
                [urn=urn:pulumi:dev::base-infra::xm_k8s_installations:HelmRelease$kubernetes:helm.sh/v3:Release$kubernetes:core/v1:Service::ingress-nginx-service]
                [provider=urn:pulumi:dev::base-infra::xm_aws_eks:EksCluster:eks-dev$pulumi:providers:kubernetes::eks-dev_k8s_provider::83fde744-efb1-4eb0-9163-5bcdfc0403d0]
        ~ kubernetes:helm.sh/v3:Release: (update)
            [id=kube-system/autoscaler-2e4f430f]
            [urn=urn:pulumi:dev::base-infra::xm_k8s_installations:HelmRelease$kubernetes:helm.sh/v3:Release::autoscaler]
            [provider=urn:pulumi:dev::base-infra::xm_aws_eks:EksCluster:eks-dev$pulumi:providers:kubernetes::eks-dev_k8s_provider::83fde744-efb1-4eb0-9163-5bcdfc0403d0]
          - values: {
              - autoDiscovery   : {
                  - clusterName: "eks-dev"
                }
              - awsRegion       : "eu-west-1"
              - deployment      : {
                  - annotations: {
                      - cluster-autoscaler.kubernetes.io/safe-to-evict: "false"
                    }
                }
              - extraArgs       : {
                  - balance-similar-node-groups: "true"
                  - skip-nodes-with-system-pods: "false"
                }
              - fullnameOverride: "cluster-autoscaler"
              - rbac            : {
                  - serviceAccount: {
                      - annotations: {
                          - eks.amazonaws.com/role-arn: "arn:aws:I am::<redacted>:role/cluster-node-autoscaler-6fac752"
                        }
                      - name       : "cluster-autoscaler"
                    }
                }
            }
          + values: output<string>

This is going to constantly offer a change, notice the output object reference at the end of the plan.
Thing is, when I apply the suggested changes - nothing is deployed, Pulumi realizes there's nothing to change, but it is still annoying and confusing to the end user.

Please advise,
Thanks

[mjeffryes]

Hi @IdoOzeri, this issue is a few months old now, so your comment is likely to get lost here; I suggest opening a new issue and linking to this one.