Change image rule to copy config instead of symlink #1571

exoson · 2020-07-22T14:08:54Z

The image rule in container/image.bzl was symlinking the config file for
strutcture tests. This caused remote execution with
--remote_download_minimal to fail as symlinks aren't yet supported.

googlebot · 2020-07-22T14:08:57Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2020-07-22T14:12:24Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

smukherj1 · 2020-07-22T16:55:03Z

/gcbrun

smukherj1 · 2020-07-22T19:00:56Z

It appears the test in https://github.com/bazelbuild/rules_docker/blob/master/tests/contrib/repro_imgs.yaml now fails, specifically, the following step:

- name: "l.gcr.io/google/bazel"
  args:
  - --output_base=/workspace/output_base
  - test
  - --test_output=errors
  - --spawn_strategy=standalone
  - --verbose_failures
  - //tests/contrib:set_cmd_repro_test
  dir: 'rules_docker'

I think the problem is in the readlink here

readlink -f bazel-bin/%s/%s

In the test, this resolves to the command:

/usr/bin/docker run -d --entrypoint '' -v /var/run/docker.sock:/var/run/docker.sock -v /workspace/output_base/execroot/io_bazel_rules_docker/bazel-out/k8-fastbuild/bin/tests/contrib/bazel_out2:/workspace/output_base/execroot/io_bazel_rules_docker/bazel-out/k8-fastbuild/bin/tests/contrib/bazel_out2 d3387731109edd0a64e068a0962a286a92392a4784b4049552c45fe414da470c sh -c 'sleep 10 && cd $(cat /set_cmd_repro_test_src_project_root.txt) && bazel --output_base=/workspace/output_base/execroot/io_bazel_rules_docker/bazel-out/k8-fastbuild/bin/tests/contrib/bazel_out2 build //tests/container:set_cmd //tests/container:set_cmd.tar //tests/container:set_cmd.digest //tests/container:set_cmd.json && mkdir /img_outs && cp bazel-bin/tests/container/set_cmd.tar /img_outs/set_cmd.tar && cp bazel-bin/tests/container/set_cmd.digest /img_outs/set_cmd.digest && cp $(readlink -f bazel-bin/tests/container/set_cmd.json).sha256 /img_outs/set_cmd.id'

which results in:

cp: cannot stat '/workspace/output_base/execroot/io_bazel_rules_docker/bazel-out/k8-fastbuild/bin/tests/contrib/bazel_out2/execroot/io_bazel_rules_docker/bazel-out/k8-fastbuild/bin/tests/container/set_cmd.json.sha256': No such file or directory

not sure why though.

exoson · 2020-07-23T08:51:47Z

Originally it followed the symlink and at the symlink target there existed this *.sha256 file. Made it so the digest file is also copied for the final layer and exposed so test can use it.

smukherj1 · 2020-07-23T15:44:38Z

container/image.bzl

+        command = "cp %s %s" % (config_file.path, output_config.path),
+    )
+    output_config_digest = ctx.outputs.config_digest
+    ctx.actions.run_shell(


Actually, this seems to be causing a failure :(:

[1A�[K(08:51:50) �[35mWARNING: �[0merrors encountered while analyzing target '//tests/docker/toolchain_container:test-rbe-test-xenial-with-pkgs': it will not be built (08:51:50) �[32mAnalyzing:�[0m 613 targets (355 packages loaded, 12039 targets configured) �[1A�[K(08:51:50) �[31m�[1mERROR: �[0mfile 'tests/docker/package_managers/ubuntu_gpg_image.json.sha256' is generated by these conflicting actions: Label: //tests/docker/package_managers:ubuntu_gpg_image RuleClass: add_apt_key rule Configuration: d769d4f1658740e0d5d58d8b62919a32f8f9956cb28577a4c785dd8ea1acb1b5 Mnemonic: Action Action key: ee4f0b4cb4c32de3f26b7455d961b208ac208084ed6be011fedd92a71e84a38b, 0aa9d5a6def5441bff64d39dd00fe787186a9fff26033595220095eb40734680 Progress message: Action tests/docker/package_managers/ubuntu_gpg_image.json.sha256 PrimaryInput: File:[[<execution_root>]bazel-out/k8-fastbuild/bin]tests/docker/package_managers/ubuntu_gpg_image.0.config.sha256, File:[[<execution_root>]bazel-out/k8-fastbuild/bin]tests/docker/package_managers/ubuntu_gpg_image.key.0.config.sha256 PrimaryOutput: File:[[<execution_root>]bazel-out/k8-fastbuild/bin]tests/docker/package_managers/ubuntu_gpg_image.json.sha256

looks like the digest file is already generated elsewhere and that action needs to be updated instead of adding a new one here

Updated with fixes for duplicates.
Btw should I be able to run something like bazel test //... locally? It doesn't work out of the box for me at least.

So we have some nested workspaces used by e2e tests that's probably messes up "..." invocations. What error do you get?

For bazel test //... I get

ERROR: /data/rules_docker/testing/new_pusher_tests/BUILD:17:15: no such package '@verify_new_pusher_image_contents//image': The repository '@verify_new_pusher_image_contents' could not be resolved and referenced by '//testing/new_pusher_tests:new_push_verify_pushed_configs_and_files.image' ERROR: Analysis of target '//testing/new_pusher_tests:new_push_verify_pushed_configs_and_files.image' failed; build aborted: Analysis failed

and if I try bazel test //tests/... I get

ERROR: /data/rules_docker/tests/container/BUILD:283:16: in env attribute of container_image_ rule //tests/container:set_env_make_vars: $(ENV_KEY) not defined. Since this rule was created by the macro 'container_image', the error might have been caused by the macro implementation ERROR: /data/rules_docker/tests/container/BUILD:283:16: in env attribute of container_image_ rule //tests/container:set_env_make_vars: $(ENV_VALUE) not defined. Since this rule was created by the macro 'container_image', the error might have been caused by the macro implementation

The image rule in container/image.bzl was symlinking the config file for strutcture tests. This caused remote execution with --remote_download_minimal to fail as symlinks aren't yet supported.

k8s-ci-robot · 2020-07-27T12:57:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: exoson, smukherj1

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [smukherj1]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

smukherj1 · 2020-07-27T12:57:15Z

/gcbrun

exoson requested review from alex1545 and smukherj1 as code owners July 22, 2020 14:08

googlebot added the cla: no label Jul 22, 2020

k8s-ci-robot added the size/XS label Jul 22, 2020

exoson force-pushed the remove_symlink branch from 4fd0f13 to 2a280af Compare July 22, 2020 14:12

googlebot added cla: yes and removed cla: no labels Jul 22, 2020

smukherj1 approved these changes Jul 22, 2020

View reviewed changes

k8s-ci-robot assigned smukherj1 Jul 22, 2020

k8s-ci-robot added lgtm approved labels Jul 22, 2020

exoson force-pushed the remove_symlink branch from 2a280af to 1c2f5cb Compare July 23, 2020 08:50

k8s-ci-robot added size/S and removed lgtm size/XS labels Jul 23, 2020

smukherj1 approved these changes Jul 23, 2020

View reviewed changes

k8s-ci-robot added the lgtm label Jul 23, 2020

smukherj1 suggested changes Jul 23, 2020

View reviewed changes

k8s-ci-robot removed lgtm approved labels Jul 23, 2020

Change image rule to copy config instead of symlink

c23abef

The image rule in container/image.bzl was symlinking the config file for strutcture tests. This caused remote execution with --remote_download_minimal to fail as symlinks aren't yet supported.

exoson force-pushed the remove_symlink branch from 1c2f5cb to c23abef Compare July 24, 2020 14:11

smukherj1 approved these changes Jul 27, 2020

View reviewed changes

k8s-ci-robot added the lgtm label Jul 27, 2020

k8s-ci-robot added the approved label Jul 27, 2020

rbe-toolchains-pr-bot merged commit 550ebd2 into bazelbuild:master Jul 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change image rule to copy config instead of symlink #1571

Change image rule to copy config instead of symlink #1571

exoson commented Jul 22, 2020

googlebot commented Jul 22, 2020

googlebot commented Jul 22, 2020

smukherj1 commented Jul 22, 2020

smukherj1 commented Jul 22, 2020

exoson commented Jul 23, 2020

smukherj1 Jul 23, 2020

exoson Jul 24, 2020 •

edited

Loading

smukherj1 Jul 27, 2020

exoson Jul 27, 2020

k8s-ci-robot commented Jul 27, 2020

smukherj1 commented Jul 27, 2020

Change image rule to copy config instead of symlink #1571

Change image rule to copy config instead of symlink #1571

Conversation

exoson commented Jul 22, 2020

googlebot commented Jul 22, 2020

googlebot commented Jul 22, 2020

smukherj1 commented Jul 22, 2020

smukherj1 commented Jul 22, 2020

exoson commented Jul 23, 2020

smukherj1 Jul 23, 2020

Choose a reason for hiding this comment

exoson Jul 24, 2020 • edited Loading

Choose a reason for hiding this comment

smukherj1 Jul 27, 2020

Choose a reason for hiding this comment

exoson Jul 27, 2020

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 27, 2020

smukherj1 commented Jul 27, 2020

exoson Jul 24, 2020 •

edited

Loading