fix: allow disabling kube-vip Service VIP management via svc_enable - #3196
Merged
Conversation
Motivation: The generated kube-vip static pod manifest hardcoded the svc_enable env var to "true" with no way to override it. This forces kube-vip to also manage Service (type LoadBalancer) VIPs, which conflicts with setups that run a separate Service VIP manager (e.g. MetalLB) alongside kube-vip for the apiserver VIP: kube-vip and MetalLB end up racing to manage the same VIP. Approach: Add a kubernetes.control_plane_endpoint.kube_vip.svc_enable config field (default true, preserving current behavior) and template it into the svc_enable env var in both the ARP and BGP kube-vip manifests instead of hardcoding "true". Documented the new field in docs/en and docs/zh config references, which mirror this defaults block. Validation: - go build ./... and go build -tags builtin ./... both pass. - Added a temporary test in pkg/converter/tmpl (removed before this commit, not part of the diff) that rendered both builtin/core/roles/kubernetes/pre-kubernetes/templates/kubevip/kubevip.ARP and .../kubevip.BGP through the real tmpl.Parse engine with svc_enable set to both true and false, asserted the output is valid YAML, and asserted the rendered svc_enable env var line matches "true"/"false" respectively. Both templates passed for both values, reproducing the reported hardcoded-true behavior before the fix and confirming the config knob works after it. - This repo has no existing automated test coverage for builtin YAML/template content and no live-cluster/e2e harness available in this environment; no user-facing runtime behavior changes for existing configs since the new field defaults to true. Report: kubesphere#3170 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
kubesphere-prow
Bot
added
the
size/S
Contributor
|
@pujitha24 I think we could extract the env parameters so more configuration options become available. Add the following to kubernetes:
control_plane_endpoint:
kube_vip:
env:
svc_enable: "false"
...In the template |
Per review feedback, generalize the single svc_enable field into kubernetes.control_plane_endpoint.kube_vip.env, mirroring the existing etcd.env pattern, so more kube-vip env vars can be overridden. The ARP and BGP manifest templates now reference env.<key> for each value they need instead of hardcoding it; host-loop-computed values (vip_interface, bgp_routerid, bgp_peers) and address are unchanged. Adds a template rendering test for both manifests. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
|
This PR has multiple commits, and the default merge method is: squash. 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. |
kubesphere-prow
Bot
added
size/L
redscholar
reviewed
Aug 18, 2026
Comment on lines
+120
to
+126
| bgp_enable: "true" | ||
| bgp_as: "65000" | ||
| bgp_peeraddress: "" | ||
| bgp_peerpass: "" | ||
| bgp_peeras: "65000" | ||
| lb_fwdmethod: local | ||
| prometheus_server: ":2112" |
| value: "{{ .kubernetes.control_plane_endpoint.kube_vip.env.port }}" | ||
| - name: vip_interface | ||
| value: {{ .kube_vip_interface }} | ||
| - name: vip_cidr |
Contributor
There was a problem hiding this comment.
use template to add env like:
{{ range $k,$v := .kubernetes.control_plane_endpoint.kube_vip.env }}
- name: {{ $k }}
value: {{ $v }}
{{ end }}
… templates Per review: the ARP/BGP-only env vars (vip_leaderelection group for ARP; bgp_*/lb_fwdmethod/prometheus_server for BGP) are removed from the shared kube_vip.env defaults and hardcoded back into their respective template, matching pre-PR behavior. Both templates now render the remaining shared vars via a range loop over kube_vip.env instead of listing each key individually, mirroring the range pattern already used in kubelet.env. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
|
redscholar
added
lgtm
Contributor
|
/lgtm |
|
LGTM label has been added. DetailsGit tree hash: ef2239cad6c41f55e3d04ff5dae55fc9e13c3e96 |
redscholar
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: pujitha24 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
redscholar
pushed a commit
to redscholar/kubekey
that referenced
this pull request
Aug 19, 2026
…ubesphere#3196) * fix: allow disabling kube-vip Service VIP management via svc_enable Motivation: The generated kube-vip static pod manifest hardcoded the svc_enable env var to "true" with no way to override it. This forces kube-vip to also manage Service (type LoadBalancer) VIPs, which conflicts with setups that run a separate Service VIP manager (e.g. MetalLB) alongside kube-vip for the apiserver VIP: kube-vip and MetalLB end up racing to manage the same VIP. Approach: Add a kubernetes.control_plane_endpoint.kube_vip.svc_enable config field (default true, preserving current behavior) and template it into the svc_enable env var in both the ARP and BGP kube-vip manifests instead of hardcoding "true". Documented the new field in docs/en and docs/zh config references, which mirror this defaults block. Validation: - go build ./... and go build -tags builtin ./... both pass. - Added a temporary test in pkg/converter/tmpl (removed before this commit, not part of the diff) that rendered both builtin/core/roles/kubernetes/pre-kubernetes/templates/kubevip/kubevip.ARP and .../kubevip.BGP through the real tmpl.Parse engine with svc_enable set to both true and false, asserted the output is valid YAML, and asserted the rendered svc_enable env var line matches "true"/"false" respectively. Both templates passed for both values, reproducing the reported hardcoded-true behavior before the fix and confirming the config knob works after it. - This repo has no existing automated test coverage for builtin YAML/template content and no live-cluster/e2e harness available in this environment; no user-facing runtime behavior changes for existing configs since the new field defaults to true. Report: kubesphere#3170 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> * refactor: extract kube-vip env vars into a configurable env map Per review feedback, generalize the single svc_enable field into kubernetes.control_plane_endpoint.kube_vip.env, mirroring the existing etcd.env pattern, so more kube-vip env vars can be overridden. The ARP and BGP manifest templates now reference env.<key> for each value they need instead of hardcoding it; host-loop-computed values (vip_interface, bgp_routerid, bgp_peers) and address are unchanged. Adds a template rendering test for both manifests. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> * refactor: trim kube-vip env map to shared vars, keep mode-specific in templates Per review: the ARP/BGP-only env vars (vip_leaderelection group for ARP; bgp_*/lb_fwdmethod/prometheus_server for BGP) are removed from the shared kube_vip.env defaults and hardcoded back into their respective template, matching pre-PR behavior. Both templates now render the remaining shared vars via a range loop over kube_vip.env instead of listing each key individually, mirroring the range pattern already used in kubelet.env. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --------- Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Co-authored-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
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
2 participants
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.
Now I have the full diff details to fill the template accurately.
What type of PR is this?
/kind bug
What does this PR do
Allow disabling kube-vip Service (type LoadBalancer) VIP management via a new
svc_enableconfig field.Background / Motivation
The generated kube-vip static pod manifest hardcoded the
svc_enableenv var to"true"with no way to override it. This forces kube-vip to also manage Service (type LoadBalancer) VIPs, which conflicts with setups that run a separate Service VIP manager (e.g. MetalLB) alongside kube-vip for the apiserver VIP: kube-vip and MetalLB end up racing to manage the same VIP.Implementation
Added a
kubernetes.control_plane_endpoint.kube_vip.svc_enableconfig field (defaulttrue, preserving current behavior) and templated it into thesvc_enableenv var in both the ARP and BGP kube-vip manifests instead of hardcoding"true". Documented the new field indocs/enanddocs/zhconfig references, which mirror this defaults block.Key Changes
builtin/core/roles/defaults/defaults/main/03-kubernetes.yamlkubernetes.control_plane_endpoint.kube_vip.svc_enable: truedefault fieldbuiltin/core/roles/kubernetes/pre-kubernetes/templates/kubevip/kubevip.ARPsvc_enableenv var from hardcoded"true"to"{{ .kubernetes.control_plane_endpoint.kube_vip.svc_enable }}"builtin/core/roles/kubernetes/pre-kubernetes/templates/kubevip/kubevip.BGPsvc_enableenv var from hardcoded"true"to"{{ .kubernetes.control_plane_endpoint.kube_vip.svc_enable }}"docs/en/reference/config.mdsvc_enablefield in example config and reference tabledocs/zh/reference/config.mdsvc_enablefield in example config and reference table (Chinese)Impact
builtin/core/roles/kubernetes/pre-kubernetes), defaults configkubernetes.control_plane_endpoint.kube_vip.svc_enable(defaulttrue)Breaking Changes
None
Which issue(s) this PR fixes:
Fixes #3170
Testing
Verification performed
go build ./...andgo build -tags builtin ./...)<command>)Steps to verify
kubernetes.control_plane_endpoint.kube_vip.svc_enable: falsein the cluster config.svc_enableenv var value is"false"instead of the previously hardcoded"true".Test coverage
A temporary test was added in
pkg/converter/tmpl(removed before this commit, not part of the diff) that rendered bothkubevip.ARPandkubevip.BGPthrough the realtmpl.Parseengine withsvc_enableset to bothtrueandfalse, asserted the output is valid YAML, and asserted the renderedsvc_enableenv var line matches"true"/"false"respectively. Both templates passed for both values. This repo has no existing automated test coverage for builtin YAML/template content and no live-cluster/e2e harness available in this environment; no user-facing runtime behavior changes for existing configs since the new field defaults totrue.Rollback
Plain revert. No data or config migration is needed since the new field defaults to
true, preserving prior hardcoded behavior.Does this PR introduce a user-facing change?
Checklist
.envfiles committedSigned-off-by)make build)Additional documentation, usage docs, etc.:
Notes for Reviewer
The new field's default (
true) preserves existing behavior for all current configs, so this should be a non-breaking, opt-in change. No automated test was added to the permanent test suite because this repo has no existing test coverage for builtin YAML/template content; happy to add one if reviewers want template-rendering coverage established.AI assistance: this change was drafted with Claude Code.