Do not display helm warnings for multi-config projects

[gsquared94]

Related: #5455

Description

The helm deployer displays a warning when a built artifact is not used in the deployment. Like:

WARN[0052] image [frontend:427522d4bb023a3b888c2d960e4456c6d5c56eea686a69e7065bdf8e2830a0d9] is not used. 
WARN[0052] image [frontend] is used instead.            
WARN[0052] See helm sample for how to replace image names with their actual tags: https://github.com/GoogleContainerTools/skaffold/blob/master/examples/helm-deployment/skaffold.yaml 

This warning is not accurate for multi-config since all artifacts are managed in the same flat list and passed along to every deployer. This triggers this warning for every helm deployment imported as a dependency.

[codecov]

Codecov Report

Merging #5468 (f70c8f6) into master (46d1c2f) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #5468      +/-   ##
==========================================
- Coverage   70.53%   70.51%   -0.03%     
==========================================
  Files         413      413              
  Lines       15982    15986       +4     
==========================================
- Hits        11273    11272       -1     
- Misses       3879     3881       +2     
- Partials      830      833       +3     

Impacted Files	Coverage Δ
pkg/skaffold/deploy/helm/deploy.go	76.05% <100.00%> (+0.22%)	⬆️
pkg/skaffold/runner/runcontext/context.go	79.82% <100.00%> (+0.36%)	⬆️
pkg/skaffold/util/tar.go	50.66% <0.00%> (-5.34%)	⬇️
pkg/skaffold/docker/image.go	78.34% <0.00%> (-1.39%)	⬇️
pkg/skaffold/docker/parse.go	87.14% <0.00%> (+0.95%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46d1c2f...f70c8f6. Read the comment docs.

[briandealwis]

These warnings are useful: people get tripped up on them, especially when porting an existing project into Skaffold. Until we ignore the tags, I don't think we should drop these warnings.

Maybe what we should do is teach the deployer here to consider image dependencies as a use.

[gsquared94]

we discussed this in the triage meeting and decided to improve the warning mechanism to only notify on images not used in the manifest by figuring out the list of required images from the manifest upfront. That would solve this issue. Closing this PR.

[gsquared94]

I want to reopen this with the following observations:

• This warning didn't always make sense even in pre-multiconfig era of Skaffold. Consider a single skaffold.yaml with an artifact with a corresponding kubectl deployer; and a helm deployer that sets up a database. During this helm chart deployment skaffold will show this warning even though the artifact is used in another deployer.
• With multiconfig and build dependencies support, we can't reliably say if any artifact needs to be a part of the helm chart. Skaffold supports deploying remote charts which don't need any artifacts.
• We can parse the helm templates to find container specs that are templated on {{.Values.image}} and but not providing any artifactOverrides in the skaffold.yaml. But that doesn't rely on the artifact list

[tejal29]

So, I ended up resurrecting a usecase for which the warning was introduced.

diff --git a/examples/helm-deployment/charts/values.yaml b/examples/helm-deployment/charts/values.yaml
index ba42ad6e1..be55ebdad 100644
--- a/examples/helm-deployment/charts/values.yaml
+++ b/examples/helm-deployment/charts/values.yaml
@@ -2,3 +2,4 @@
 # This is a YAML-formatted file.
 # Declare variables to be passed into your templates.
 replicaCount: 2
+image: gcr.io/kaniko-project/executor:latest
diff --git a/examples/helm-deployment/skaffold.yaml b/examples/helm-deployment/skaffold.yaml
index fcbf144ac..2d2dff587 100644
--- a/examples/helm-deployment/skaffold.yaml
+++ b/examples/helm-deployment/skaffold.yaml
@@ -8,5 +8,3 @@ deploy:
     releases:
     - name: skaffold-helm
       chartPath: charts
-      artifactOverrides:
-        image: skaffold-helm

I see the warning in this flow.

WARN[0007] image [gcr.io/tejal-gke1/skaffold-helm:v1.21.0-5-g8b4cfdae3-dirty@sha256:14b6a7b7b4112159117be7f5f7867379acf8286e83eb7df0b318027a6b3766ab] is not used. 
WARN[0007] image [skaffold-helm] is used instead.       
WARN[0007] See helm sample for how to replace image names with their actual tags: https://github.com/GoogleContainerTools/skaffold/blob/master/examples/helm-deployment/skaffold.yaml 

However, there are two issues here

1. The helm sample example link is not at all informative and we should point to the docs here https://skaffold.dev/docs/pipeline-stages/deployers/helm/#image-configuration
2. Not sure why the image is incorrect. It should say the gcr.io/kaniko-project/executor:latest is being used.
   Looking at the code, it plainly uses the value in artifacts.image[0]
   Maybe this side-effect was introduced when ArtifactGraph was added or even before that.

It would makes sense to

1. Remove image ** is used warning line since it is incorrect.
2. Update the document link.

Regarding @briandealwis's point. Yes, if i did not have that warning it would be hard for me to debug.
How about we only show the warning if

1. len(deployers) is 1 and len(skaffold) pipelines is 1?

Another approach is to add another field to Artifact similar to IsBase. Skip showing unused warnings for base artifacts as they may or may not be part of deployment.

[tejal29]

Few suggestions to keep the warning only if helm deployer is used in single skaffold config.

[gsquared94]

Few suggestions to keep the warning only if helm deployer is used in single skaffold config.

Implemented suggestions. PTAL @tejal29