Build docker bootstrap image#6
Conversation
gregcusack
force-pushed
the
build-bootstrap-image
branch
from
e8e31a3 to
fac42c4
Compare
gregcusack
force-pushed
the
build-bootstrap-image
branch
from
fac42c4 to
08f084a
Compare
gregcusack
force-pushed
the
build-bootstrap-image
branch
from
5592544 to
836d006
Compare
gregcusack
requested review from
joncinque and
yihau
and removed request for
joncinque
| FROM {} | ||
| RUN apt-get update | ||
| RUN apt-get install -y iputils-ping curl vim bzip2 | ||
|
|
||
| RUN useradd -ms /bin/bash solana | ||
| RUN adduser solana sudo | ||
| USER solana | ||
|
|
||
| RUN mkdir -p /home/solana/k8s-cluster-scripts | ||
| # TODO: this needs to be changed for non bootstrap, this should be ./src/scripts/<validator-type>-startup-scripts.sh | ||
| COPY {startup_script_directory}/bootstrap-startup-script.sh /home/solana/k8s-cluster-scripts | ||
|
|
||
| RUN mkdir -p /home/solana/ledger | ||
| COPY --chown=solana:solana ./config-k8s/bootstrap-validator /home/solana/ledger | ||
|
|
||
| RUN mkdir -p /home/solana/.cargo/bin | ||
|
|
||
| COPY ./{solana_build_directory}/bin/ /home/solana/.cargo/bin/ | ||
| COPY ./{solana_build_directory}/version.yml /home/solana/ | ||
|
|
||
| RUN mkdir -p /home/solana/config | ||
| ENV PATH="/home/solana/.cargo/bin:${{PATH}}" | ||
|
|
||
| WORKDIR /home/solana |
There was a problem hiding this comment.
nit: could consider to use less RUN command to reduce the image size.
| pub mod release; | ||
|
|
||
| static SUN: Emoji = Emoji("🌞 ", ""); | ||
| static BUILD: Emoji = Emoji("👷 ", ""); |
| let docker = DockerConfig::new( | ||
| matches | ||
| .value_of("base_image") | ||
| .unwrap_or_default() |
There was a problem hiding this comment.
nit: This should probably be an unwrap(), like all the others. Even though it won't get triggered thanks to default_value(...)
| matches.value_of("image_name").unwrap().to_string(), | ||
| matches | ||
| .value_of("image_tag") | ||
| .unwrap_or_default() |
There was a problem hiding this comment.
nit: Same here, should be an unwrap() instead
| pub fn build_path(&self) -> PathBuf { | ||
| self.build_path.clone() | ||
| } |
There was a problem hiding this comment.
oh shoot no not anymore.
| progress_bar.set_message(format!("{BUILD}Building {validator_type} docker image...",)); | ||
|
|
||
| let command = format!( | ||
| "docker build -t {}/{image_name}:{} -f {dockerfile:?} {context_path}", |
There was a problem hiding this comment.
Question on using {dockerfile:?} here: the documentation says to use the Debug formatter if you want to escape the path at https://doc.rust-lang.org/std/path/struct.PathBuf.html#method.display
Is using display() no good for your use-case?
There was a problem hiding this comment.
did not realize that. will update.
There was a problem hiding this comment.
Given that you're using the _lossy bits for Windows, and display "This may perform lossy conversion, depending on the platform", I'm more convinced that it's the better approach
| let manifest_path = | ||
| PathBuf::from(env::var("CARGO_MANIFEST_DIR").expect("$CARGO_MANIFEST_DIR")); |
There was a problem hiding this comment.
I just did a little test, and CARGO_MANIFEST_DIR is only set when invoking the program using cargo, and not just calling the executable directly.
Since this might be slightly brittle, would it make sense to also pass a path to the scripts directory?
There was a problem hiding this comment.
what is the best way to do this? Just add a flag that is something like: --startup-scripts-dir <path>? or were you thinking something else?
There was a problem hiding this comment.
we also use this here:
Lines 19 to 21 in 044c9d2
release-channel deployment method. So maybe we just pass in a --validator-lab-dir <absolute-path-to-validator-lab-directory>. From here we can get startup script directory and then of course the directory needed for release-channel deployment
3 participants
Summary of Changes
6th PR in a series of PRs that will build out the mongon testing framework for deploying validator clusters on Kubernetes
Previous PRs: