add controller-driven kube-vip load balancer for K3S_BASE clusters#5683
add controller-driven kube-vip load balancer for K3S_BASE clusters#5683eriknordmark merged 2 commits intolf-edge:masterfrom
Conversation
ceda466 to
9c5b309
Compare
| // collectLBPoolStatus reads the kubevip ConfigMap from kube-system to get the configured | ||
| // load balancer pool, and gathers IPs currently allocated to LoadBalancer-type services. | ||
| // Returns nil if the kubevip ConfigMap does not exist (kubevip not yet deployed). | ||
| func collectLBPoolStatus(clientset *kubernetes.Clientset, services []types.KubeServiceInfo) *types.KubeLBPoolStatus { |
There was a problem hiding this comment.
Do we publish this LBPoolStatus in any eve pub/sub calls for collect info ?
There was a problem hiding this comment.
this LBPoolStatus is published by zedkube in KubeUserServices, so it is part of the collectinfo.
9c5b309 to
7952e18
Compare
andrewd-zededa
left a comment
There was a problem hiding this comment.
@naiming-zededa A few questions and a small requested change to the PR description.
| if ifName == "" || len(cidrs) == 0 { | ||
| continue | ||
| } | ||
| if _, _, lbErr := net.ParseCIDR(cidrs[0]); lbErr != nil { |
There was a problem hiding this comment.
Only the first address CIDR here is parsed, is len(cidrs) > 1 an error case or just not supported currently?
There was a problem hiding this comment.
yes, we currently only support one intf w/ one prefix, i'll add a log warning message here if it is more than one.
There was a problem hiding this comment.
@naiming-zededa let me know when you've updated the PR with the log message.
There was a problem hiding this comment.
@eriknordmark log message already updated above, for the warning.
if len(cidrs) > 1 {
log.Warnf("parseEdgeNodeClusterConfig: interface %s has %d CIDRs, only the first is supported; ignoring the rest",
ifName, len(cidrs))
}
There was a problem hiding this comment.
also updated in the cluster-init.sh for the log:
if [ "$lb_count" -gt 1 ] 2>/dev/null; then
logmsg "check_kubevip_lb: $lb_count LB interfaces configured, only the first is supported; ignoring the rest"
fi
| local lb_iface="" lb_cidr="" lb="" | ||
| if [ -f "$enc_status_file" ]; then | ||
| enc_data=$(cat "$enc_status_file") | ||
| lb_iface=$(echo "$enc_data" | jq -r '.LBInterfaces[0].Interface // ""') |
There was a problem hiding this comment.
Is only 1 LB interface supported currently?
There was a problem hiding this comment.
yes, i'll add a log message here for potential more than 1 case.
| labels: | ||
| app.kubernetes.io/name: kube-vip-ds | ||
| app.kubernetes.io/version: v0.8.9 | ||
| app.kubernetes.io/version: v1.1.0 |
There was a problem hiding this comment.
What was the motivator for the upgrade here? Was this a required feature in the major version change? Please note this in the PR description.
There was a problem hiding this comment.
this older version was put before, but we didn't have a use for kube-vip until now, so update this to the latest. this kube-vip v1.1.0 already supported in k3s 1.26, so it's a while there. I'll add a description on this.
| // Only the bootstrap node manages kube-vip load balancing; other nodes leave | ||
| // LBInterfaces empty so cluster-init.sh does not apply kubevip. | ||
| if z.clusterConfig.BootstrapNode { | ||
| status.LBInterfaces = z.clusterConfig.LBInterfaces | ||
| } | ||
|
|
There was a problem hiding this comment.
How will the lb be handled if the bootstrap node has failed and is pending replacement? Can the stats lease holder perform this instead?
There was a problem hiding this comment.
Not really, if the seed node is down and before the clusterstatus w/ this config is applied, then we will not have LB service, until a new seed node is identified from the controller side. But if it is already applied, then it does not matter, it's in the cluster regardless of seed node is there. That would be a very rare corner case, and we will to handle this separately.
7952e18 to
97eb753
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5683 +/- ##
===========================================
+ Coverage 19.52% 29.87% +10.34%
===========================================
Files 19 18 -1
Lines 3021 2417 -604
===========================================
+ Hits 590 722 +132
+ Misses 2310 1549 -761
- Partials 121 146 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # successfully applied config so that container/device restarts do not | ||
| # trigger a redundant re-apply (which causes "configmaps kubevip already | ||
| # exists" errors from kube-vip-cloud-provider). | ||
| KUBEVIP_STATE_FILE="/var/lib/kubevip-applied" |
There was a problem hiding this comment.
@naiming-zededa is this file and the kubevip config-map available in collect-info to view?
There was a problem hiding this comment.
@andrewd-zededa no, i have updated the 'collect-info.sh' and i now do a list of /var/lib for all the directory so we don't have to do one-by-one later. the config-map, there is nothing new there, only has the interface and prefix we already know, so i'm not adding that.
97eb753 to
27f70ac
Compare
There was a problem hiding this comment.
Please increase the script version
| # Uses a targeted merge patch on developerConfiguration.featureGates only, so that | ||
| # permittedHostDevices (PCIe/GPU/USB passthrough) is never disturbed. | ||
| Kubevirt_migrate_feature_gates() { | ||
| if [ -f /var/lib/base-k3s-mode ]; then |
There was a problem hiding this comment.
Is this related to this PR? Or is it a separate fix that should be maybe backported to 16.0 LTS?
There was a problem hiding this comment.
No, this is only a fix in the previous committed PR. That was on enhance the virtio driver for eve-k image, this is not part of the 16.0 LTS.
|
Thanks for the detailed documentation, helps a lot! |
27f70ac to
cddcacf
Compare
|
@naiming-zededa looks like a docker hash issue: |
cddcacf to
1b34d5c
Compare
thanks @andrewd-zededa. Just updated. |
This PR implements controller-driven Kubernetes LoadBalancer services
for CLUSTER_TYPE_K3S_BASE in eve-k.
- pkg/pillar/types/clustertypes.go: Add LBInterfaceConfig (interface +
CIDR string) and LBInterfaces []LBInterfaceConfig to both
EdgeNodeClusterConfig and EdgeNodeClusterStatus.
- pkg/pillar/cmd/zedagent/parseconfig.go: Parse LoadBalancerService
from the controller proto and populate
EdgeNodeClusterConfig.LBInterfaces (K3S_BASE only; first
interface/CIDR entry applied).
- pkg/pillar/cmd/zedkube/clusterstatus.go: Relay LBInterfaces from
EdgeNodeClusterConfig into EdgeNodeClusterStatus on the bootstrap
node only; non-bootstrap nodes publish an empty list so they do not
trigger kube-vip setup.
- pkg/pillar/dpcmanager/dns.go: Filter kube-vip VIPs out of
DeviceNetworkStatus.AddrInfoList using the LBInterfaces CIDR range,
preventing VIPs from being used as source addresses for
controller-bound traffic.
- pkg/kube/cluster-init.sh: Add check_kubevip_lb loop that reads
EdgeNodeClusterStatus JSON each iteration and calls kubevip-apply.sh
or kubevip-delete.sh when the LB config changes. Persists
last-applied state to avoid redundant re-applies across restarts.
- pkg/kube/kubevip-apply.sh / kubevip-delete.sh: Scripts to
install/remove the kube-vip DaemonSet and kube-vip-cloud-provider
Deployment, configuring the IP pool via a kubevip ConfigMap.
- pkg/kube/kubevip-ds.yaml: kube-vip DaemonSet manifest (ARP mode,
control-plane nodes).
- pkg/kube/config.yaml: Disable k3s built-in ServiceLB (servicelb) and
Traefik for K3S_BASE — kube-vip replaces ServiceLB; users bring
their own ingress.
- pkg/pillar/docs/zedkube.md: Document the feature with an overview
diagram, data-flow, EVE-API proto, and DeviceNetworkStatus filtering
notes.
Signed-off-by: naiming-zededa <naiming@zededa.com>
1b34d5c to
3628d23
Compare
… K3S_BASE - add the pillar vendor files updated eve-api Signed-off-by: naiming-zededa <naiming@zededa.com>
3628d23 to
510f519
Compare
andrewd-zededa
left a comment
There was a problem hiding this comment.
LGTM, but should wait for master riscv build fix before merge I guess.
- PR lf-edge#5720 for using the logical-label for clustering interface and the PR lf-edge#5683 for clustering load-balancing, this one addes the handling of logical label in the cluster load-balancing interface Signed-off-by: naiming-zededa <naiming@zededa.com>
Description
PR dependencies
How to test and validate this PR
Create a native orchestration type cluster on EVE devices, and in the controller config the enabling of 'loadbalancing'
for the cluster. Specify the interface and IP prefix of the LB.
In the kubernetes side, user specify the helm/yaml definition of App and Service with type of 'loadbalancer'. Verify
the service has the IP address allocated on the interface for this service, and endpoint created for the App.
User the specify client to access that ip address and port. and verify when one device is down, the IP is reallocated
to another device of the cluster, and the app is still reachable.
there are many different ways to use the LB service, see examples in pkg/pillar/docs/zedkube.md for detail
Changelog notes
add controller-driven kube-vip load balancer for K3S_BASE clusters
PR Backports
Checklist
For backport PRs (remove it if it's not a backport):
And the last but not least:
check them.