fix: kmeshctl version exits 0 even when pod version fetch fails - #1872
bhumikadangayach wants to merge 3 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
Hi @LiZhenCheng9527 — this has been open a few weeks, CI is green, no outstanding comments. Could you take a look when you get a chance? |
|
|
||
| if err := json.Unmarshal(body, &version); err != nil { | ||
| log.Errorf("failed to unmarshal version info: %v", err) | ||
| if uerr := json.Unmarshal(body, &version); uerr != nil { |
There was a problem hiding this comment.
Even if the JSON is parsed correctly, there is no guarantee that the version data is valid.
86abf32 to
dd47b30
Compare
Signed-off-by: Bhumika Dangayach <139267865+bhumikadangayach@users.noreply.github.com>
2e7f159 to
2bb213b
Compare
3 participants
Fixes #1871
kmeshctl version was exiting 0 even when the version fetch failed. getVersion() only returned version.Info with no error, so callers had no way to detect failure - they just checked if v.GitVersion was empty, which is indistinguishable from a real failure.
Added an error return to getVersion(), set it at every failure point inside the function, and updated both call sites in runVersion() to check it: the single-pod case now exits 1 on failure (matching the existing len(args) == 0 branch's behavior), and the multi-pod loop skips a pod on failure instead of silently counting it as empty.
Verified: kmeshctl version now exits 1 instead of 0.