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

Fix set flag for write-values #1503

Closed
wants to merge 1 commit into from
Closed

Fix set flag for write-values #1503

wants to merge 1 commit into from

Conversation

vixus0
Copy link

@vixus0 vixus0 commented Sep 29, 2020

The --set flag for write-values is currently a no-op. I'm guessing this is because we can't simply pass the flag through to helm like we do for the other commands.

I've worked around this by using Helm's own strvals package to parse the values provided by --set and then merging the resulting YAML with the generated values.

My attempt at a test is failing due to a nil pointer dereference. I'm not very good at debugging Go yet, so any help would be appreciated!

@mumoshu
Copy link
Collaborator

mumoshu commented Oct 20, 2020

@vixus0 Thanks for the PR! I'm eager to merge this once the following error gets resolved:

--- FAIL: TestWriteValues_WithSetFlag (0.01s)
panic: unexpected error: exec: "helm": executable file not found in $PATH [recovered]
	panic: unexpected error: exec: "helm": executable file not found in $PATH

goroutine 3140 [running]:
testing.tRunner.func1.1(0x1c54b00, 0xc00077cba0)
	/usr/local/go/src/testing/testing.go:940 +0x421
testing.tRunner.func1(0xc0008c1200)
	/usr/local/go/src/testing/testing.go:943 +0x600
panic(0x1c54b00, 0xc00077cba0)
	/usr/local/go/src/runtime/panic.go:975 +0x3e3
github.com/roboll/helmfile/pkg/helmexec.Output(0xc00023b1e0, 0xc000adac60, 0x1, 0x1, 0xc00041a800, 0x32, 0x40, 0x203000, 0x0)
	/home/circleci/workspace/helmfile/pkg/helmexec/runner.go:83 +0x1007
github.com/roboll/helmfile/pkg/helmexec.ShellRunner.Execute(0x0, 0x0, 0xc0009980f0, 0x1f02643, 0x4, 0xc00089b020, 0x3, 0x3, 0x0, 0xc00089b020, ...)
	/home/circleci/workspace/helmfile/pkg/helmexec/runner.go:39 +0x1af
github.com/roboll/helmfile/pkg/helmexec.getHelmVersion(0x1f02643, 0x4, 0x2c2ed00, 0xc000b80120, 0xc000648e00, 0xc000adaf98, 0x7f91d3a1dc28, 0x0, 0xc000b80120)
	/home/circleci/workspace/helmfile/pkg/helmexec/exec.go:93 +0x15b
github.com/roboll/helmfile/pkg/helmexec.New(0x1f02643, 0x4, 0xc0009980f0, 0x1f06212, 0x7, 0x2c2ed00, 0xc000b80120, 0x0)
	/home/circleci/workspace/helmfile/pkg/helmexec/exec.go:104 +0x78
github.com/roboll/helmfile/pkg/app.(*App).getHelm(0xc0008c1680, 0xc0007edb80, 0x0, 0x0)
	/home/circleci/workspace/helmfile/pkg/app/app.go:637 +0x47b
github.com/roboll/helmfile/pkg/app.(*App).visitStatesWithSelectorsAndRemoteSupport.func1(0xc0007edb80, 0xc000000001, 0xa3, 0x0, 0xc0007ed900)
	/home/circleci/workspace/helmfile/pkg/app/app.go:896 +0x8f
github.com/roboll/helmfile/pkg/app.(*App).visitStates.func1(0x1f0d7e9, 0xd, 0xc0005829d0, 0x8, 0x0, 0x0)
	/home/circleci/workspace/helmfile/pkg/app/app.go:736 +0xb6c
github.com/roboll/helmfile/pkg/app.(*App).visitStateFiles.func1(0x37ddfa8, 0xc000000001)
	/home/circleci/workspace/helmfile/pkg/app/app.go:570 +0x19f
github.com/roboll/helmfile/pkg/app.(*App).within(0xc0008c1680, 0x1f01415, 0x1, 0xc0005b17e0, 0xc0005b17a0, 0x2)
	/home/circleci/workspace/helmfile/pkg/app/app.go:512 +0x94c
github.com/roboll/helmfile/pkg/app.(*App).visitStateFiles(0xc0008c1680, 0x1f0d7e9, 0xd, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/home/circleci/workspace/helmfile/pkg/app/app.go:564 +0x604
github.com/roboll/helmfile/pkg/app.(*App).visitStates(0xc0008c1680, 0x1f0d7e9, 0xd, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/home/circleci/workspace/helmfile/pkg/app/app.go:648 +0x15a
github.com/roboll/helmfile/pkg/app.(*App).visitStatesWithSelectorsAndRemoteSupport(0xc0008c1680, 0x1f0d7e9, 0xd, 0xc00035acb0, 0xc00035ad20, 0x1, 0x1, 0x1, 0xc0005829c0)
	/home/circleci/workspace/helmfile/pkg/app/app.go:903 +0x8e9
github.com/roboll/helmfile/pkg/app.(*App).ForEachState(0xc0008c1680, 0xc00035ad30, 0xc00035ad20, 0x1, 0x1, 0x438cc0, 0xc00035ad50)
	/home/circleci/workspace/helmfile/pkg/app/app.go:784 +0x110
github.com/roboll/helmfile/pkg/app.(*App).WriteValues(0xc0008c1680, 0x2c551a0, 0xc00089bb60, 0xc00089bb60, 0xc0009980f0)
	/home/circleci/workspace/helmfile/pkg/app/app.go:240 +0xe9
github.com/roboll/helmfile/pkg/app.TestWriteValues_WithSetFlag(0xc0008c1200)
	/home/circleci/workspace/helmfile/pkg/app/app_test.go:4265 +0x536
testing.tRunner(0xc0008c1200, 0x1f6a7c0)
	/usr/local/go/src/testing/testing.go:991 +0x1ec
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:1042 +0x661
FAIL	github.com/roboll/helmfile/pkg/app	367.655s

@vixus0
Copy link
Author

vixus0 commented Oct 21, 2020

Ok, after taking a deeper look at this today, I think it's not possible to test this using the TestFs like the other tests because of the use of temporary files. For the time being I've switched things over to using bytes.Buffer instead of temporary files. Hopefully it hasn't broken things anywhere else 🙈

Currently, the `--set` flag doesn't do anything when passed to the
`write-values` subcommand. This uses the `strvals` package from Helm to
parse the flag values as they would be by Helm and merges the resulting
YAML into the generated values.

The use of temporary files makes it difficult to test the write-values
subcommand with a unit test, so I've added an integration test instead.
@vixus0
Copy link
Author

vixus0 commented Oct 22, 2020

I've reverted the buffer change and added an integration test instead - there are too many moving parts for this to be tested with a unit test, I think.

@vixus0 vixus0 closed this by deleting the head repository Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants