Simplify topology validation for NAP enabled clusters#5964
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
f1acb8f to
de40d5c
Compare
|
/gemini review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
e58eee5 to
74753cd
Compare
|
/gemini review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
74753cd to
de16ef2
Compare
|
/gemini review |
Neelabh94
added
the
release-bugfix
There was a problem hiding this comment.
Code Review
This pull request updates the validateRequestedTopology function in gke_job_orchestrator.go to include a check for Node Auto-Provisioning (NAP) status, allowing requested topologies that differ from discovered limits when NAP is enabled. The reviewer suggested a refactoring of the validation loop to improve code conciseness and idiomatic flow by returning early upon finding a valid topology, thereby eliminating the need for a boolean flag.
de16ef2 to
a0dc7ea
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the validateRequestedTopology function in gke_job_orchestrator.go to improve flow and support Node Auto-Provisioning (NAP). The logic now prioritizes direct topology validation, followed by a NAP-enabled check, before falling back to containment checks. I have reviewed the provided feedback and recommend sorting the list of valid topologies in the error message to ensure deterministic output, as map iteration order in Go is non-deterministic.
There was a problem hiding this comment.
Code Review
This pull request updates the validateRequestedTopology function in gke_job_orchestrator.go to support Node Auto-Provisioning (NAP) by allowing requested topologies that differ from currently discovered cluster limits. The reviewer suggests an improvement to include an early format validation check for the requested topology string, even when NAP is enabled, to catch malformed inputs before they reach the node pool creation stage.
a0dc7ea to
564a2d5
Compare
2 participants
This pull request updates the validateRequestedTopology function in gke_job_orchestrator.go to include a check for Node Auto-Provisioning (NAP) status, allowing requested topologies that differ from discovered limits when NAP is enabled.