Skip to content

[DO NOT MERGE] Copy of #2735 - #2771

Draft
Xiaoyu Z (xiaoyu-work) wants to merge 2 commits into
mainfrom
docker-readonly-input-mounts
Draft

Xiaoyu Z (xiaoyu-work) wants to merge 2 commits into
mainfrom
docker-readonly-input-mounts

Conversation

@xiaoyu-work

Copy link
Copy Markdown
Member

Describe your changes

Independent copy of #2735: Mount Docker system input resources read-only.

Copies Iyanuoluwa007/Olive:docker-readonly-input-mounts to the same branch name in microsoft/Olive, preserving the original commits. The original PR remains unchanged.

Validation: the copied branch head matches the source PR head. Tests and lintrunner -a were not run for this copy-only operation; the existing code has not been revalidated.

Checklist before requesting a review

  • Add unit tests for this change.
  • Make sure all tests can pass.
  • Update documents if necessary.
  • Lint and apply fixes to your code by running lintrunner -a
  • Is this a user-facing change? If yes, give a description of this change to be included in the release notes.

(Optional) Issue link

Source PR: #2735.

Workflow resources found in the run config were all mounted into the
container read-write. Input models, data files and user scripts only need
to be read, so they are now mounted with `:ro`. Only `engine.output_dir`
and the engine cache directory stay writable. The temporary config.json and
the runner script mounts are read-only as well.

Also document that `hf_token: true` passes the full HuggingFace token into
the container environment for the Docker system.

Fixes #2675
- Match writable resources by config field name (output_dir, cache_dir,
  logging_dir, sample_log_dir) instead of only engine keys, so existing
  LoRA/QLoRA training output dirs and evaluator sample log dirs are not
  mounted read-only.
- An input that lives inside a writable directory (for example under the
  default output dir, which is the working directory) was still writable
  through that directory's mount. Bind it read-only at its nested location
  as well so the read-only mount shadows the writable view.

This branch has not been deployed

No deployments
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