Helm deployer is looking for a Helm test pod by default resulting in confusing warning text

[j2udev]

Helm tests are awesome, but you opt-in to them by running the helm test <my-release> command when using the Helm CLI directly. When using Skaffold's Helm deployer, it appears to be looking for the existence of a test pod, but at least on the surface doesn't appear to give us the ability to run a Helm test command. This kind of causes a chicken-and-egg problem for a couple of reasons.

1. You can't run a Helm test if the release the test targets doesn't exist so we can't manually run a helm test <my-release that is deployed using the skaffold helm deployer>
2. This means we have to deploy our release first, using skaffold, but that deployment results in confusing warning messages which is even more exaggerated if you are attempting to deploy many things at once. You end up with a wall of noise similar to this:

WARN[0092] error adding label to runtime object: patching resource default/"<MY-SERVICE>-test-connection": pods "<MY-SERVICE>-test-connection" not found

It appears that Skaffold is just grabbing all templates, including those templates under tests (I don't believe the directory structure actually dictates what is a test and what isn't... I believe the helm hook annotation is what actually matters). I think there is room for a Helm test specific feature in the future here, but in the meantime can we maybe ignore the templates with the "helm.sh/hook": test or "helm.sh/hook": test-success annotation?

Expected behavior

I should be able to use the Skaffold Helm deployer without it caring about Helm tests.

Actual behavior

Skaffold spits out confusing warnings because it can't find a corresponding pod that would only ever exist after running the helm test command... which we can't run until we deploy the release anyway.

Information

• Skaffold version: v1.9.1 and/or v1.17.1
• Operating system: macOS
• Contents of skaffold.yaml: Can be recreated with any Skaffold the utilizes the Helm deployer. If it's really needed I can whip something up.

Steps to reproduce the behavior

skaffold run/deploy

in any directory with a skaffold.yaml that utilizes the Helm deployer.

[tejal29]

Thank you @j2udevelopment for opening this issue from slack conversation.

As far i see, skaffold team can do two things here.

1. Ignore helm test templates during deploy. - P1
2. Integration helm tests in testing phase - P2 // TODO create a new kind/feature bug for this issue.
   /cc @nkubala

[j2udev]

No problem! Let me know if I can provide any further input

[imrenagi]

Hi @tejal29 is this open for contribution? I faced similar issue because of test template and I think I can start taking a look into this problem.

[tejal29]

Please do @imrenagi. We are always looking for more contributions. Please comment on the bug if you need any pointers.

[imrenagi]

@tejal29 Im planning to add SkipTests bool to HelmRelease struct. However, Im not sure whether I need to update the latest config.go, or should I start new v3alpha2 version?

The Developer.md mention that it is okay to add new config to v3 since it is not released yet. However, when running the test, I observe this error.

ERRO[0000] Schema "v3" is already released. Changing it is not allowed.

What should I do?
-----------------
 + If this retroactive change is required and is harmless to users, indicate on your PR.
 + Check if a new unreleased version has been created:
     - Ensure that your branch is up-to-date with the "origin/main" branch.
     - Check for a pending PR to create a new version.
 + Create a separate PR with just the result of running the 'hack/new-version.sh' script.

Invalid changes:
----------------
diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go
index 0d982d6b6..9ae84a131 100644
--- a/pkg/skaffold/schema/latest/config.go
+++ b/pkg/skaffold/schema/latest/config.go
@@ -856,6 +856,10 @@ type HelmRelease struct {
        // Ignored for `remoteChart`.
        SkipBuildDependencies bool `yaml:"skipBuildDependencies,omitempty"`

+       // SkipTests should ignore helm test during manifests generation.
+       // Defaults to `false`
+       SkipTests bool `yaml:"skipTests,omitempty"`
+
        // UseHelmSecrets instructs skaffold to use secrets plugin on deployment.
        UseHelmSecrets bool `yaml:"useHelmSecrets,omitempty"`

FATA[0000] structural changes detected
exit status 1
FAILED hack/check-schema-changes.sh

Any pointer will be appreciated. Thanks!

[aaron-prindle]

@imrenagi you should create a new schema version - v3beta1 (I didn't invent this schema ordering as alpha -> X -> beta is not necessarily intuitive but this is historically how the skaffold apiVersion is updated; eg: v3alphaX (pre-release) -> v3 (initial major release) -> v3betaX (post major release updates) ) and add it there

EDIT: This should be v4beta1 for the next version, we have been debating what to do here and landed on v4beta1

[imrenagi]

Thanks for the pointer @aaron-prindle . I will update the PR

[aaron-prindle]

@imrenagi I am actually in the process of adding a new schema version:
#8026

as our tooling does not do this 100% automatically (some issues w/ hack/new-version.sh currently) with some recent changes I think it will be easier if I do this and I have some schema changes to make as well. Once this is merged you can just add these changes to the latest version as it is not released yet (as your PR does now).

TLDR; wait for my PR to get merged (eg: 11/3/2022) and I will ping this thread. Then you can rebase and submit a PR identical to what you have now

[imrenagi]

Hi @aaron-prindle I checked the PR for the new schema version you created and it was closed. Should I wait for another PR to be merged? Thanks!

[aaron-prindle]

Sorry @imrenagi, it took me a bit longer than I thought. This is the current PR to track:
#8034

Hoping this next run will be all passing tests against our CI/CD and I can merge the change in the next ~1-2 hours. Thanks for your patience

[imrenagi]

Thanks @aaron-prindle . I have updated my PR. waiting for review now. Thanks once again :)