Error if no Dockerfiles are found for skaffold init --analyze #1810
Conversation
priyawadhwa
requested review from
balopat,
dgageot,
nkubala and
r2d4
as code owners
This should fix GoogleContainerTools#1795; now, if no Dockerfiles are found in the workspace then skaffold will error out. If no k8s manifests are found it will error out (already implemented).
Codecov Report
@@ Coverage Diff @@
## master #1810 +/- ##
==========================================
+ Coverage 45.53% 45.55% +0.01%
==========================================
Files 143 143
Lines 6685 6687 +2
==========================================
+ Hits 3044 3046 +2
Misses 3335 3335
Partials 306 306
Continue to review full report at Codecov.
|
pkg/skaffold/initializer/init.go
Outdated
| @@ -290,6 +290,9 @@ func generateSkaffoldPipeline(k Initializer, dockerfilePairs []dockerfilePair) ( | |||
| } | |||
|
|
|||
| func printAnalyzeJSON(out io.Writer, dockerfiles, images []string) error { | |||
| if len(dockerfiles) == 0 { | |||
There was a problem hiding this comment.
This condition is used here as well.
skaffold/pkg/skaffold/initializer/init.go
Line 124 in 0211c2c
@dgageot and @priyawadhwa should we consider failing skaffold init if no docker files are present?
AFAIK, analyze is used by IDE? What is it used for?
There was a problem hiding this comment.
Can we get some more context around why skaffold init should fail when there are no docker files?
There was a problem hiding this comment.
skaffold init currently fails if there are no Dockerfiles or Kubernetes manifests found, this PR adds the same functionality to skaffold init --analyze.
I'm guessing skaffold init fails if either is not present because then there is nothing to match either an image or a k8s manifest with when generating the skaffold.yaml. The --analyze flag was recently added so that IDEs could support skaffold init (ref #1710)
There was a problem hiding this comment.
It does not right now. Can we move this check to L112
And then remove it from if c.SkipBuild flow as well as the --analyze flow.
There was a problem hiding this comment.
Sorry, I thought we only want to see this error if !c.skipBuild as written? Because if we're skipping the build section of the skaffold.yaml, it shouldn't matter if there are any Dockerfiles?
In this case, should I check if !c.SkipBuild for --analyze as well?
There was a problem hiding this comment.
yes makes sense. I think we should also change the error message. Technically you don't need a Dockerfile for running skaffold. You can't "create" skaffold config file with sane config without docker files.
This should fix #1795; now, if no Dockerfiles are found in the workspace
then skaffold will error out. If no k8s manifests are found it will
error out (already implemented).