Expose architecture in container_image

[murilofv]

container_image builds all images labeled as amd64 despite the
possibility of it being intended for another architecture, without a way
to overwrite this value. Aside from being confusing to a user, this could
also be an issue when using said images as part of a manifest list, given
the wrong label won't pull the proper image to the system.

This patch exposes an architecture attribute that can be used by an user
in order to properly label the container image. The default is still amd64
in case it is not set.

It also adds a sh_test called architecture_test to verify that when a
labeled is set it was actually be properly modified in the config of the
target image.

Signed-off-by: Murilo Fossa Vicentini [email protected]

Fixes #1421

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[murilofv]

@smukherj1 is this what you had in mind regarding your comment in #1421 (comment) ?

[murilofv]

@googlebot I signed it!

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[smukherj1]

Thanks! The PR looks good! Could you please add a test? I suggest adding a container_image target here and then maybe write a sh_test that depends on the <target>.json and validates that the architecture string is present in the config.

[murilofv]

Thanks! The PR looks good! Could you please add a test? I suggest adding a container_image target here and then maybe write a sh_test that depends on the <target>.json and validates that the architecture string is present in the config.

Sure, I can do that, do you want me to run it as as part of any automated test currently in place like .bazelci/presubmit.yml or testing/e2e.sh or you want me to just add it so folks can run it manually if needed?

[smukherj1]

Thanks! The PR looks good! Could you please add a test? I suggest adding a container_image target here and then maybe write a sh_test that depends on the <target>.json and validates that the architecture string is present in the config.

Sure, I can do that, do you want me to run it as as part of any automated test currently in place like .bazelci/presubmit.yml or testing/e2e.sh or you want me to just add it so folks can run it manually if needed?

Would prefer it runs automatically in the Bazel CI presubmit which it should (if added as an sh_test target) unless explicitly excluded in the .bazelci/presubmit.yaml file.

[murilofv]

Would prefer it runs automatically in the Bazel CI presubmit which it should (if added as an sh_test target) unless explicitly excluded in the .bazelci/presubmit.yaml file.

I might be missing something, was it expected to run automatically indeed? I am looking at the build results in https://buildkite.com/bazel/rules-docker-docker/builds/3872 and I don't see the test listed there. I assume it doesn't actually run automatically and I need to explicitly make it a part of test_targets in .bazelci/presubmit.yaml, is this the case?

[smukherj1]

Would prefer it runs automatically in the Bazel CI presubmit which it should (if added as an sh_test target) unless explicitly excluded in the .bazelci/presubmit.yaml file.

I might be missing something, was it expected to run automatically indeed? I am looking at the build results in https://buildkite.com/bazel/rules-docker-docker/builds/3872 and I don't see the test listed there. I assume it doesn't actually run automatically and I need to explicitly make it a part of test_targets in .bazelci/presubmit.yaml, is this the case?

Apologies. I was assuming we used bazel test ... to run the test and only listed -<taget> for tests we wanted to exclude. I see this was changed by this commit. So yes, please go ahead and add your new test to the presubmit.yaml for all the test environments.

[murilofv]

Would prefer it runs automatically in the Bazel CI presubmit which it should (if added as an sh_test target) unless explicitly excluded in the .bazelci/presubmit.yaml file.

I might be missing something, was it expected to run automatically indeed? I am looking at the build results in https://buildkite.com/bazel/rules-docker-docker/builds/3872 and I don't see the test listed there. I assume it doesn't actually run automatically and I need to explicitly make it a part of test_targets in .bazelci/presubmit.yaml, is this the case?

Apologies. I was assuming we used bazel test ... to run the test and only listed -<taget> for tests we wanted to exclude. I see this was changed by this commit. So yes, please go ahead and add your new test to the presubmit.yaml for all the test environments.

No problem, I added there an it shows up now, thank you. Let me know if you have any additional feedback / concerns regarding the patch.

[smukherj1]

/gcbrun

[smukherj1]

Thanks!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: murilofv, 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

[murilofv]

Just out of curiosity, now that I noticed, did something happen that the commit message body was not added during the merge? This doesn't look like something expected, but I unfortunately deleted the branch / fork after the merge happened and looks like the references were lost, so haven't been able to look into what could possibly have happened.