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

[mysten-service] 6/n update the sui-services dockerfile with new service #15873

Merged
merged 2 commits into from
Jan 25, 2024

Conversation

after-ephemera
Copy link
Member

Description

The sui-services docker image exists to support non-node-operator services. Put new services generated in the sui repo into it to bundle them with other services.

Test Plan

» RUST_LOG=debug cargo r -- s i --path ../my-dumb-service
    Blocking waiting for file lock on build directory
   Compiling suiop-cli v0.1.5 (/Users/jkjensen/mysten/sui/crates/suiop-cli)
    Finished dev [unoptimized + debuginfo] target(s) in 14.76s
     Running `/Users/jkjensen/mysten/sui/target/debug/suiop s i --path ../my-dumb-service`
2024-01-23T18:42:03.699278Z  INFO suioplib::cli::service::init: creating rust service in ../my-dumb-service
2024-01-23T18:42:03.699616Z DEBUG suioplib::cli::service::init: sui service: true
diff --git a/docker/sui-services/Dockerfile b/docker/sui-services/Dockerfile
index e7962453ac..ff0100e7a3 100644
--- a/docker/sui-services/Dockerfile
+++ b/docker/sui-services/Dockerfile
@@ -41,6 +41,7 @@ COPY sui-execution sui-execution
 COPY narwhal narwhal
 COPY external-crates external-crates
 RUN cargo build --release \
+    --bin my-dumb-service \
     --bin sui-oracle
 
 # Production Image
@@ -52,4 +53,3 @@ ARG BUILD_DATE
 ARG GIT_REVISION
 LABEL build-date=$BUILD_DATE
 LABEL git-revision=$GIT_REVISION
-

If your changes are not user-facing and do not break anything, you can skip the following section. Otherwise, please briefly describe what has changed under the Release Notes section.

Type of Change (Check all that apply)

  • protocol change
  • user-visible impact
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

Copy link

vercel bot commented Jan 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-core ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2024 11:37pm
sui-typescript-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2024 11:37pm
4 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) Visit Preview Jan 24, 2024 11:37pm
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Jan 24, 2024 11:37pm
mysten-ui ⬜️ Ignored (Inspect) Visit Preview Jan 24, 2024 11:37pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Jan 24, 2024 11:37pm

@pei-mysten
Copy link
Contributor

I have a general question, if i understand it correctly, we're trying to bake all the binaries into a single docker image, right?
I feel like it's better to use a common base image - then build different sub-images from there.
For example, this is a ts service I built lately

FROM node:18.16.0-alpine3.17 as base
ARG PNPM_VERSION=8.1.1
RUN apk add g++ make py3-pip
RUN npm --global install pnpm@${PNPM_VERSION}

WORKDIR /app

COPY . ./

RUN pnpm install

FROM base as app1
RUN pnpm turbo build --filter ./apps/app1
ENV NODE_ENV production
CMD [ "pnpm", "--filter=./apps/app1", "start" ]

FROM base as app2
RUN pnpm turbo build --filter ./apps/app2
ENV NODE_ENV debug
CMD [ "pnpm", "--filter=./apps/app2", "start" ]

@after-ephemera
Copy link
Member Author

@pei-mysten since rust binaries are relatively tiny I planned on combining sui repo binaries into this single image (similar to how we do for the sui-tools image. sui-services was originally a spin off from that image for services that weren't necessarily relevant to node operators. This image also acts as a base image for internal service-specific images at Mysten. I'm open to rolling a new image per service, but we'd also need to add each one to CI and manage them individually. I expect that many of these services will be tiny and provide limited utility, so I wanted to start as simply as possible and then if we need to rip apart the image into multiple in the future we can.

## Description 

Describe the changes or additions included in this PR.

## Test Plan 
```
» RUST_LOG=debug cargo r -- s i --path ../my-dumb-service                                         
   Compiling suiop-cli v0.1.5 (/Users/jkjensen/mysten/sui/crates/suiop-cli)                                                                    
    Finished dev [unoptimized + debuginfo] target(s) in 2.62s
     Running `/Users/jkjensen/mysten/sui/target/debug/suiop s i --path ../my-dumb-service`                                                     
2024-01-23T20:39:35.910330Z  INFO suioplib::cli::service::init: creating rust service in ../my-dumb-service                                    
2024-01-23T20:39:35.910729Z DEBUG suioplib::cli::service::init: sui service: true  
```

```
diff --git a/Cargo.toml b/Cargo.toml
index afa17929a1..b194896523 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -187,7 +187,7 @@ members = [
   "sui-execution/v0/sui-verifier",
   "sui-execution/v1/sui-adapter",
   "sui-execution/v1/sui-move-natives",
-  "sui-execution/v1/sui-verifier",
+  "sui-execution/v1/sui-verifier", "my-dumb-service",
 ]
 
```

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
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.

2 participants