Make the rules easily reusable in workflows.

[hsyed]

We should be able to reuse the docker rules logic in other rules. Unless I am missing something we can't bazel run from within a bazel run. A lot of this work will be done for rules_k8s it would be great if the problem is addressed in a general way.

Use cases:

• Minikube environment variables injection pre incremental loading Add support for minikube rules_k8s#9.
• Authentication pre (and post) hook for pull and push rules (authentication). #147 (although this should probably have support in the rules).
• Building CI systems.

What I tried:

Tried to prototype a higher level rule that would get a "handle" to a docker_build runnable and then do something before or after incrementally loading the image.

ci_environment = rule(
  attrs = {
    "platform": attr.label(single_file = True ,executable=True, cfg = "host"),  # <-- docker_build target
  },
  executable=True,
  implementation = _impl
)

Somewhere in the rule

ctx.file_action(
  output=ctx.outputs.executable,
  content="./%s" % ctx.executable.platform.short_path,
  executable=True
)

Problems

It's not straight forward to setup the runtime context of the target rules. I tried to repurpose the logic by studying the internals and this but even if I had managed to get it to work it would not be the way forward as it depends on the internals.

What I hope for

A simple way to reuse the logic in the rules defined here. What would be nicest is a simple handle to the target that I can execute at some arbitrary point in a rule / genrule. If this is not possible some utility functions with examples would be usefull. --e.g.,

def _my_rule_impl(ctx):
  info = DefaultInfo()
  run_docker_build(ctx, ctx.attr.my_image, info)
  // my actions
  return info

[mattmoor]

@hsyed There is clearly a bug today where:

  # Works
  bazel run path/to:docker_build
  # Does not work
  bazel build path/to:docker_build
  bazel-bin/path/to/docker_build     <-- This is essentially what you want to include into other rules.

I bet fixing this would unblock you (if you "hold it" right, which is often harder than it looks with Bazel/Skylark), so I will poke at this a little to see if I can figure out how I'm "holding it wrong". :)

For your first two scenarios, I think we have a tentative plan or a solution (I've updated the linked issues).

The CI system bit is a little vague, but I'm happy to discuss (and/or pull folks in to help).

FWIW, part of the reason we do the docker load during run today is that it isn't hermetic due to the daemon dependency, so your sketch of a rule implementation probably violates that.

[mattmoor]

@hsyed Can you try some of your logic that wraps the rules with the linked PR? If you are still having issues, I'd be happy to TAL if you send me a pointer to some WIP rules?

[hsyed]

@mattmoor docker_build() targets now seems to work with the change you pushed:

def _setup_deploy_rule(kwargs):
  native.genrule(
      name = "local",
      tools = ["//:platform.image"],
      outs = ["local_deploy.sh"],
      cmd = """
# I was trying to do a $$(minikube docker-env) here but no joy -- i'll await the work from rules_k8s.
echo "$(location //:platform.image)" >$@
""",
      local = 1,
      executable = True,
  )

docker_push rules do however do not work:

def _setup_deploy_rule(kwargs):
  native.genrule(
      name = "local",
      tools = ["//ci:ax-dev-staging"], # also tried relative target path --i.e., :ax-dev-staging
      outs = ["local_deploy.sh"],
      cmd = """
echo "$(location //ci:ax-dev-staging)" >$@
""",
      local = 1,
      executable = True,
  )

It reports this, I get the same when trying to run it via bazel-bin/ci/ax-dev-staging:

./pusher/file/pusher.par: No such file or directory

[mattmoor]

I will take a look, it should be pretty easy to reproduce. Sorry for the trouble.

[mattmoor]

I pushed #153 which should make build/run separable for push as well.

[mattmoor]

Ok, #153 is in, so let me know if you are unblocked (or if I should keep whacking moles).

[mattmoor]

@hsyed I just want to follow up to see if I can close this now? Are there any remaining issues for you?

[hsyed]

I'd Want to do some more work on what I had in mind, But I'm blocked on the incremental load into minikube front. I wanted to write a deploy step that could switch between loading into minikube vs a docker push depending on the "environment" being pushed to.

One thing that occurs to me is that the pusher script will be primed to push to a specific registry:tag, I wonder if it's possible to get a pusher that takes this as an argument.

Have you made any more progress on the minikube front ? I still have to set the docker machine env before calling into bazel.

[mattmoor]

You can pass --name to the pusher script to override where it pushes.

@dlorenc for the minikube stuff.

[mattmoor]

@r2d4 may know too

[ash2k]

I've made a rule to run multiple executable bazel targets sequentially (see the PR). I tried it with several executable targets and it works.
When I tried it with container_push from rules_docker I discovered that it does not work properly - when I run it, it just exits with 1 so I started debugging.

rules_docker/container/push-tag.sh.tpl

Lines 19 to 21 in e5ebe3d

	pushd ${BASH_SOURCE[0]}.runfiles > /dev/null 2>&1
	pwd
	popd > /dev/null 2>&1

When I removed redirections that swallowed errors, turned out it cannot pushd/popd into the directories.

./build/kitt/composition.push_docker: line 19: pushd: ./build/kitt/composition.push_docker.runfiles: No such file or directory
./build/kitt/composition.push_docker: line 21: popd: directory stack empty

Turned out PR #153 introduced some assumptions about where files are actually stored on disk instead of just relying on .path/.short_path. If my understanding of bazel documentation is correct, a rule can only access what is available via the runfiles. So my rule sets them up properly, I believe. I can confirm that inside of the runfiles directory the layout of directories and files is identical in both cases - when I invoke container_push directly or via my rule. The only difference is the name of the runfiles directory itself. If rules_docker was not relying on the name on the runfiles directory but instead used just short_paths (i.e. like before #153) it seems it would work.

1. Am I doing it wrong? This is quite possible, I'm new to Bazel. How do I fix my rule, keeping it generic?
2. If I'm not doing it wrong, is my understanding of the problem correct or I'm missing something?

[ash2k]

ping @mattmoor

[mattmoor]

cc @erain

It's entirely possible that we're holding these path things wrong. My recollection is that it's a hot mess when WORKSPACE files come into play.

Could you perhaps share a small repro of what you expect to work that doesn't that @erain can look at?

[ash2k]

@mattmoor @erain
Please see https://github.com/atlassian/smith repo, branch rules_docker_148_reproducer.
Install dependencies using make setup-base update-bazel. Then run bazel run //:run_all to see the issue. Well, you'll see nothing because rules_docker hides the errors and exits with 1. See above for how I debugged it (use local_repository to pick up your local version of rules_docker).

@jayconrod I've also tried running Gazelle via this rule and it also does not work - is this not supported?

bazel run //:run_all
INFO: Analysed target //:run_all (18 packages loaded).
INFO: Found 1 target...
Target //:run_all up-to-date:
  bazel-bin/run_all.bash
INFO: Elapsed time: 0.849s, Critical Path: 0.53s
INFO: 1 process: 1 darwin-sandbox.
INFO: Build completed successfully, 5 total actions
INFO: Build completed successfully, 5 total actions
Running //:gazelle
Loading: 
Loading: 0 packages loaded
ERROR: Skipping '//:gazelle': no such package '': BUILD file not found on package path
WARNING: Target pattern parsing failed.
ERROR: no such package '': BUILD file not found on package path
INFO: Elapsed time: 0.206s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded)
FAILED: Build did NOT complete successfully (0 packages loaded)
FAILED: Build did NOT complete successfully (0 packages loaded)
error: ./gazelle.bash: script is out of date. Refresh it with the command:
  bazel build //:gazelle && cp -fv "" "./gazelle.bash"

I think Gazelle also makes some assumptions about how it is invoked. Maybe the script can now be simplified using $BUILD_WORKSPACE_DIRECTORY environment variable? 🤔

[erain]

Thanks for the pinging @mattmoor and @ash2k . I will look into the repo you provided for debug the issue (probably next week).

[jayconrod]

@ash2k I've opened bazel-contrib/bazel-gazelle#259 to track this.

The wrapper script produced by the gazelle rule was supposed to work around the lack of bazel run --direct_run. Now that that's the only way to run things, most of the complexity in there is no longer needed.

[ash2k]

I think this issue has been fixed by #522. Thanks a lot @scele! May be closed now, I believe. At least things work for me atlassian/smith#380