Conversation
|
Hi @Copilot. Thanks for your PR. I'm waiting for a kubermatic member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/close |
|
@KhizerRehan: Closed this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[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 |
There was a problem hiding this comment.
Pull request overview
This PR addresses several UX and spec-generation issues around configuring KubeLB in the cluster creation wizard and the edit-cluster dialog, and adds a provider-specific warning to the cluster summary.
Changes:
- Prevent KubeLB sub-options (
useLoadBalancerClass,enableGatewayAPI) from being sent astruewhen the main KubeLB toggle is disabled (wizard + edit). - Fix the “double tooltip” behavior on the KubeLB checkbox by disabling the enforced tooltip when not enforced and conditionally rendering the info icon.
- Add a cluster summary warning banner for AWS/Azure when KubeLB is enabled, reminding users about “Assign Public IP”.
- Update
_isKubeLBEnabled()logic to add datacenter-level explicit disable precedence (wizard + edit).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/web/src/app/wizard/step/cluster/template.html | Adjust KubeLB tooltip behavior and conditionally show info tooltip icon. |
| modules/web/src/app/wizard/step/cluster/component.ts | Reset KubeLB sub-options when disabled; update _isKubeLBEnabled() precedence logic. |
| modules/web/src/app/shared/components/cluster-summary/template.html | Render AWS/Azure warning banner when KubeLB is enabled. |
| modules/web/src/app/shared/components/cluster-summary/style.scss | Add styling for the new warning banner. |
| modules/web/src/app/shared/components/cluster-summary/component.ts | Add computed flag for showing the new warning banner. |
| modules/web/src/app/cluster/details/cluster/edit-cluster/template.html | Mirror tooltip behavior changes for KubeLB checkbox in edit dialog. |
| modules/web/src/app/cluster/details/cluster/edit-cluster/component.ts | Mirror sub-option reset + _isKubeLBEnabled() precedence changes in edit dialog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
modules/web/src/app/cluster/details/cluster/edit-cluster/component.ts
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7958 +/- ##
==========================================
- Coverage 45.19% 45.14% -0.06%
==========================================
Files 447 447
Lines 20831 20859 +28
Branches 2977 2999 +22
==========================================
+ Hits 9414 9416 +2
- Misses 11413 11437 +24
- Partials 4 6 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/test all |
6f87040 to
14d127a
Compare
|
/test all |
| // If KubeLB is explicitly configured at the datacenter level with enabled: false, respect that | ||
| if ( | ||
| datacenter.spec.kubelb && | ||
| (datacenter.spec.kubelb.enabled === undefined || datacenter.spec.kubelb.enabled === false) |
There was a problem hiding this comment.
@Waseem826 we needed undefined check because here this struct is receiving from backend side. Manually changing seed setting false e.g
hetzner-fsn1:
country: DE
location: Falkenstein 1 DC 14
spec:
hetzner:
...
kubelb:
enabled: false
is returning undefined when set to false or when property was not added completely and according to @shinushaju we need to hide Kubermatic KubeLB option when it was set to false.
Therefore this check was added as this is omitempty skips property when JSON serialization is done
https://github.com/kubermatic/kubermatic/blob/28dcb972592be71f9369cd55aa54b72ed2c9ee83/sdk/apis/kubermatic/v1/datacenter.go#L1348
However i tested it works find with undefined check but false is added as fallback in case it is fixed from backend side
Ref:
What this PR does / why we need it:
Fixes several KubeLB option issues in the cluster wizard and edit-cluster dialog:
Fixes double tooltip on KubeLB checkbox — when KubeLB is enforced, only the enforced tooltip shows; when not enforced, only the info icon tooltip shows. Uses
[matTooltipDisabled]and conditional rendering of the info icon.Hides Kubermatic KubeLB option when
enabled: falseAdds a warning message on the cluster summary page for AWS/Azure providers when KubeLB is enabled, reminding users to ensure "Assign Public IP" is selected in node settings.
Which issue(s) this PR fixes:
Fixes #7902
What type of PR is this?
/kind bug
Does this PR introduce a user-facing change? Then add your Release Note here:
Documentation: