Adding support for config file and environment variables - #35
Merged
Conversation
Six correctness fixes ahead of tig-cli becoming the only entry point: - Run the container as the invoking uid/gid on Linux so output files are owned by the user instead of root. - Pass -i to docker exec (and -t only when interactive) so tools that read stdin work and pipes are not silently swallowed. - Translate paths inside VICAR keyword=value arguments and parenthesized value lists; non-path values such as SIZE=(1,1,500,500) are untouched. - Mount the current directory read-write so working outside $HOME does not fail with 'Read-only file system'. - Remove the container on SIGINT/SIGTERM/SIGHUP, force-remove on exit, and reap a stale container of the same name before reuse. - Force platform linux/amd64 (image is amd64-only) and report Docker failures as one-line errors instead of tracebacks. Adds --version. Co-Authored-By: jason.han <jason.han@jpl.nasa.gov>
…iles Replaces the container-per-command lifecycle with a reusable container, and adds the calibration mounting the native toolkit had, so tig-cli can cover the terrain workflow on its own. - ensure_container() reuses a container whose name is a digest of the run configuration, starts it if stopped, and replaces it when the image tag has moved. --status lists them, --shutdown removes them. - DISPLAY moves from the container environment to each docker exec, so switching displays does not need a new container. - MARS_CONFIG_PATH / --calibration-path mounts calibration data read-only at /usr/local/vicar/mars_calib and exports it inside the container. - Signals now stop the running command rather than the shared container. - The integration test invoking python3 used whichever interpreter was on PATH; use sys.executable. Co-Authored-By: jason.han <jason.han@jpl.nasa.gov>
…tarts Both from review of #2, and both reproduced: - signal_command waited on a child the main flow is already waiting on; Popen.wait() holds the waitpid lock, so the nested wait could never observe the exit and every Ctrl-C stalled for the full 10s timeout (measured 10.02s, now 0.03s). Send the signal and return. - Concurrent invocations share the configuration-derived name, so one of six parallel runs lost the create race and aborted with a 409 conflict. ensure_container now retries the get-or-create path, adopting whichever container won, and tolerates a container removed concurrently. 30/30 parallel runs now succeed against one container. Co-Authored-By: jason.han <jason.han@jpl.nasa.gov>
…ent-container * feat(tig-cli): reuse a persistent container, mount MARS calibration files Replaces the container-per-command lifecycle with a reusable container, and adds the calibration mounting the native toolkit had, so tig-cli can cover the terrain workflow on its own. - ensure_container() reuses a container whose name is a digest of the run configuration, starts it if stopped, and replaces it when the image tag has moved. --status lists them, --shutdown removes them. - DISPLAY moves from the container environment to each docker exec, so switching displays does not need a new container. - MARS_CONFIG_PATH / --calibration-path mounts calibration data read-only at /usr/local/vicar/mars_calib and exports it inside the container. - Signals now stop the running command rather than the shared container. - The integration test invoking python3 used whichever interpreter was on PATH; use sys.executable. Co-Authored-By: jason.han <jason.han@jpl.nasa.gov> * fix(tig-cli): don't stall on interrupt, don't collide on concurrent starts Both from review of #2, and both reproduced: - signal_command waited on a child the main flow is already waiting on; Popen.wait() holds the waitpid lock, so the nested wait could never observe the exit and every Ctrl-C stalled for the full 10s timeout (measured 10.02s, now 0.03s). Send the signal and return. - Concurrent invocations share the configuration-derived name, so one of six parallel runs lost the create race and aborted with a 409 conflict. ensure_container now retries the get-or-create path, adopting whichever container won, and tolerates a container removed concurrently. 30/30 parallel runs now succeed against one container. Co-Authored-By: jason.han <jason.han@jpl.nasa.gov> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: jason.han <jason.han@jpl.nasa.gov>
Co-Authored-By: jason.han <jason.han@jpl.nasa.gov>
Co-Authored-By: jason.han <jason.han@jpl.nasa.gov>
Co-Authored-By: jason.han <jason.han@jpl.nasa.gov>
…file * feat(tig-cli): layered TOML config files with env var overrides Co-Authored-By: jason.han <jason.han@jpl.nasa.gov> * feat(tig-cli): mars_config_path for MARS calibration data Co-Authored-By: jason.han <jason.han@jpl.nasa.gov> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: jason.han <jason.han@jpl.nasa.gov>
Adds the three host-environment behaviors the vicar-native-toolkit had: - SELinux: detect Enforcing at runtime and run the container with --security-opt label=disable, overridable via --selinux-label-disable, TIG_SELINUX_LABEL_DISABLE and selinux_label_disable. No mount is ever relabeled (:z/:Z), since tig mounts the host root filesystem. - X11: authorize the host display when a container is created (xhost +local: on Linux, XQuartz TCP + xhost +localhost on macOS), skipped silently without DISPLAY or xhost. Drops the dead XFILESEARCHPATH and XBMLANGPATH exec env vars, which pointed at a directory absent from the image. - Container proliferation: keep at most two containers, reaping older ones when a new one is created. Claim files keyed by PID mark the container each live invocation uses, so a concurrent tig is never reaped out from under; tig --status now also shows each container's writable mounts. Co-Authored-By: jason.han <jason.han@jpl.nasa.gov>
…-x11-container-reaping Adds the three host-environment behaviors the vicar-native-toolkit had: - SELinux: detect Enforcing at runtime and run the container with --security-opt label=disable, overridable via --selinux-label-disable, TIG_SELINUX_LABEL_DISABLE and selinux_label_disable. No mount is ever relabeled (:z/:Z), since tig mounts the host root filesystem. - X11: authorize the host display when a container is created (xhost +local: on Linux, XQuartz TCP + xhost +localhost on macOS), skipped silently without DISPLAY or xhost. Drops the dead XFILESEARCHPATH and XBMLANGPATH exec env vars, which pointed at a directory absent from the image. - Container proliferation: keep at most two containers, reaping older ones when a new one is created. Claim files keyed by PID mark the container each live invocation uses, so a concurrent tig is never reaped out from under; tig --status now also shows each container's writable mounts. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: jason.han <jason.han@jpl.nasa.gov>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Also addresses exit code passing behavior from container to frontend.