Rules_docker: Workspace status isn't enough to tag images

[mouadino]

Problem

Currently, it's possible to pass stamp=True to container_push rule to be able to tag images from workspace status, however for us this means that we need to put a lot of logic in the workspace status and since this later run each time for everything (whether it needs building or not) it makes building step slower

Proposed Solution

What I was thinking of doing is adding an extra attribute to container_push (and maybe to container_build too) to be able to pass a file to use for stamping beside bazel-out/volatile-status.txt and stable-status.txt), e.g.

gen_rule(
    name = "gen_image_hash",
    outs = ["status.txt"],
    cmd = "echo FOO bar > $@",
)

container_push(
    name = "push",
    image = ":image",
    stamp_inputs = [":gen_image_hash"],
    tag="{FOO}",
)

This shouldn't be that hard to do as we have already the building code here https://github.com/bazelbuild/rules_docker/blob/58d022892232e5d59daba7760289976d5f6e7433/container/push.bzl#L37, however, I was wondering how does this look like? in other words, if I make this patch will it be accepted?

[mattmoor]

/cc @ixdy who taught me everything I know about stamping :)

Generally stamping injects non-hermetic/non-deterministic information into the build, so either your introducing non-determinism into your build via the genrule, or stamping (as-is) may not be the right tool for the job. There may be a use case for stamping with values that are legitimately the output of a build step, but I just wanted to call out the potential dangers of what you are suggesting.

If you can share more, I'd love to know more :)

@davidstanke is there an expert from the Bazel team I should include on stamping related questions?

[mouadino]

Sure, I can share more, so what we currently do is tag images using the
hash of its dependencies, that's how we did it before Bazel and we are
looking to keep the same behavior with Bazel, currently, we do that as part
of the workspace status script, where we loop over all packages (in the general
sense of the word) and then compute an md5 hash of each package from
specific files in this later, however, since currently, we do it using
workspace status this script get to run each time although you may have a
non-dirty build which is slow (avg of 8s), beside that putting this logic
in a centralized place like "workspace status" doesn’t fit our use case
(since its hidden and couple packages hence teams).

Reason for tagging this way is that its easier for us, given the way our
deployment is done, to recompute the tag in deployment phase from the
repository, and with that tag we know which version to deploy.

I hope that makes it clear.

As for the hermetic subject, correct me if I am wrong, but I don’t think
genrule is much different than a workspace status beside the fact that one
run only once something change while other run each time, right?

On Thu 15. Feb 2018 at 18:22, Matt Moore [e-mail] wrote:

/cc @ixdy https://github.com/ixdy who taught me everything I know about
stamping :)

Generally stamping injects non-hermetic/non-deterministic information into
the build, so either your introducing non-determinism into your build via
the genrule, or stamping (as-is) may not be the right tool for the job.
There may be a use case for stamping with values that are legitimately the
output of a build step, but I just wanted to call out the potential dangers
of what you are suggesting.

If you can share more, I'd love to know more :)

@davidstanke https://github.com/davidstanke is there an expert from the
Bazel team I should include on stamping related questions?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/bazelbuild/rules_docker/issues/322#issuecomment-365999414,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABzBcyZhce9yYoW1f_LPASrsXDS8-TpJks5tVGfLgaJpZM4SHLu-
.

[mouadino]

Any update on this, we made the change in our fork here https://github.com/bazelbuild/rules_docker/compare/master...NewStore:dev?expand=1 which for now it impacts only container_push but we want to do the same for container_build and so far it works well and I am still curious what other things about this? and mainly would be possible to have such a thing upstream?

[ixdy]

So, a long time ago, back when the docker rules still lived in bazelbuild/bazel, I actually started work on being able to use files for stamping. In our particular use case at the time, we were wrapping go binaries in docker images, and were using the md5 hash of the binary as the docker tag.

I took inspiration from the labels attribute and used @ as a special value; I updated docker_build to take a new image_tags argument, which would either be static values, "make" variables, or a filename (as "@foo.whatever") which would read that file as input.

We eventually went in a different direction; I realized that using the git version made more sense for tags, and the current workspace-based stamping was implemented for the docker rules, which we now use.

It does still seem reasonable to me to have support for reading metadata (such as tags) from the output of another build rule; prior art includes the labels attribute I mentioned above, as well as the version_file attribute from pkg_rpm.

It'd be nice if we could converge on a consistent approach for rules. I'm not sure if the bazel team has any guidance on this.

[ixdy]

As far as the @ convention vs. additional attributes for files - there was no precedent 2 years ago. I'm not sure if anything has changed, maybe @mattmoor or @damienmg can clarify. :)

[mattmoor]

A very loose precedent is (what I believe) are called "response files". e.g. if a command-line gets too long, many commands have adopted the pattern of accepting their flags via a file e.g. foo @foo.rsp where foo.rsp is just a flag-per-line text file.

In the context of Bazel, the @ is confusing because of repository_rule, which I wasn't aware of at the time.

It'd be nice if we could converge on a consistent approach for rules. I'm not sure if the bazel team has any guidance on this.

I'd not only echo this, but go a step further. I say one of the greatest pieces of value of Bazel is the normality across languages/rules. Every time a rules_* author reinvents the wheel a subtly different way dilutes that value.

@davidstanke Something Bazel needs for rule authors is a set of principles like this which clearly document the "dos" and "don'ts".

[numbsafari]

I'd like to :+1: this with a bit of added context.

In past efforts, I've used the approach that bazel seems to be taking here: utilizing the "git version" to tag images. The seeming benefit of this over, for example, semver tags, being that you know what source generated your image and it is, therefore, hopefully not mutable the way that semver tagging is . It's also much more automatic, meaning that you don't have to remember to update a "version.txt" file somewhere ever time you want to cut a new build, and it also handles branch merging in that if I'm working on "3.1.25" at the same time as another developer, then the one who merges first doesn't "own" the tag.

A critical downside to the "git version" approach, though, is actually branching, especially when working in a mono-repo. Say, for example, I have two packages, //services/A and //services/B. On my current branch, I'm working on //services/B. I've dutifully made a few commits to my branch and now I want to make a build. Unfortunately, this either causes a rebuild of //services/A or it creates a situation where I need to do some magic to figure out what the appropriate tag for //services/A is supposed to be.

To work around this, we have been using the approach outlined by @mouadino, more or less, of computing the content-hash (really a hash-tree) of the source files and dependencies for the container in order to determine its tag. This means that, in the case outlined above, the tag for //services/A is stable across branches where it or its dependencies are not modified.

It seems like some other rules are attempting to use the digest, which I think is a somewhat similar approach, but it depends on actually loading things into docker.

Would it make sense to have some kind of rule like sha256 or md5 that could compute the hash of a given dependency set and then use this to produce a file that could be used for stamping? In my current (Make-bubble-gum-and-bailing-wire-based) build system, these are computed for dependencies in such a way that they are cached such that there's some efficiency gain between build runs where dependencies don't change.

[bigkraig]

@numbsafari @mouadino Can either of you comment on how this was accomplished? The fork that was mentioned no longer exists.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_docker!

[github-actions[bot]]

This issue was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"