[agrawalkhushi18]

Summary

When --pathways-colocated-python-sidecar-image is set, gcluster starts the colocated Python sidecar in each worker pod but never told pathways-proxy to use it. Without --sidecar_name, the proxy doesn't set up the sidecar connection on the workers, so colocated Python work never reaches the sidecar.
This matches the Cloud Pathways team's sidecar/MTC configurations and XPK usage (confirmed with the Pathways team).

Changes

• Add --sidecar_name=external to the pathways-proxy args when the colocated Python sidecar is enabled (batch and headless). A --sidecar_name already passed via --pathways-proxy-args is kept and not duplicated.
• Update the --pathways-colocated-python-sidecar-image row in the job guide.

The flag is added through the existing proxy args list, so pathways_jobset.tmpl is unchanged and custom templates pick it up without re-extracting.

Compatibility

No new flags. Only jobs using --pathways-colocated-python-sidecar-image are affected,
and their sidecar is now actually used. Proxy images too old to recognize --sidecar_name
would reject the flag; current images support it.

Testing

• Unit tests for the helper (including single-dash and user-provided --sidecar_name)
  and for the generated manifest (batch, headless, no sidecar, user value via
  --pathways-proxy-args).
• E2E on a v6e 2x4 cluster with --pathways-colocated-python-sidecar-image: the proxy
  registers colocated_python (port=50051), and workers log "External sidecar connected
  to backing GRPC sidecar at localhost:50051".
• go test ./pkg/orchestrator/gke/... ./cmd/job/... passes.

Submission Checklist

NOTE: Community submissions can take up to 2 weeks to be reviewed.

Please take the following actions before submitting this pull request.

• Fork your PR branch from the Toolkit "develop" branch (not main)
• Test all changes with pre-commit in a local branch #
• Confirm that "make tests" passes all tests
• Add or modify unit tests to cover code changes
• Ensure that unit test coverage remains above 80%
• Update all applicable documentation
• Follow Cluster Toolkit Contribution guidelines #

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 ensures that when a colocated Python sidecar is enabled for Pathways jobs, the pathways-proxy is correctly configured to communicate with it. By automatically appending the necessary sidecar argument, it resolves an issue where colocated Python work was failing to reach the sidecar. The implementation includes robust checks to preserve any custom sidecar naming provided by the user and adds comprehensive unit tests to verify both the argument generation logic and the resulting manifest structure.

Highlights

• Automatic Sidecar Configuration: Automatically injects --sidecar_name=external into pathways-proxy arguments when a colocated Python sidecar image is specified, ensuring proper routing of colocated Python work.
• Argument Preservation: Implements logic to respect existing user-provided --sidecar_name arguments in --pathways-proxy-args, preventing duplication or overwriting of custom configurations.
• Documentation Update: Updated the job guide to reflect the new automated behavior of the --pathways-colocated-python-sidecar-image flag.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request automatically appends the --sidecar_name=external argument to the pathways-proxy container when the colocated Python sidecar is enabled, ensuring that Pathways routes colocated Python work to the sidecar. It preserves any user-defined --sidecar_name passed via --pathways-proxy-args to avoid duplication. The changes include updates to the documentation, implementation of the argument-appending logic in the GKE manifest generator, and comprehensive unit tests validating the behavior under various configurations. The implementation is clean, well-tested, and I have no additional feedback to provide.

[Neelabh94]

Nice work! The automatic injection of --sidecar_name=external aligns with upstream pathways-utils golden YAMLs (isc_pathways.py) and the Cloud Pathways proxy_server binary contract, ensuring colocated Python calls are properly routed to the worker sidecars without requiring manual proxy flag configuration.

A few recommendations to improve code simplicity, test coverage, and documentation consistency:

1. Code Simplification / YAGNI (pkg/orchestrator/gke/manifest_generator.go:684, 783):
   withSidecarNameArg has a single call site and currently takes a sidecarEnabled bool parameter just to return early when false. We can simplify both the helper signature and its unit tests by guarding directly at the call site:

   // pkg/orchestrator/gke/manifest_generator.go:684
   if opts.Pathways.ColocatedPythonSidecarImage != "" {
       proxyArgsList = withSidecarNameArg(proxyArgsList)
   }

   // pkg/orchestrator/gke/manifest_generator.go:783
   func withSidecarNameArg(proxyArgs []string) []string {
       for _, arg := range proxyArgs {
           name := strings.TrimLeft(arg, "-")
           if name == "sidecar_name" || strings.HasPrefix(name, "sidecar_name=") {
               return proxyArgs
           }
       }
       return append(proxyArgs, "--sidecar_name=external")
   }

2. Test Coverage Additions (pkg/orchestrator/gke/manifest_generator_test.go):

   • In TestWithSidecarNameArg, consider adding test cases for:
     • nil / empty slice (args: nil, args: []string{})
     • Empty assignment (--sidecar_name=)
     • Multiple unrelated flags ([]string{"--foo=1", "--bar=2"})
   • In TestGeneratePathwaysManifest_SidecarNameArg, add the negative headless case to verify the flag is omitted when no sidecar image is specified:

     {name: "headless no sidecar", headless: true, wantArg: false},

   • In TestGeneratePathwaysManifest_SidecarNameArgUserValue, test space-separated format (--sidecar_name custom) through --pathways-proxy-args to verify strings.Fields integration end-to-end.

3. CLI Help Text (cmd/job/submit.go:193):

   • The documentation in docs/gcluster_job_guide.md was updated, but the flag description in cmd/job/submit.go still has the older text. Consider updating the SubmitCmd.Flags().StringVar description to mention that --sidecar_name=external is automatically passed to pathways-proxy so gcluster job submit --help stays aligned with the guide.

[agrawalkhushi18]

Thanks! addressed in 8536e9a:

1. withSidecarNameArg now takes only the args; the call site checks the sidecar image.
2. Added the headless no-sidecar case, the space-separated --sidecar_name custom case through --pathways-proxy-args, and nil / --sidecar_name= cases. I skipped the empty-slice and two-unrelated-flags cases since they take the same path as nil and --foo=1.
3. Kept the --help text as is: --sidecar_name=external is internal proxy wiring that users don't act on (like the proxy/worker flags --pathways-elastic-slices sets), and kept the submit.go flag description precise and as is since the job guide already documents it along with the --pathways-proxy-args override.