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

Add --helm-debug Flag to Kustomize for Enhanced Helm Debugging #5751

Merged
merged 7 commits into from
Sep 12, 2024

Conversation

isarns
Copy link
Contributor

@isarns isarns commented Aug 19, 2024

Summary of the Enhancement

This PR adds a new flag, --helm-debug, to the Kustomize CLI. When this flag is provided, the --debug flag is passed to the Helm executable. This enables more detailed logging and error information from Helm, which is helpful when diagnosing issues with Helm charts during the Kustomize build process.

Why This is Useful

In cases where Helm chart templates fail to render correctly, the default error messages can be opaque and difficult to diagnose. By enabling Helm’s --debug flag via --helm-debug, users can see detailed logs from Helm, including the specific files and lines where errors occur, as well as the internal processes Helm is performing. This can save significant time in troubleshooting and resolving issues.

Example Usage

Without --helm-debug:

kustomize build . --load-restrictor=LoadRestrictionsNone --enable-helm

Output:

Error: Error: YAML parse error on s3/templates/bucket.yaml: error converting YAML to JSON: yaml: line 9: found character that cannot start any token

Use --debug flag to render out invalid YAML
: unable to run: 'helm template s3...'

With --helm-debug:

kustomize build . --load-restrictor=LoadRestrictionsNone --enable-helm --helm-debug

Output:

Error: install.go:218: [debug] Original chart version: ""
install.go:235: [debug] CHART PATH: /Users/isarn/git/IAC/crossplane/devops/live/devops/prod/ca-central-1/s3/iaas-scripts/charts/s3-0.1.1/s3

Error: YAML parse error on s3/templates/bucket.yaml: error converting YAML to JSON: yaml: line 9: found character that cannot start any token
helm.go:84: [debug] error converting YAML to JSON: yaml: line 9: found character that cannot start any token
YAML parse error on s3/templates/bucket.yaml
helm.sh/helm/v3/pkg/releaseutil.(*manifestFile).sort
        helm.sh/helm/v3/pkg/releaseutil/manifest_sorter.go:146
helm.sh/helm/v3/pkg/releaseutil.SortManifests
        helm.sh/helm/v3/pkg/releaseutil/manifest_sorter.go:106
helm.sh/helm/v3/pkg/action.(*Configuration).renderResources
        helm.sh/helm/v3/pkg/action/action.go:168
helm.sh/helm/v3/pkg/action.(*Install).RunWithContext
        helm.sh/helm/v3/pkg/action/install.go:304
main.runInstall
        helm.sh/helm/v3/cmd/helm/install.go:310
main.newTemplateCmd.func2
        helm.sh/helm/v3/cmd/helm/template.go:95
github.com/spf13/cobra.(*Command).execute
        github.com/spf13/[email protected]/command.go:983
github.com/spf13/cobra.(*Command).ExecuteC
        github.com/spf13/[email protected]/command.go:1115
github.com/spf13/cobra.(*Command).Execute
        github.com/spf13/[email protected]/command.go:1039
main.main
        helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
        runtime/proc.go:271
runtime.goexit
        runtime/asm_amd64.s:1695
---
# Source: s3/templates/bucket.yaml
apiVersion: s3.aws.upbound.io/v1beta1
kind: Bucket
metadata:
  labels:
    project: 
    env: 
    region: us-west-2
    crossplane-name: my-bucket
  name: %!!(MISSING)s(<nil>)-%!!(MISSING)s(<nil>)-usw2-my-bucket
spec:
  forProvider:
    tags:
      Name: %!!(MISSING)s(<nil>)-%!!(MISSING)s(<nil>)-usw2-my-bucket
  providerConfigRef:
    name:%!!(MISSING)s(<nil>)-%!!(MISSING)s(<nil>)
: unable to run: 'helm template s3 ...'
...

How to Test

  1. Add the --helm-debug flag to your Kustomize command as shown above.
  2. Run the command against a Helm chart with known issues.
  3. Verify that the detailed debug output is displayed, helping to identify the root cause of the error.

Additional Notes

  • This enhancement is backward compatible and does not interfere with the existing Helm integration in Kustomize.
  • The --helm-debug flag is optional and can be omitted if detailed Helm debugging is not required.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 19, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @isarns. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 19, 2024
Copy link
Member

@stormqueen1990 stormqueen1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello there, @isarns! 👋🏻

Thanks for your PR! I left a few comments, mostly around testing. Could you please take a look at those?

@@ -174,7 +177,7 @@ func (p *HelmChartInflationGeneratorPlugin) runHelmCommand(
fmt.Errorf(
"unable to run: '%s %s' with env=%s (is '%s' installed?): %w",
helm, strings.Join(args, " "), env, helm, err),
stderr.String(),
stderr.String()+stdout.String(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about the concatenation of the stdout content here. What's the scenario this aims to address?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

helm --debug write both to stderr (stack trace) and to stdout for the incomplete YAML template.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Perhaps there would be value to adding a prefix so the user can identify which output is stdout and which output is stderr?
I am a bit concerned that this might become a wall of text difficult to parse through.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an error format.

The output looks like that now:

Error:  Helm stack trace:
install.go:218: [debug] Original chart version: ""
install.go:235: [debug] CHART PATH: /Users/isarn/temp/helm-debug

Error: YAML parse error on helm-debug/templates/test.yaml: error converting YAML to JSON: yaml: line 4: block sequence entries are not allowed in this context
helm.go:84: [debug] error converting YAML to JSON: yaml: line 4: block sequence entries are not allowed in this context
YAML parse error on helm-debug/templates/test.yaml
helm.sh/helm/v3/pkg/releaseutil.(*manifestFile).sort
        helm.sh/helm/v3/pkg/releaseutil/manifest_sorter.go:146
helm.sh/helm/v3/pkg/releaseutil.SortManifests
        helm.sh/helm/v3/pkg/releaseutil/manifest_sorter.go:106
helm.sh/helm/v3/pkg/action.(*Configuration).renderResources
        helm.sh/helm/v3/pkg/action/action.go:168
helm.sh/helm/v3/pkg/action.(*Install).RunWithContext
        helm.sh/helm/v3/pkg/action/install.go:304
main.runInstall
        helm.sh/helm/v3/cmd/helm/install.go:310
main.newTemplateCmd.func2
        helm.sh/helm/v3/cmd/helm/template.go:95
github.com/spf13/cobra.(*Command).execute
        github.com/spf13/[email protected]/command.go:983
github.com/spf13/cobra.(*Command).ExecuteC
        github.com/spf13/[email protected]/command.go:1115
github.com/spf13/cobra.(*Command).Execute
        github.com/spf13/[email protected]/command.go:1039
main.main
        helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
        runtime/proc.go:271
runtime.goexit
        runtime/asm_amd64.s:1695

Helm template:
---
# Source: helm-debug/templates/test.yaml
---
apiVersion: test.kustomize.io/v1
kind: ValuesMergeTest
metadata:
  name: -

: unable to run: 'helm template helm-debug-test /Users/isarn/temp/helm-debug -f /var/folders/6d/ck0zlsjx3vsd4x9r42y6knvr0000gn/T/kustomize-helm-194917191/helm-debug-kustomize-values.yaml --debug' with env=[HELM_CONFIG_HOME=/var/folders/6d/ck0zlsjx3vsd4x9r42y6knvr0000gn/T/kustomize-helm-194917191/helm HELM_CACHE_HOME=/var/folders/6d/ck0zlsjx3vsd4x9r42y6knvr0000gn/T/kustomize-helm-194917191/helm/.cache HELM_DATA_HOME=/var/folders/6d/ck0zlsjx3vsd4x9r42y6knvr0000gn/T/kustomize-helm-194917191/helm/.data] (is 'helm' installed?): exit status 1

api/types/helmchartargs.go Outdated Show resolved Hide resolved
go.work.sum Outdated Show resolved Hide resolved
@k8s-ci-robot
Copy link
Contributor

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 29, 2024
@stormqueen1990
Copy link
Member

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Sep 3, 2024
@isarns
Copy link
Contributor Author

isarns commented Sep 4, 2024

Thanks @stormqueen1990!

  • Added the test. I'm not quite sure if this is the best way to test it as I commented on your comment.
  • Formatted the debug output Helm stack trace: for stderr (the stack trace) and Helm template: for stdout (the incomplete template)

Please let me know if there is anything else that needs to be done.

Copy link
Member

@stormqueen1990 stormqueen1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @isarns!

/lgtm
/assign @koba1t
for maintainer review

go.work.sum Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 9, 2024
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 9, 2024
Copy link
Member

@stormqueen1990 stormqueen1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 9, 2024
@koba1t
Copy link
Member

koba1t commented Sep 10, 2024

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Sep 10, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 10, 2024
@koba1t
Copy link
Member

koba1t commented Sep 12, 2024

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: isarns, koba1t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 12, 2024
@k8s-ci-robot k8s-ci-robot merged commit 4034e36 into kubernetes-sigs:master Sep 12, 2024
10 checks passed
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Oct 17, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [kubernetes-sigs/kustomize](https://github.com/kubernetes-sigs/kustomize) | minor | `v5.4.3` -> `v5.5.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>kubernetes-sigs/kustomize (kubernetes-sigs/kustomize)</summary>

### [`v5.5.0`](https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize/v5.5.0)

[Compare Source](kubernetes-sigs/kustomize@kustomize/v5.4.3...kustomize/v5.5.0)

### Breaking change

A starlark support for krm functions was removed to cleanup dependencies. kubernetes-sigs/kustomize#5768
This feature was deprecated 3 years ago and removed because there was no desire to continue using it.
kubernetes-sigs/kustomize#5768 (comment)

#### Feature

[#&#8203;5751](kubernetes-sigs/kustomize#5751): Add `--helm-debug` Flag to Kustomize for Enhanced Helm Debugging

#### Fix Bugs

[#&#8203;5458](kubernetes-sigs/kustomize#5458): Sort built-in Namespace kind before CRDs with the same name
[#&#8203;5745](kubernetes-sigs/kustomize#5745): Add Annotation to Control Inline List Conversion in Kustomize Resources"

#### Dependencies

[#&#8203;5763](kubernetes-sigs/kustomize#5763): Update go 1.22.7
[#&#8203;5781](kubernetes-sigs/kustomize#5781): Update kyaml to v0.18.1
[#&#8203;5782](kubernetes-sigs/kustomize#5782): Update cmd/config to v0.15.0
[#&#8203;5783](kubernetes-sigs/kustomize#5783): Update api to v0.18.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants