fix(enterprise): return the advisor console pool from cluster-info - #113
Merged
Merged
Conversation
cluster-info serialises an inline struct, so SuggestedAdvisorPool -- added to ClusterQuery when the pool probe landed -- was computed by Introspect on every call and then dropped on the floor. The install form could never offer a pool, and no caller could learn one without running the probe itself, which is the whole thing the probe exists to spare them. Caught by asking a real cluster for its pool and getting a response with no such key. Fixes #105 follow-up Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZ5umjjCedZwWtbAbiMjfj Signed-off-by: Travis Wu <travis.wu@bigstack.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
What this PR does / why we need it
cluster-infoserialises an inline anonymous struct.SuggestedAdvisorPoolwas added toClusterQuerywhen the pool probe landed (#112) but never to that struct, soIntrospectprobed the cluster for free addresses on every call and the result was dropped on the floor:{"projects":[…],"suggestedLBIP":"10.32.1.110","suggestedStorage":"CubeStorage","version":"3.1.20", …}No
suggestedAdvisorPoolkey at all. The install form could never offer a pool, and no caller could learn one without running the probe itself — which is exactly what the probe exists to spare them. #112's description claimed the form was offered the whole pool; it was not.Which issue(s) this PR fixes
Follow-up to #105 / #112.
Special notes for your reviewer
How it was caught: by asking the 1cc r630 for its pool through the API after deploying the new driver, and getting a response with no such key. Unit tests did not catch it because nothing asserted on the response shape — the field existed on the struct the manager returns, and every test that touched the pool went through
IntrospectorBuildPlandirectly. The new test asserts the key is present in the JSON, which is the only place the bug could live.Still missing, deliberately out of scope: the web UI does not yet read
suggestedAdvisorPoolor sendAdvisorPoolin the install request. An API caller can now do both; the form cannot. That wiring is UI work and wants its own change.Additional documentation