feat(job): Enhance mount flag to pass additional mount options for gcsfuse mounts#6005
feat(job): Enhance mount flag to pass additional mount options for gcsfuse mounts#6005Neelabh94 wants to merge 8 commits into
Conversation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new --mount-options flag to the gcluster job submit command, enabling custom mount options for GCS fuse volumes, and updates the orchestrator, GKE storage manager, and documentation accordingly. The review feedback suggests enhancing validation to fail-fast if --mount-options is specified without any mounts, and formatting struct fields in pkg/orchestrator/orchestrator.go to comply with standard Go alignment.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new --mount-options flag to the gcluster job submit command, enabling custom mount options for GCS fuse volumes, and updates the relevant documentation. Feedback suggests adding a validation check to ensure --mount-options is not silently ignored when no mounts are specified, and formatting the JobDefinition struct to align the new field according to standard Go guidelines.
… options for GCS fuse volumes
…idation and update docs
cf0212d to
9decd22
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the volume mount format from using colons to semicolons (<src>;<dest>[;<mode>][;options=<options>]) to support custom mount options, particularly for GCS fuse volumes. The changes span the CLI flags, documentation, orchestrator storage manager, and associated tests. Feedback points out a critical issue where using StringSliceVar for the --mount flag will incorrectly split comma-separated mount options, and suggests switching to StringArrayVar. Additionally, recommendations are made to simplify the MountInfo initialization in storage.go and to add a test case verifying the parsing of multiple comma-separated options.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the --mount flag format from colon-separated (:) to semicolon-separated (;) and adds support for custom mount options (e.g., options=<options>) specifically for GCS fuse volumes. The changes include updates to CLI flags, volume parsing logic, Kubernetes manifest generation, documentation, and unit tests. I have no feedback to provide as the implementation is robust and complete.
9a2bbb3 to
c5c7732
Compare
Description
This PR adds support for passing custom mount options when submitting a job, bringing
gclusterinto closer parity withxpkbehavior.Key Changes:
optionskey to themountflag: Users can now pass custom storage options (e.g.,logging:severity:info,enable-atomic-rename-object:true) togcluster job submit.optionskey is currently only utilized for GCS fuse volumes (i.e. it enforces that all supplied--mountvalues start withgs://when options are present).docs/gcluster_job_guide.md.