Pulumi-kubernetes: Support helm.Chart with helm hooks

[gobengo]

Pulumi program: https://gist.github.com/gobengo/590a88e3b951fa3f66c8d8e963bf34a0

I am trying to use the latest wordpress chart (version 5.9.0) to test out pulumi-kubernetes' helm.Chart. The pulumi kubernetes docs for helm use a much earlier one.

const wordpressChart = new k8s.helm.v2.Chart(`${namespaceName}-wordpress`, {
    namespace: namespaceName,
    repo: "stable",
    version: "5.9.0",
    chart: "wordpress",
    transformations: [
        addNamespaceTransformation(namespaceName),
        removeHelmHooksTransformation()
    ]
});

However, when I pulumi up, it ends up failing because this helm chart includes a helm.sh/hook: test-success annotation. The mariadb chart it composes has a test hook as well, so there are two that fail.

Pulumi seems to be unaware of hook semantics (understandable). The end result is that the hook pods get created at the same time (sometimes) before the app Pods they depend on (i.e. mariadb), and so they fail with exit code 1, which causes the pulumi deployment to fail as well.

I was able to work around this by providing a transformation created with

function removeHelmHooksTransformation() {
    return (o: any) => {
        if ( ! o) return
        if (o.metadata && o.metadata.annotations && o.metadata.annotations['helm.sh/hook']) {
            console.log('skipping helm hook', o.metadata)
            // transforms in nodejs expects you to mutate input object, not return a new one :/
            // https://github.com/pulumi/pulumi-kubernetes/blob/4b01b5114df3045cecefd2ff3e2f2ed64430e3dd/sdk/nodejs/yaml/yaml.ts#L2214
            for (const key in o) {
                delete o[key]
            }
            Object.assign(o, {
                kind: "List",
                apiVersion: "v1",
                metadata: {},
                items: [],
            })
            return
        }
        return o
    }
}

Ideally, pulumi-kubernetes would be aware of helm hook semantics and would, e.g., wait for other Pods to be deployed and ready before creating Pods with the helm.sh/hook: test-success annotation. In lieu of full semantic support, it would be nice to maybe just not deploy pods with that annotation at all (and warn with a link to this issue or a more appropriate one).

[gobengo]

Worth noting that https://github.com/pulumi/pulumi-kubernetes/issues/511 would be at least some way of supporting these charts without modification, but ofc with the overhead of running and configuring Tiller securely.

[hausdorff]

This might involve a helm provider proper. Should we do this now, or should we be waiting for Helm 3?

[lukehoban]

@hausdorff @lblackstone What's the proposal for addressing this?

[lblackstone]

I'm not very familiar with Helm hooks, but it looks like a pretty big list of scenarios that it supports: https://github.com/helm/helm/blob/master/docs/charts_hooks.md#the-available-hooks

AFAICT, Pulumi can natively handle all of the lifecycle hooks (pre/post/crd install hooks), so we probably just need to specially handle the test-success and test-failure annotations.

For now, I'm inclined to skip deploying Pods with the helm.sh/hook: test-success or helm.sh/hook: test-failure annotation -- these Pods are intended to be a short-lived test, and aren't part of the application.

[hausdorff]

I'm not sure that we'll ever support lifecycle hooks, but certainly not in Q3—I'll remove the label. In the mean time, we can commit to skipping the test hooks, and documenting that our Helm support is limited to the semantics of helm template. I'll file a separate issue for that, which we will close in Q3.

[Place1]

The workaround posted in this issue no longer works for me.

I get an error saying there's no metadata.name on the List.

After adding a metadata.name to the List I get a apiVersion does not have both a group and a version: "v1"

[lblackstone]

@Place1 I have a PR open to fix the regression you reported in #1156. The workaround will be valid again once that merges.

[shinebayar-g]

Looks like ingress-nginx uses helm hooks to deploy admission-webhook things. For this limitation we cannot deploy ingress-nginx.

[draconisNoctis]

@shinebayar-g
Although the documentation of ingress-nginx mentioned, that only Helm v3 is supported a workaround is to use the k8s.helm.v2.Chart.
Helm 2 doesn't support the hooks, so the annotations are ignored.
Pulumi Helm 3 doesn't support them neither and removing any entry with the annotation before processed by the transformations. So it isn't even possible to remove the annotation in a transformation.

[lblackstone]

@shinebayar-g @draconisNoctis We fixed a bug in the v3 SDK in #1335 that was likely the cause of the issues you were seeing. Can you give it a try with the v2.8.0 k8s provider and let me know if you're still having problems? Thanks!

[shinebayar-g]

@lblackstone I deployed ingress-nginx chart as k8s.helm.v3.Chart resource today. It worked like a charm. All ingress-nginx resources with helm.sh/hook annotations deployed successfully. Thank you.

---

I did another test today. Upgraded nginx chart version from 3.17.0 to 3.23.0 to demonstrate upgrade scenario. Also looks like everything went smooth.

[bob-bins]

The transformation function in the original comment no longer works. Here's an updated one (in Typescript) in case someone else needs it.

import * as k8s from "@pulumi/kubernetes"
import * as pulumi from "@pulumi/pulumi"

const removeHelmHooksTransformation = (
  o: pulumi.ResourceTransformationArgs
): pulumi.ResourceTransformationResult => {
  if (o.props?.metadata?.annotations?.["helm.sh/hook"]) {
    const {
      "helm.sh/hook": junk,
      "helm.sh/hook-delete-policy": junk2,
      ...validAnnotations
    } = o.props.metadata.annotations
    return {
      props: {
        ...o.props,
        metadata: {
          ...o.props.metadata,
          annotations: validAnnotations,
        },
      },
      opts: o.opts,
    }
  }
  return o
}

// example use-case
const ingressNginx = new k8s.yaml.ConfigFile(
  "ingress-nginx",
  {
    file:
      "https://raw.githubusercontent.com/kubernetes/ingress-nginx/controller-v0.44.0/deploy/static/provider/aws/deploy.yaml",
  },
  {
    provider: eksCluster.provider,
    transformations: [removeHelmHooksTransformation],
  }
)

[maxromanovsky]

Is it possible to suppress Diagnostics messages in pulumi up cause by Helm hooks such as this one:

  kubernetes:rbac.authorization.k8s.io/v1:RoleBinding (ingress-nginx-internal/ingress-nginx-internal-admission):
    warning: This resource contains Helm hooks that are not currently supported by Pulumi. The resource will be created, but any hooks will not be executed. Hooks support is tracked at https://github.com/pulumi/pulumi-kubernetes/issues/555

[bob-bins]

@maxromanovsky If you're looking to suppress only those messages, I don't think that's possible. But you can use the transformation function I posted above to remove the helm hook annotations so it won't be an issue.

[maxromanovsky]

@bob-bins thank you for the tip! I thought there's more idiomatic approach :)

Anyway, here's the transformation very similar to @bob-bins, but in Python:

def remove_helm_hooks(obj):
    if 'metadata' in obj and 'annotations' in obj['metadata']:
        for key in list(obj['metadata']['annotations'].keys()):
            if key in ['helm.sh/hook', 'helm.sh/hook-delete-policy']:
                del obj['metadata']['annotations'][key]

[gerrywastaken]

I'm not sure that we'll ever support lifecycle hooks

@hausdorff In this case isn't it better in the docs and errors to advise against using Chart when hooks are involved and instead pushing people towards more appropriate methods of adding a package of resources. Just removing blindly ignoring/removing hooks probably isn't a great idea, but at the moment it's the only solution presented.

p.s. an alternative transform to remove those annotations (typescript noob so use with caution):
```typescript
// A transformation to remove Helm hooks. Pulumi does not support running them and will complain if they exist.
// See: https://github.com/pulumi/pulumi-kubernetes/issues/555#issuecomment-810383577
removeHelmHooksTransformation(o: pulumi.ResourceTransformationArgs): pulumi.ResourceTransformationResult {
if (o.props?.metadata?.annotations?.["helm.sh/hook"]) {
delete o.props.metadata.annotations['helm.sh/hook']
delete o.props.metadata.annotations['helm.sh/hook-delete-policy']
}

return o
}
```

[shinebayar-g]

@shinebayar-g @draconisNoctis We fixed a bug in the v3 SDK in #1335 that was likely the cause of the issues you were seeing. Can you give it a try with the v2.8.0 k8s provider and let me know if you're still having problems? Thanks!

Hi, @lblackstone can this issue considered as a fixed now? If yes it would be awesome to remove these warnings introduced in https://github.com/pulumi/pulumi-kubernetes/issues/1454

[lblackstone]

We're planning to take another look at this soon, and will add toggles for the warnings if we aren't able to support all of the hooks.