Skip to content
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

skaffold cloudbuild ignores gitignore #1998

Closed
panitaxx opened this issue Apr 22, 2019 · 14 comments · Fixed by #2104
Closed

skaffold cloudbuild ignores gitignore #1998

panitaxx opened this issue Apr 22, 2019 · 14 comments · Fixed by #2104

Comments

@panitaxx
Copy link
Contributor

Expected behavior

Cloudbuild builds should files in ignore .gitignore

Actual behavior

It uploads .git and uploads node_modules.

Sometime they are ignored sometimes they aren't. I have another project with same .gitignore and no .dockerignore file in which .git and node_modules dirs are not uploaded. I provided a sample repo in which everything is ignore notwithstanding the contents of .gitignore.

Information

  • Skaffold version: v0.27.0
  • Operating system: macOs mojave
  • Contents of skaffold.yaml:
apiVersion: skaffold/v1beta8
kind: Config
build:
  artifacts:
  - image: gcr.io/ipcom-vcc/example
    sync:
      '*.js': .
  googleCloudBuild:
    projectId: ipcom-vcc
deploy:
  kubectl:
    manifests:
    - k8s/pod.yaml

Steps to reproduce the behavior

  1. pull repo https://github.com/panitaxx/skaffold-test
  2. npm install (so to have a local node_modules)
  3. skaffold run (change project name in skaffold.yaml)
  4. download the generated tar.gz from cloudstorage to see what is uploaded

This may not be an issue with small projects, but I have here a project with >1000 packages in node_modules and almost 150mb in .git and is very slow to upload.

@balopat
Copy link
Contributor

balopat commented Apr 23, 2019

Hey, thanks for filing this issue!
GCB builder definitely does not take .gitignore in account.
The way it works:

  • it calculates the files required for your Dockerfile build (we call them dependencies) - and it honors .dockerignore when it does this!!
  • it tars up the dependencies and sends it as the build context to the GCB build which is just a single step cloudbuild.yaml that calls docker build

So. To reduce the size, I would recommend using .dockerignore. You are using npm install anyway in your Dockerfile that gets the dependencies during build.

@balopat balopat added the kind/question User question label Apr 23, 2019
@panitaxx
Copy link
Contributor Author

Thanks for your answer. I did add a dockerignore file. IMHO if the skaffold tool is going to support cloudbuild It should use the .gitignore file as this is used when doing gcloud build submit and it is easier to adapt your code to use this tool. Just a suggestion.

@corneliusweig
Copy link
Contributor

@panitaxx To me it would feel like an abuse of .gitignore when using it as an additional .dockerignore. Just imagine somebody has assets in his workingdir which he does not want in git (-> .gitignore), but nevertheless are important for his docker build (-> NOT .dockerignore). Semantically, .gitignore does git stuff, whereas .dockerignore does docker stuff.

The node examples in the Skaffold repo all contain a .dockerignore with ignored node_modules. Where else would you look for such documentation?

@panitaxx
Copy link
Contributor Author

I feel is more something of developer experience. If I am using cloudbuild and it uses gitignore and dockerignore then when I incorporate to my workflow skaffold I expect cloudbuild in skaffold to work like cloudbuild outside skaffold. Also it would be maintaining another file with little differences (like dockerignore most of the time has to add .git).

@priyawadhwa
Copy link
Contributor

@panitaxx based on the docs it looks like cloudbuild only respects the .gcloudignore file (which can pull in contents from a .gitignore). Is that what you're doing?

@panitaxx
Copy link
Contributor Author

panitaxx commented Apr 27, 2019

After reading the docs of gcloud build submit (they are a little bit confusing), it seems gcloud builds submit uses a default .gcloudignore that includes the .gitignore contents in the build, therefore in a transient way respecting .gitignore. I also made a small test to prove this. So in the DX aspect is the same if you add something to .gitignore it will be ignored by gcloud build submit command but not by skaffold. Also tested and gcloud build does not respect .dockerignore.

@tejal29
Copy link
Member

tejal29 commented May 1, 2019

Thanks @panitaxx. We were not aware gcloud build submits includes the contents of .gitignore.
It seems reasonable to duplicate the behavior. Do you want to submit a PR for the same?

Thanks
Tejal

@panitaxx
Copy link
Contributor Author

panitaxx commented May 2, 2019

Ok give a couple of a days. This is my first PR ever.

@tejal29
Copy link
Member

tejal29 commented May 2, 2019

Looking forward to it!

@priyawadhwa
Copy link
Contributor

priyawadhwa commented May 2, 2019

cc @tejal29 @panitaxx

I'm not sure if we actually want to add support for this -- gcloud build submit is meant for running an arbitrary cloudbuild.yaml, but skaffold is using GCB specifically as a hosted docker build (we don't even run gcloud build submit). Personally, I think it only makes sense to support the .dockerignore in this case. WDYT?

@tejal29
Copy link
Member

tejal29 commented May 3, 2019

Thanks @priyawadhwa. Makes sense what you just mentioned.
@panitaxx skaffold is not trying to provide GCB builder functionality but uses GCB to do a remote docker build.
So, we only respect ".dockerignore".
Is there a reason why you don't want to not add node_modules and git in doeckerignore ?
https://nodejs.org/en/docs/guides/nodejs-docker-webapp/

@panitaxx
Copy link
Contributor Author

panitaxx commented May 3, 2019

I guess it's just a little programmer's laziness and the fact that you would have to maintain two very similar files (plus the whole skaffold setup). It's no biggie. Maybe it should be explained in the docs only?

@priyawadhwa
Copy link
Contributor

Definitely think it should be clarified in the docs. Would you be open to submitting a PR @panitaxx?

@panitaxx
Copy link
Contributor Author

panitaxx commented May 7, 2019

yes. will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants