tilt-dev / tilt

Define your dev environment as code. For microservice apps on Kubernetes.
https://tilt.dev/
Apache License 2.0
7.59k stars 298 forks source link

Option to include `.git` in `docker_build` context #2169

Open epotex opened 5 years ago

epotex commented 5 years ago

Copying from slack:

Hey team Tilt… could you please elaborate more info why docker_build() is ignoring .git directory? We relay on it in one of our services - is there away to enable the copy of .git directory with docker_build()? from the docs: Ignoring Files .git Tilt will always ignore changes under the .git directory. When you use docker_build() in your Tiltfile, Tilt will remove .git from the Docker context. (edited) epotex 12:00 PM can trace this behavior to this commit: https://github.com/windmilleng/tilt/commit/b5dae35ca5328b97b1d827cd95eff8a29cb4e6c5 But I’m not sure why it’s working the way it works :smile:

dbentley 2 hours ago I thought we maybe took that out, but maybe it's still in there... Is there any chance your .dockerignore includes .git? And what are you trying to get from the .git directory into the context?

epotex 2 hours ago No in this repo I don’t have any .dockerignore

epotex 2 hours ago We use the git hash in our asset generation for cache busting

epotex 2 hours ago We probably could use other technics for doing that but what’s the benefits to remove .git from the build?

dbentley 2 hours ago It's big, so ti takes more time to tar up.

dbentley 2 hours ago One workaround is to use custom_build to call docker build, which will skip our tarring step...

epotex 2 hours ago Thanks! That’s something that I can try to work with…(custom_build) Would you like me to open an issue for it?

epotex commented 5 years ago

@maiamcc I'd call it a bug... :D

dbentley commented 5 years ago

It'd be nice to have a way to turn off that ignoring.

maiamcc commented 5 years ago

Ah whoops, @epotex fair enough.

~Are you currently blocked on this?~ (never mind, just saw your reply in #tilt)

maiamcc commented 5 years ago

Hey @epotex, taking a closer look at this: I understand that you want your .git directory in the Docker image built by Tilt, but do you also want Tilt to watch .git (and update your image whenever it changes)? That's certainly the technically easier case to implement, but I feel like that would result in a lot of spurious updates and might not be what you want?

Along those lines: what specific information do you need out of .git? I.e. could you copy in a few specific files, or do you need the whole directory?

maiamcc commented 5 years ago

(It'd also be useful to see your Dockerfile/what operations you're doing on the .git directory/what git calls you're making, if you don't know the specific files you need -- we can figure it out from there.)

epotex commented 5 years ago

So yes, we would like Tilt to watch the .git dir as we would like to switch branches as we work with the environments that tilt was creating, even if we work on different tickets, for this example we have a webpack container that compiles assets as the dev make changes - so wehn switching to a new branch the webpack should recompile the assets - as for the usage of the .Git folder our code reads the git revision via https://www.npmjs.com/package/git-revision-webpack-plugin,

Does that provide you with the information that you need?

Thanks!

nicks commented 5 years ago

for what it's worth, git rev-parse HEAD should work ok if you just have .git/HEAD, .git/refs/heads, and an empty .git/objects dir, so maybe we could set up default rules that just include those

landism commented 5 years ago

for what it's worth, git rev-parse HEAD should work ok if you just have .git/HEAD, .git/refs/heads, and an empty .git/objects dir, so maybe we could set up default rules that just include those

also .git/packed-refs, the other files in .git/refs if they're building from a remote branch or a tag, and I wouldn't be surprised if there are other potential dependencies I'm unaware of. Digging into git internals is a bit of a rathole.

This would leave us in a situation where rev-parse magically works, but describe or log if they want other commit details magically don't work. That seems like a tricky line to walk.

epotex commented 5 years ago

I see, I think we are OK with just getting the revision hash, but others might need other functionality with git, So the idea of removing the .git folder is intentional in order to avoid watching this folder? If that's the case, maybe tilt can follow the . dockerignore file to decide what should be watched or not?

maiamcc commented 4 years ago

Just jotting down ideas: could kill two birds with one stone by implementing #3070, which has been requested in other contexts but could also be used to un-ignore .git

electrofelix commented 4 years ago

Running into this locally as we're using setuptools-scm for retrieval of the version. Typically also make use of git describe and git rev-parse to include information in built binaries to make it easier to understand what commit and/or branch code came from.

Can probably work around this via the local_resource() to switch to running setup.py with some changed options to generate a version file before building the container. Unfortunately this does make it a little harder for people to do the naive and simple approach of replicating their current builds when including information from git, and while sometimes it might be possible to workaround it maybe quite cumbersome if you don't know how the library included is picking up the version information.

electrofelix commented 4 years ago

For our golang projects I can manage this by passing some make variables via the docker build args and tweaking our Dockerfile & Makefile. For our python projects using setuptools-scm, luckily I can do something similar with:

build_args['SETUPTOOLS_SCM_PRETEND_VERSION'] = local("git describe --tags")
bkcsfi commented 4 years ago

I am using a git repo that has submodules. When building the docker image, I need to get revision information about submodules and write them to a file in the image.

The contents of the submodules can change at any point, in which case the image is rebuilt (not using live_update on this particular case).

The local trick looks handy, but I think that only runs on startup or when Tiltfile changes, in which case it won't work for my use-case. If docker_build or custom_build had the ability to call any tilt function before executing the build, then this could work for me.

build_args = {}
def update_versions():
   build_args["VERSION"] = local("some cmd")

docker_build(
   'my-image', 
   '.', 'dockerfile',
   build_args=build_args,
   before_build=update_versions
)

regarding .git in the build context vs watching .git files for changes..

I think the current out of the box experience with Tilt is a better experience by not watching .git for changes, and not automatically importing .git into the build context. I think that default makes everything faster.

But I think once you move beyond basic usage, the 'magic' handling of .git can be confusing.

My preferred use-case is wanting to include .git (or portions of .git) into the build context, but NOT wanting to watch for changes in .git.

Unfortunately ignore= is applied to both watching and including in the context.

--

unrelated, why does custom_build have deps= but docker_build has only= ?

Don't these serve the same purpose?

Also documentation for deps doesn't indicate how relative paths are interpreted.. from the Tiltfile?

kevin-lindsay-1 commented 2 years ago

Just encountered this issue. Is there a workaround?

I'm trying to build someone else's image for the sake of testing in a local k8s cluster, and everything is working perfectly fine, except for this particular bug, which completely breaks the workflow when trying to avoid the headache of pushing the image to a remote repo.

kevin-lindsay-1 commented 2 years ago

Discovered a super incredibly jank workaround. I just copy the .git folder, give the copy a different name, pass it to the image, and rename it back to .git in the image.

Oof.

TGPSKI commented 2 years ago

I am attempting to use tilt as a development tool for kubernetes / openshift operator development, however, ignoring .git directories is an issue with our makefile patterns. A critical part of docker file is calling RUN make, which then triggers git submodule activities from a operator sdk.

Choosing to ignore .git for live builds / rebuilds makes a lot of sense. However, ignoring the .git directory during builds blocks a large subset of projects that use a common SDK involving submodules.

I poked around the project a bit and was looking for a quick or easy patch that could unblock projects that depend on submodules. I didn't see anything that was obvious or easy. If anyone has an idea of where I can look to control ignores during docker builds, I would love to see it. I know filter (pathMatcher) types may have something to do with it.

Hacks that rename directories or otherwise work around ignoring git during docker builds is a non-starter for my use case.

nicks commented 2 years ago

@TGPSKI is there some reason why the custom_build workaround discussed above doesn't work for you?

cf https://docs.tilt.dev/custom_build.html

custom_build(
  'frontend',
  'docker build -t $EXPECTED_REF frontend',
  ['./frontend'],
)

(in general, tilt has lots of support for invoking image builds in different ways)

TGPSKI commented 2 years ago

@nicks that worked, super easy. I didn't actually see this workaround in the discussion above - I thought most was centered around sourcing git refs before running operations and presenting them to the build environment, instead of being included.

I migrated my build declaration from:

docker_build(
    'dvo-local/dv-operator',
    ROOT_DIR,
    dockerfile='build/Dockerfile',
    ignore=[
        'deploy/',
        'develop/',
        'hack/',
    ],
)

to the custom build resource:

custom_build(
  'dvo-local/dv-operator',
  'docker build -t $EXPECTED_REF -f ./build/Dockerfile ./',
  ['./'],
)

And now I'm building properly.

lizzthabet commented 2 years ago

since ignoring .git directory is intentional tilt behavior and using custom_build offers a good workaround, i'm going to close this issue.

electrofelix commented 2 years ago

@lizzthabet I added a note on that PR about maybe expanding on why it would not be possible (or acceptable) for tilt to ignore changes to the .git directory without removing it from the docker context by default. But maybe this is the better place so that anyone who bumps into this can understand.

Whenever someone looks at trying out tilt, I would imagine that part of the goal with docker images would be to have the default behaviour align with what they experience running docker build directly, and since tilt is sitting on top of everything else it makes sense that it can safely have other tools ignore additional tilt related files by default. However anytime someone looks to take their existing docker build ... step and replace it with docker_build, they are at risk of bumping into a difference in behaviour. Since tilt wasn't needed before it should have no impact for the docker context to ignore all tilt related files by default, but removing any source control directories is making an assumption that they were not in use before by the existing build/dev approach.

As an aside as to whether excluding .git from both tilt watching it and the docker context, assessing the impact of this difference may be difficult, because it would be unclear as to how many giving tilt a test drive might just stop at this step because it didn't work as expected compared to how many are sufficiently driven to use tilt to dig in enough to identify the problem and subsequently switch to custom_build.

I'm obviously biased as I ran into the same issue, but I can see that the current default as being the best option risks a certain amount of survivor bias in that most people who are still using tilt and working around the issue were already committed, while you might never hear from those that had a quick look and moved on when it didn't quite work.

So while I might feel that tilt should feel safe ignoring .git by default, and don't think it should remove it from the default context of any other tools called (is docker the only one impacted here), I may be overlooking issues changing this behaviour would cause both conceptually in being consistent and practically in implementing such a difference.

nicks commented 2 years ago

@electrofelix Ya, I appreciate that having tilt's docker_build work differently than docker build . is frustrating!

Let me add a bit of historical context (since this was a fairly old product decision):

when we first started building tilt, using kubernetes for inner-loop dev wasn't very common. We were definitely NOT targeting people who were perfectly happy using docker build . in dev and switching to Tilt.

Instead, we were targeting people who we thought could really benefit from this way of working, but were skeptical. When we did user research, we learned that the big skepticism was around performance - e.g. "the docker build cache never works for me! it's always spuriously rebuilding things when I don't expect it too!" And when we dug into it, we found that it was often because of the .git issue.

So we removed .git by default, because it was causing so many performance problems for people that couldn't articulate why they were having those problems.

I don't know that there's a good product answer here.

all of that said, i think it should be WAY easier to implement docker_build(include_git_metadata=True) now, most of the work was in https://github.com/tilt-dev/tilt/pull/5708, which went in last week

kevin-lindsay-1 commented 2 years ago

To me the product answer would be to make it clear which file(s) triggered an update in the UI, and where that particular inclusion or exclusion rule came from, and allow overrides. I should be allowed to shoot my own foot, but it should be obvious when I'm using it who is shooting my foot (me or tilt).

I don't have an issue with tilt having reasonable defaults, however I just don't want it to do magic that I could only discover via reading the "correct page" in the docs, and I always want to be able to control it at the end of the day.

As for the custom build solution, I'm not particularly in love with the idea of needing to make my own "docker build" command just to override an option.

It feels like the answer we're getting here is "we put this rule in place with good intentions but as a break/fix and not as a designed feature. it would be too hard to parameterize this, because tilt is now coupled to this behavior, if you want to use it, it's possible but not supported".

lizzthabet commented 2 years ago

@electrofelix and @kevin-lindsay-1, i really appreciate your thoughtful feedback! i didn't participate in the original discussions, so @nicks historical context is useful for me to understand as well.

based on what y'all have shared, it sounds like:

let me know if there are things i've missed or misinterpreted!