Skip to content

Conversation

@dkoshkin
Copy link
Contributor

@dkoshkin dkoshkin commented Nov 1, 2025

What problem does this PR solve?:
Fixes a panic in the addon webhook and handles the case when creating a ClusterClass workload cluster in a non ClusterClass management cluster.

Which issue(s) this PR fixes:
Fixes #

How Has This Been Tested?:

Special notes for your reviewer:

@dkoshkin dkoshkin requested a review from jimmidyson November 1, 2025 18:13
@github-actions github-actions bot added the fix label Nov 1, 2025
@bish-ntx
Copy link

bish-ntx commented Nov 1, 2025

Ticket: https://jira.nutanix.com/browse/NCN-110783 - [NKP 2.17.0-dev4] EKS Cluster deploy is erroring out: webhook panic when management cluster has nil topology

Copy link
Member

@jimmidyson jimmidyson left a comment

Choose a reason for hiding this comment

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

Let's add match conditions to the webhook config too to prevent non cc clusters being sent to this webhook at all (keeping the code you've added on this pr as well).

@dkoshkin
Copy link
Contributor Author

dkoshkin commented Nov 3, 2025

Let's add match conditions to the webhook config too to prevent non cc clusters being sent to this webhook at all (keeping the code you've added on this pr as well).

Good idea, done in #1375

@jimmidyson jimmidyson merged commit 6b35c6d into main Nov 3, 2025
37 of 40 checks passed
@jimmidyson jimmidyson deleted the dkoshkin/fix-addon-enabler-npe branch November 3, 2025 18:31
@github-actions github-actions bot mentioned this pull request Nov 3, 2025
dkoshkin added a commit that referenced this pull request Nov 4, 2025
…opology (#1381)

**What problem does this PR solve?**:
Backport
#1372

Fixes a panic in the addon webhook and handles the case when creating a
ClusterClass workload cluster in a non ClusterClass management cluster.

**Which issue(s) this PR fixes**:
Fixes #

**How Has This Been Tested?**:
<!--
Please describe the tests that you ran to verify your changes.
Provide output from the tests and any manual steps needed to replicate
the tests.
-->

**Special notes for your reviewer**:
<!--
Use this to provide any additional information to the reviewers.
This may include:
- Best way to review the PR.
- Where the author wants the most review attention on.
- etc.
-->
dkoshkin added a commit that referenced this pull request Nov 5, 2025
**What problem does this PR solve?**:
Recently found a bug
#1372
This adds another level of safety and skips unnecessarily calling this
webhook for clusters not using topology.

**Which issue(s) this PR fixes**:
Fixes #

**How Has This Been Tested?**:
<!--
Please describe the tests that you ran to verify your changes.
Provide output from the tests and any manual steps needed to replicate
the tests.
-->

**Special notes for your reviewer**:
<!--
Use this to provide any additional information to the reviewers.
This may include:
- Best way to review the PR.
- Where the author wants the most review attention on.
- etc.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants