Skip to content

skaffold render exits normally when no inputs are found #4979

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

Closed
ekupershlak opened this issue Oct 30, 2020 · 5 comments · Fixed by #6043
Closed

skaffold render exits normally when no inputs are found #4979

ekupershlak opened this issue Oct 30, 2020 · 5 comments · Fixed by #6043
Assignees
Labels
area/render kind/friction Issues causing user pain that do not have a workaround planning/Q2-21 priority/p2 May take a couple of releases
Milestone

Comments

@ekupershlak
Copy link

Expected behavior

Running skaffold render --digest-source=none using a skaffold.yaml that has no inputs exits with a non-zero 0.

Actual behavior

skaffold exits with a code of 0.

$ skaffold render --digest-source=none
--digest-source set to 'none', tags listed in Kubernetes manifests will be used for render
WARN[0000] k8s-* did not match any file                 
$ echo $?
0

Information

Steps to reproduce the behavior

  1. Clone the skaffold project
  2. cd examples/getting-started; rm k8s-pod.yaml
  3. skaffold render --digest-source=none
@tejal29 tejal29 added kind/friction Issues causing user pain that do not have a workaround kind/question User question area/render labels Nov 2, 2020
@tejal29
Copy link
Contributor

tejal29 commented Nov 2, 2020

@ekupershlak It does make sense to error out in this case.
However, by design we always set kubectl deployer as default deployer.

We had a bunch of discussion here #4172 to change that default.
However, the team felt we shd keep this as is.

We could error out when kubectl deployer does not end up finding any manifests in the default location.

@tejal29 tejal29 added the priority/p2 May take a couple of releases label Nov 2, 2020
@ekupershlak
Copy link
Author

I think the error when no deployer is specified is a bit different - something like:

WARN[0000] k8s/*.yaml did not match any file 

I agree that this should probably be an error (k8s/*.yaml isn't actually in any of my config, so it's an unexpected error message).

I think that I'm describing a slightly different case - I'm proposing treating any case where no manifests are found as an error. Consider the following skaffold.yaml.

apiVersion: skaffold/v2beta9
kind: Config
build:
  artifacts:
  - image: skaffold-example
deploy:
  kubectl:
    manifests:
      - no-such-files-*

It specifies a deployer, but the manifests pattern doesn't match anything.

@nkubala nkubala removed the kind/question User question label Nov 16, 2020
@nkubala
Copy link
Contributor

nkubala commented Nov 16, 2020

yeah this seems like something we could just validate at runtime. feels like if files are specified in the skaffold.yaml that don't exist we should give an error saying so.

@aaron-prindle
Copy link
Contributor

aaron-prindle commented Jun 11, 2021

Can somewhere here verify that the suggested solution - adding a runtime validation on listed files, is acceptable? Additionally if someone does glob matching on a directory - ex: yamls/* (vs directly listing a file), does there have to be validation in those cases as well (1 or more files match the glob)?

@aaron-prindle
Copy link
Contributor

Talked w/ @nkubala offline, adding notes here:

we do some pre-run config validation, mostly related to modules e.g. making sure all configs have unique names, etc. most of this lives in https://github.com/GoogleContainerTools/skaffold/blob/master/pkg/skaffold/parser/config.go

[...] probably start here to find the entrypoints where you could add more validation around inputs, etc.

and for globs i think it would be reasonable to make sure at least one file matches the glob pattern passed. e.g. if a user passes k8s/*.yaml and there's either no k8s directory or it's empty, then error out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/render kind/friction Issues causing user pain that do not have a workaround planning/Q2-21 priority/p2 May take a couple of releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants