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

feat(cli): run containerised jstzd with 'sandbox start' #772

Draft
wants to merge 1 commit into
base: huanchengchang-jstz-274
Choose a base branch
from

Conversation

huancheng-trili
Copy link
Collaborator

@huancheng-trili huancheng-trili commented Jan 16, 2025

Context

Completes JSTZ-274.
JSTZ-274

Description

Enable users to run containerised jstzd with jstz sandbox --container start.

Users can choose to launch the sandbox running containerised jstzd with a newly introduced flag --container. All sandbox commands with this flag operates on the container.

  • jstz sandbox --container start: launch a new container running a pre-defined image with jstzd inside. If the image does not exist, the program then tries to pull that from the repository. This command sets some default config values so that the CLI can interact with the octez node and the jstz node inside the container later on. Note that this command generates some temporary files. They are not going to be cleaned up by the CLI after the sandbox is removed and garbage collection is left to the host OS. It's possible to implement something for this, but I'm not going to do it at the moment just to save some time.
  • jstz sandbox --container stop: kill the running sandbox container if there is any. This command also cleans up the CLI sandbox config stored locally. I'm not sure why in the old sandbox the sandbox config is not removed when a sandbox is torn down, but I think it makes more sense to clean up the config because every sandbox is new.
  • jstz sandbox --container restart: restart the sandbox if it exists. Note that this does not raise an error if the sandbox is not running. This command simply prints a reminder message and won't start the sandbox.

Note that the module container might look a bit awkward. I have to move testable but trivial logic out of the main handlers so that at least we get some test coverage when integration tests are not available.

Manually testing the PR

  • Manual test: built the CLI and jstzd image locally and ran the CLI
  • Unit test: added some test cases
$ cargo test sandbox
running 6 tests                                                                            
test sandbox::container::tests::create_mounts ... ok
test sandbox::container::tests::new_create_container_options ... ok
test sandbox::container::tests::create_port_bindings ... ok     
test sandbox::container::tests::new_create_container_config ... ok   
test sandbox::container::tests::create_exposed_ports ... ok  
test sandbox::container::tests::create_config_file_and_client_dir ... ok           
 
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 9 filtered out; finished in 0.01s

@huancheng-trili huancheng-trili force-pushed the huanchengchang-jstz-274-1 branch 5 times, most recently from 8cfa815 to dbd91d9 Compare January 16, 2025 17:15
Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 51.13350% with 194 lines in your changes missing coverage. Please review.

Project coverage is 48.42%. Comparing base (2c6aa02) to head (fa71c86).

Files with missing lines Patch % Lines
crates/jstz_cli/src/sandbox/container.rs 54.56% 164 Missing and 5 partials ⚠️
crates/jstz_cli/src/sandbox/mod.rs 0.00% 22 Missing ⚠️
crates/jstz_cli/src/main.rs 0.00% 3 Missing ⚠️
Files with missing lines Coverage Δ
crates/jstz_cli/src/main.rs 0.00% <0.00%> (ø)
crates/jstz_cli/src/sandbox/mod.rs 0.00% <0.00%> (ø)
crates/jstz_cli/src/sandbox/container.rs 54.56% <54.56%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c6aa02...fa71c86. Read the comment docs.

@huancheng-trili huancheng-trili force-pushed the huanchengchang-jstz-274-1 branch from dbd91d9 to fa71c86 Compare January 16, 2025 19:53
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.

1 participant