Applied namespace to Cleanup and getArgs

[BlenderDude]

Description

Fixes #3762

This PR changes how the namespace applied in the release in the skaffold.yaml file is sent to Helm 3. It currently does not look for helm releases in the defined namespace, this PR applies the --namespace parameter to the helm calls, but only for Helm 3.

User facing changes

With Helm 3 supporting fully namespace'd releases, it would be expected that setting the namespace in the Skaffold release would make it deploy there. Currently this is only the case for the deployment step. For cleanup and getting the current lists of deployments, it does not apply the namespace.

Before

Skaffold would successfully push the first version of the Helm release successfully. When tearing down, it would fail because the delete command did not have the namespace sent to it with the --namespace argument. When running Skaffold again, the un-deleted release would still exist, but it would not pick up on it because the function that gets the current releases did not apply the namespace properly either.

After

The normal workflow for skaffold is properly executed. Skaffold accesses the current helm releases in the namespace defined in the skaffold.yaml file. If it does not exist, it deploys to the namespace, or upgrades if it does. When tearing down, it properly deletes the release from the namespace.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes unit tests
• Mentions any output changes.
• Adds documentation as needed: user docs, YAML reference, CLI reference.
• Adds integration tests if needed.

Reviewer Notes

• The code flow looks good.
• Unit test added.
• User facing changes look good.

Release Notes

This was a bug noticed before support for helm 3 was published, so I'm not sure anything needs to be added here.

[BlenderDude]

@nkubala Here is the PR for the Helm 3 namespace issue. Some pointers on where to figure out the unit/integration testing would be appreciated!

[codecov]

Codecov Report

Merging #3787 into master will decrease coverage by 0.25%.
The diff coverage is 85.71%.

Impacted Files	Coverage Δ
pkg/skaffold/deploy/helm.go	77.77% <85.71%> (+0.3%)	⬆️
pkg/skaffold/initializer/build/init.go	51.72% <0%> (-29.23%)	⬇️
pkg/skaffold/initializer/deploy/init.go	68.96% <0%> (-21.04%)	⬇️
pkg/skaffold/initializer/deploy/kubectl.go	85.18% <0%> (-14.82%)	⬇️
pkg/skaffold/initializer/init.go	38.63% <0%> (-11.37%)	⬇️
pkg/skaffold/initializer/build/resolve.go	86.44% <0%> (-8.01%)	⬇️
...affold/kubernetes/portforward/kubectl_forwarder.go	65.85% <0%> (-2.44%)	⬇️
pkg/skaffold/initializer/build/util.go	100% <0%> (ø)	⬆️
cmd/skaffold/app/cmd/init.go	100% <0%> (ø)	⬆️
cmd/skaffold/app/cmd/flags.go	100% <0%> (ø)	⬆️
... and 3 more

[nkubala]

@BlenderDude thanks for the PR! pkg/skaffold/deploy/helm_test.go is where the helm unit tests live - you can add new test cases there. you'll probably want to update TestHelmDeploy or TestHelmCleanup to make sure that the commands are being issued with the provided namespace. you can add a field to the test struct, namespace, and then change the test call to pass the namespace to makeRunContext if you want:

func makeRunContext(deploy latest.HelmDeploy, force bool, namespace string) *runcontext.RunContext {
	pipeline := latest.Pipeline{}
	pipeline.Deploy.DeployType.HelmDeploy = &deploy
	ns := testNamespace
	if namespace != "" {
	  ns = namespace
	}

	return &runcontext.RunContext{
		Cfg:         pipeline,
		KubeContext: testKubeContext,
		Opts: config.SkaffoldOptions{
			Namespace:  ns,
			KubeConfig: testKubeConfig,
			Force:      force,
		},
	}
}

don't worry about integration tests for now. we're only testing helm 2 at the moment, I'm gonna need to make a bigger change to add support for helm 3 as well.

[BlenderDude]

@nkubala Sounds good, I'll get those implemented.

[BlenderDude]

@nkubala I have gotten the tests merged in. I added a new deploy config which includes a namespace to test against. These tests should have everything covered!

[nkubala]

@BlenderDude nice job, and thanks for the fix!