Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Release x86_64 & arm64 docker images built by bazel system #670

Merged
merged 3 commits into from
Aug 26, 2024
Merged

Release x86_64 & arm64 docker images built by bazel system #670

merged 3 commits into from
Aug 26, 2024

Conversation

0405ysj
Copy link
Collaborator

@0405ysj 0405ysj commented Aug 12, 2024

I tried to build docker image via bazel, but haven't worked. During some experimentals, I found rlocation command in debs-tar-builder haven't worked properly. So removed the logic around using rlocation, with replacing to the one based on PWD.

$ cd /path/to/android-cuttlefish
$ bazel clean
$ bazel build docker/orchestration_image_tar

@0405ysj 0405ysj requested review from ser-io and removed request for Databean August 12, 2024 05:41
@0405ysj 0405ysj added the kokoro:run Run e2e tests. label Aug 12, 2024
@0405ysj
Copy link
Collaborator Author

0405ysj commented Aug 12, 2024 •
edited
Loading

But I'm not certain if this PR is right for E2E test on our CI. Let's watch the presubmit result in advance.

It failed on e2e-test-orchestration presubmit test. Currently investigating how to use bazel-based script for deploying docker image via iterating github action.

@GoogleCuttlefishTesterBot GoogleCuttlefishTesterBot removed the kokoro:run Run e2e tests. label Aug 12, 2024
@0405ysj 0405ysj requested a review from ser-io as a code owner August 12, 2024 06:08
@0405ysj 0405ysj force-pushed the docker_deploy branch 7 times, most recently from 2d2d902 to 378c9d9 Compare August 12, 2024 08:19
ser-io
ser-io requested changes Aug 12, 2024
View reviewed changes
Copy link
Member

@ser-io ser-io left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the PR description and the actual code it's not clear what this PR is about. It seems it's still work in progress, although review was requested.

The BAZEL rules within docker were specifically tailored to be used within the e2etest Bazel project, they might need some changes if wanted to be used standalone. To build the image, within e2etests directory run: bazel build @images//docker:orchestration_image_tar

@0405ysj
Copy link
Collaborator Author

0405ysj commented Aug 13, 2024

It seems it's still work in progress, although review was requested.

Yes, It's WIP. I tried to remove all reviewers, but Github system automatically added the reviewer again :(

To build the image, within e2etests directory run: bazel build @images//docker:orchestration_image_tar

I'll follow your suggestion in the future trial. Though this WIP PR is with docker/orchestration/build.sh, I think using bazel is better for unifying the image for the deployment and the test.

@0405ysj 0405ysj force-pushed the docker_deploy branch 7 times, most recently from 90a7f25 to 49fff90 Compare August 13, 2024 01:52
ser-io
ser-io requested changes Aug 22, 2024
View reviewed changes
.github/workflows/presubmit.yaml Outdated Show resolved Hide resolved
@0405ysj
Copy link
Collaborator Author

0405ysj commented Aug 22, 2024

How do we get the host packages now within the image?

With running docker image, we can take host debian packages in the path /root/debian of docker image. But I haven't considered the scenario for obtaining host packages. Does it matter? AFAIK, nothing except docker image uses that approach. Do we?

@0405ysj 0405ysj marked this pull request as draft August 22, 2024 04:09
@0405ysj 0405ysj marked this pull request as ready for review August 22, 2024 04:32
@0405ysj 0405ysj requested a review from ser-io August 22, 2024 04:32
@0405ysj 0405ysj changed the title Release x86_64 docker image built by bazel system Release x86_64 & arm64 docker images built by bazel system Aug 22, 2024
@ser-io
Copy link
Member

ser-io commented Aug 22, 2024

How do we get the host packages now within the image?

With running docker image, we can take host debian packages in the path /root/debian of docker image. But I haven't considered the scenario for obtaining host packages. Does it matter? AFAIK, nothing except docker image uses that approach. Do we?

I didn't understand the question. Rephrasing my original question, the commit message says: Deprecate docker image for building host packages, if we deprecate the docker image for building the host packages, how do we get the host packages in the exported docker image?

ser-io
ser-io requested changes Aug 22, 2024
View reviewed changes
docker/Dockerfile Outdated Show resolved Hide resolved
docker/Dockerfile Show resolved Hide resolved
docker/Dockerfile Outdated Show resolved Hide resolved
@ser-io
Copy link
Member

ser-io commented Aug 22, 2024

Commit 74b1d12 is a really good cleanup as it simplifies the logic around building the host packages and the docker image. However, I didn't understand what the commit was about by reading its description.

WeiChungHsu
WeiChungHsu approved these changes Aug 22, 2024
View reviewed changes
Copy link
Collaborator

@WeiChungHsu WeiChungHsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Didn't see any issue in workflow side.

.github/workflows/docker.yaml Outdated Show resolved Hide resolved
.github/workflows/docker.yaml Outdated Show resolved Hide resolved
.github/workflows/host-image-ci.yml Show resolved Hide resolved
@0405ysj 0405ysj marked this pull request as draft August 23, 2024 01:09
@0405ysj 0405ysj force-pushed the docker_deploy branch 4 times, most recently from 21f1553 to 436e4fd Compare August 23, 2024 01:23
@0405ysj 0405ysj marked this pull request as ready for review August 23, 2024 01:24
@0405ysj 0405ysj requested a review from ser-io August 23, 2024 01:24
@0405ysj 0405ysj added the kokoro:force-run Trigger a presubmit build unconditionally. label Aug 23, 2024
@GoogleCuttlefishTesterBot GoogleCuttlefishTesterBot removed the kokoro:force-run Trigger a presubmit build unconditionally. label Aug 23, 2024
0405ysj added 3 commits August 26, 2024 09:47
@0405ysj
Cleanup docker/ in android-cuttlefish repository
ca43627 
This commit includes:
- Make cuttlefish-orchestration buildable without building and running
  any other docker images
- Make bazel buildable in the top dir of this repository

Now docker image could be built with one of following steps:
- cd docker && ./image-builder.sh
- bazel build --sandbox_writable_path=$HOME //docker:orchestration_image_tar
- cd e2etests && bazel build --sandbox_writable_path=$HOME @images//docker:orchestration_image_tar

Docker image could be still tested with Host Orchestrator:
- cd e2etests && bazel test --sandbox_writable_path=$HOME --test_output=errors --local_test_jobs=1 orchestration/...
@0405ysj
Deploy x86_64 docker image into github release
b6a3a6e
@0405ysj
Deploy arm64 docker image into github release
48fa2b2
@0405ysj 0405ysj added the kokoro:force-run Trigger a presubmit build unconditionally. label Aug 26, 2024
@GoogleCuttlefishTesterBot GoogleCuttlefishTesterBot removed the kokoro:force-run Trigger a presubmit build unconditionally. label Aug 26, 2024
@0405ysj 0405ysj merged commit 57ca01e into google:main Aug 26, 2024
7 checks passed
@0405ysj 0405ysj deleted the docker_deploy branch August 26, 2024 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WeiChungHsu WeiChungHsu WeiChungHsu approved these changes

@ser-io ser-io ser-io approved these changes

@ikicha ikicha Awaiting requested review from ikicha ikicha is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@0405ysj @ser-io @WeiChungHsu @GoogleCuttlefishTesterBot