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.

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

Add docker image for example MorpheusVM #1915

Merged
merged 5 commits into from
Feb 6, 2025
Merged

Add docker image for example MorpheusVM #1915

merged 5 commits into from
Feb 6, 2025

Conversation

aaronbuchwald
Copy link
Collaborator

This PR adds a script to build a Docker image for MorpheusVM. Since MorpheusVM needs the full HyperSDK codebase present, this is added at the root of the repo and set up so that it could support any other example VM in examples/$VM_NAME/

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@aaronbuchwald aaronbuchwald marked this pull request as ready for review February 6, 2025 15:51
Copy link
Contributor

@tsachiherman tsachiherman left a comment

Choose a reason for hiding this comment

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

looks good.

@aaronbuchwald aaronbuchwald merged commit 0cb30c8 into main Feb 6, 2025
10 checks passed
@aaronbuchwald aaronbuchwald deleted the docker branch February 6, 2025 17:25
AVALANCHEGO_IMAGE_NAME="${AVALANCHEGO_IMAGE_NAME:-avaplatform/avalanchego}"

# set git constants if available, otherwise set to empty string (say building from a release)
if git rev-parse --is-inside-work-tree > /dev/null 2>&1; then
Copy link

Choose a reason for hiding this comment

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

(No action required) Maybe follow the example of ava-labs/avalanchego#3660 to enable building the images in a non-primary worktree?

fi

# Default to the release image. Will need to be overridden when testing against unreleased versions.
AVALANCHEGO_NODE_IMAGE="${AVALANCHEGO_NODE_IMAGE:-${AVALANCHEGO_IMAGE_NAME}:${AVALANCHE_DOCKER_VERSION}}"
Copy link

Choose a reason for hiding this comment

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

(No action required) Maybe support building the avalanchego image for an as-yet unmerged hash as per the example of https://github.com/ava-labs/subnet-evm/blob/master/scripts/build_antithesis_images.sh#L24?

ARG CURRENT_BRANCH

# ============= Compilation Stage ================
FROM --platform=$BUILDPLATFORM golang:1.22.8-bullseye AS builder
Copy link

Choose a reason for hiding this comment

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

Specifying BUILDPLATFORM here without enabling cross-compilation is unlikely to result in a viable image when TARGETPLATFORM differs. For example, when BUILDPLATFORM=arm64 and TARGETPLATFORM=amd64, the resulting image will contain an arm64 binary instead of the desired amd64 one.

Note also that there are two steps in enabling cross compilation:

sudo apt -y install qemu-system qemu-user-static
- name: Build MorpheusVM Docker Image
shell: bash
run: bash -x scripts/build_docker_image.sh
Copy link

@maru-ava maru-ava Feb 6, 2025

Choose a reason for hiding this comment

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

I recommend validating image build since, by itself, executing this script without error provides no guarantee of the viability of the images produced.

Example from avalanchego: https://github.com/ava-labs/avalanchego/blob/master/scripts/tests.build_image.sh

@@ -26,6 +26,19 @@ jobs:
- shell: bash
run: scripts/tests.clean.sh

build_image:
Copy link

@maru-ava maru-ava Feb 6, 2025

Choose a reason for hiding this comment

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

(No action required) Did you mean to add a publish script in addition to this CI test? That would be a likely precondition for performing kube-based scale testing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not yet, I didn't have permission to create a new DockerHub repo, so was blocked on this and just wanted to add some check to make sure this didn't break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants