Skip to content

fix panic on repeated IsReachable calls - #32184

Merged
TerryHowe merged 1 commit into
helm:mainfrom
atkrad:fix/kube-isreachable-nil-panic
Jun 18, 2026
Merged

fix panic on repeated IsReachable calls#32184
TerryHowe merged 1 commit into
helm:mainfrom
atkrad:fix/kube-isreachable-nil-panic

Conversation

@atkrad

@atkrad atkrad commented Jun 4, 2026

Copy link
Copy Markdown
Contributor
  • only cache kube client when creation succeeds
  • add regression test for two failed reachability checks

Closes #32183

Copilot AI review requested due to automatic review settings June 4, 2026 22:26
@pull-request-size pull-request-size Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR hardens Kubernetes client creation/caching to avoid invalid cached state after a clientset creation failure, and adds a regression test to ensure IsReachable() behaves safely across repeated calls.

Changes:

  • Update getKubeClient() to only cache the client after successful factory creation.
  • Add a regression test that calls IsReachable() twice after a forced client creation error.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pkg/kube/client.go Avoids caching kubeClient when factory creation returns an error.
pkg/kube/client_test.go Adds a regression test for repeated IsReachable() calls after client creation failure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/kube/client_test.go
@benoittgt

Copy link
Copy Markdown
Contributor

I can't approve your work but I think it is a good fix.

TerryHowe
TerryHowe previously approved these changes Jun 11, 2026

@TerryHowe TerryHowe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@TerryHowe TerryHowe added the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Jun 11, 2026
@scottrigby

Copy link
Copy Markdown
Member

just approved workflows to run

@scottrigby scottrigby added the bug Categorizes issue or PR as related to a bug. label Jun 12, 2026
@scottrigby

Copy link
Copy Markdown
Member

CI succeeded.

As I said in #32183 (comment) the Copilot comment is actually a good point in this case. Should be an easy update.

@atkrad
atkrad force-pushed the fix/kube-isreachable-nil-panic branch from 446aedc to d46dbaa Compare June 12, 2026 09:19
@pull-request-size pull-request-size Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 12, 2026
@atkrad
atkrad requested a review from Copilot June 12, 2026 09:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread pkg/kube/client_test.go Outdated
@atkrad
atkrad force-pushed the fix/kube-isreachable-nil-panic branch from d46dbaa to b82bbd5 Compare June 12, 2026 09:31
@atkrad
atkrad requested a review from Copilot June 12, 2026 09:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread pkg/kube/client_test.go Outdated
- only cache kube client when creation succeeds
- add regression test for two failed reachability checks

Signed-off-by: Mohammad Abdolirad <m.abdolirad@gmail.com>
@atkrad
atkrad force-pushed the fix/kube-isreachable-nil-panic branch from b82bbd5 to 1f7869c Compare June 12, 2026 09:34
@atkrad
atkrad requested a review from Copilot June 12, 2026 09:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread pkg/kube/client.go
Comment thread pkg/kube/client.go
@atkrad
atkrad requested a review from TerryHowe June 12, 2026 20:50

@TerryHowe TerryHowe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@TerryHowe
TerryHowe merged commit 05b2746 into helm:main Jun 18, 2026
5 checks passed
@scottrigby scottrigby removed the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Aug 12, 2026
@scottrigby scottrigby added this to the 4.2.4 milestone Aug 12, 2026
@scottrigby scottrigby added needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. and removed needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. labels Aug 12, 2026
@scottrigby scottrigby added the picked Indicates that a PR has been cherry-picked into the next release candidate. label Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Categorizes issue or PR as related to a bug. picked Indicates that a PR has been cherry-picked into the next release candidate. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kube: IsReachable panics on second call when client creation fails (typed-nil cached in getKubeClient)

6 participants