Fix(gke): Support shared reservations in job submission#5826
Conversation
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 enables support for GKE shared reservations by ensuring the correct reservation owner project is identified and included in job manifests. By extracting the project ID from the reservation URI and injecting it into the nodeSelector, the orchestrator ensures that jobs can correctly consume reservations owned by different projects. 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 support for shared GKE reservations by extracting the reservation owner project from the reservation resource URI and adding it as a node selector label (cloud.google.com/reservation-owner-project) in the generated GKE manifest. It includes the helper function extractReservationOwnerProject in nap.go, along with comprehensive unit tests in nap_test.go and gke_job_orchestrator_test.go to verify the extraction logic and manifest generation. I have no feedback to provide.
76fe006 to
7752889
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for extracting and configuring shared reservation owner projects and subblocks in GKE node selectors, complete with comprehensive unit tests. The feedback highlights a critical correction: the GKE node selector label key for the owner project of a shared reservation must be updated from 'cloud.google.com/reservation-project' to 'cloud.google.com/reservation-owner-project' in both the orchestrator implementation and the corresponding unit tests to ensure GKE correctly identifies and consumes the shared reservation.
Extract and inject the reservation owner project when submitting a job with a shared reservation. GKE requires 'cloud.google.com/reservation-owner-project' to be specified in the nodeSelector to correctly consume shared reservations owned by a different project. This CL extracts the owner project from the reservation URI (if provided) and injects it into the generated manifest's nodeSelector.
7752889 to
94755ac
Compare
94755ac to
e630bbd
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors reservation URI parsing to extract and inject GKE reservation labels (such as reservation name, owner project, blocks, and subblocks) into the node selector of GKE jobs, accompanied by new unit tests. The reviewer identified that the label key used for the reservation owner project is incorrect and should be updated from cloud.google.com/reservation-project to cloud.google.com/reservation-owner-project in both the implementation and the corresponding tests to prevent scheduling failures.
Extract and inject the reservation owner project when submitting a job with a shared reservation.
GKE requires 'cloud.google.com/reservation-owner-project' to be specified in the nodeSelector to correctly consume shared reservations owned by a different project. This CL extracts the owner project from the reservation URI (if provided) and injects it into the generated manifest's nodeSelector.