Fixes on masquerade forwarding mode #812

wyike · 2024-04-10T06:30:05Z

Fixes:

Set ipvs conntrack enablement after IPVS kernel module validation, otherwise with current code, it will raise error like

time="2024-04-04T12:11:15Z" level=info msg="Starting kube-vip.io [v0.7.2]"
time="2024-04-04T12:11:15Z" level=info msg="namespace [kube-system], Mode: [ARP], Features(s): Control Plane:[true], Services:[false]"
time="2024-04-04T12:11:15Z" level=info msg="sysctl set net.ipv4.vs.conntrack to 1"
time="2024-04-04T12:11:15Z" level=fatal msg="failed to open file: open /proc/sys/net/ipv4/vs/conntrack: no such file or directory

when controlplane backend health checks should regard kubeconfig either from "/etc/kubernetes/admin.conf" when kubevip running as a kubeadm Pod or incluster service account when kubevip running as a daemonset. Otherwise it will raise error when daemonset

time="2024-04-04T12:27:58Z" level=info msg="Kube-Vip is watching nodes for control-plane labels"
panic: stat /etc/kubernetes/admin.conf: no such file or directory

correct the comment value of masquerade iptables deletion, to be consistent with the adding operation.
remove useless -d in the command for masquerade iptables deletion. Otherwise it will raise error:

time="2024-04-10T04:29:50Z" level=info msg="Beginning cluster membership, namespace [kube-system], lock name [plndr-cp-lock], id [c100-md-0-fjcl8-4rzrp-j5w7j]"
time="2024-04-10T04:29:50Z" level=error msg="could not delete virtualIP: could not remove iptables masquerade rules : could not del masquerade rule for VIP 10.10.10.201: running [/sbin/iptables-nft -t nat -C POSTROUTING -d -m ipvs --vaddr 10.10.10.201 -j MASQUERADE -m comment --comment  kube-vip load balancer IP --wait]: exit status 2: Bad argument `ipvs'\nTry `iptables -h' or 'iptables --help' for more information.\n"

To enable masquerade forwarding mode as control plane LB default mode, there are still things to do:
- the cleanup on VIP/Iptables/IPVS on kube-vip deletion (SIGTERM or SIGINT) doesn't ever happen which is not good. The staled things may impact setup.
- enable masquerade as the default mode.
I will open two new issues regarding on above.

wyike · 2024-04-10T06:30:30Z

cc @lou-lan for review as well

wyike · 2024-04-10T06:42:53Z

pkg/loadbalancer/ipvs.go

+	err = sysctl.WriteProcSys("/proc/sys/net/ipv4/ip_forward", "1")
+	if err != nil {
+		log.Errorf("ensure net.ipv4.ip_forward is enabled")
+		log.Fatalf("Error ensuring net.ipv4.ip_forward enabled [%v]", err)
+	}
+	log.Infof("sysctl set net.ipv4.ip_forward to 1")


Hi @thebsdbox @lou-lan please suggest on this part. I can remove this part if both of you think it's not quite making sense. Thanks.

Hi @thebsdbox @lou-lan please suggest on this part. I can remove this part if both of you think it's not quite making sense. Thanks.

Adding /proc/sys/net/ipv4/ip_forward looks suitable to me. I noticed that you moved it out of the if block. It is worth noting that this parameter requires appropriate permissions to be set.

initConfig.LoadBalancerForwardingMethod == "masquerade"

Permissions ref:

kube-vip/pkg/kubevip/config_generator.go

Line 512 in 0cedf6a

Privileged: &privileged,

Thanks! Let me correct it. I somehow get a wrong impression that only masquerade mode will enable IPVS..

lubronzhan · 2024-04-10T06:54:59Z

pkg/loadbalancer/ipvs.go

 	i, err := c.Info()
 	if err != nil {
 		log.Errorf("ensure IPVS kernel modules are loaded")
 		log.Fatalf("Error getting IPVS version [%v]", err)
 	}
 	log.Infof("IPVS Loadbalancer enabled for %d.%d.%d", i.Version[0], i.Version[1], i.Version[2])

+	err = sysctl.WriteProcSys("/proc/sys/net/ipv4/vs/conntrack", "1")
+	if err != nil {
+		log.Errorf("ensure net.ipv4.vs.conntrack is enabled")


Is this duplicated with next line?
Or you want to add a log before the command

oh that's because I followed format in this file like https://github.com/kube-vip/kube-vip/blob/main/pkg/loadbalancer/ipvs.go#L57-L65

lubronzhan · 2024-04-10T06:55:35Z

pkg/loadbalancer/ipvs.go

+
+	err = sysctl.WriteProcSys("/proc/sys/net/ipv4/ip_forward", "1")
+	if err != nil {
+		log.Errorf("ensure net.ipv4.ip_forward is enabled")


Same comment as above

thebsdbox · 2024-04-10T16:53:05Z

A few linting bugs that need addressing, these can be tested locally with make check.

wyike · 2024-04-14T14:13:11Z

pkg/vip/address.go

@@ -466,6 +466,8 @@ func (configurator *network) removeIptablesRulesForMasquerade() error {
 	return nil
 }

+// TODO: investigate if adding "--vport <port>" would be better or not quite necessary
+// After this rule is added, ipvs kernel module is also loaded
 func addMasqueradeRuleForVIP(ipt *iptables.IPTables, vip, comment string) error {
 	err := ipt.InsertUnique(iptables.TableNat, iptables.ChainPOSTROUTING,
 		1, "-m", "ipvs", "--vaddr", vip, "-j", "MASQUERADE", "-m", "comment", "--comment", comment)


This rule is quite interesting. After this iptables added, ip_vs kernel module will be loaded to current container host automatically like:

sudo lsmod | grep ip_vs ip_vs 176128 1 xt_ipvs nf_conntrack 172032 10 xt_conntrack,nf_nat,nfnetlink_cttimeout,xt_nat,openvswitch,nf_conntrack_netlink,xt_CT,nf_conncount,xt_MASQUERADE,ip_vs nf_defrag_ipv6 24576 3 nf_conntrack,openvswitch,ip_vs libcrc32c 16384 7 nf_conntrack,nf_nat,openvswitch,btrfs,nf_tables,raid456,ip_vs

Because the AddIP() happens before NewIPVSLB(), kube-vip doesn't require users to load ipvs manually in advance anymore in masquerade mode.

Signed-off-by: Yike Wang <yikew@vmware.com>

thebsdbox · 2024-04-17T14:48:15Z

This is passing all of the tests, is it ready to merge?

wyike · 2024-04-18T02:21:15Z

This is passing all of the tests, is it ready to merge?

yes Dan, waiting for your approvals.

thebsdbox

/lgtm

wyike requested a review from thebsdbox as a code owner April 10, 2024 06:30

wyike commented Apr 10, 2024

View reviewed changes

lubronzhan reviewed Apr 10, 2024

View reviewed changes

wyike force-pushed the fix_masquerade_issues branch 3 times, most recently from 6e41c86 to 45349f2 Compare April 14, 2024 14:01

wyike commented Apr 14, 2024

View reviewed changes

wyike force-pushed the fix_masquerade_issues branch from 45349f2 to c7cebc6 Compare April 16, 2024 10:49

Fixes on masquerade forwarding mode

5100cd0

Signed-off-by: Yike Wang <yikew@vmware.com>

wyike force-pushed the fix_masquerade_issues branch from c7cebc6 to 5100cd0 Compare April 17, 2024 01:00

thebsdbox approved these changes Apr 18, 2024

View reviewed changes

thebsdbox merged commit b0acf84 into kube-vip:main Apr 18, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes on masquerade forwarding mode #812

Fixes on masquerade forwarding mode #812

wyike commented Apr 10, 2024 •

edited

wyike commented Apr 10, 2024

wyike Apr 10, 2024 •

edited

lou-lan Apr 11, 2024 •

edited

wyike Apr 12, 2024 •

edited

lubronzhan Apr 10, 2024 •

edited

wyike Apr 10, 2024 •

edited

lubronzhan Apr 10, 2024

thebsdbox commented Apr 10, 2024

wyike Apr 14, 2024

thebsdbox commented Apr 17, 2024

wyike commented Apr 18, 2024 •

edited

thebsdbox left a comment

Fixes on masquerade forwarding mode #812

Fixes on masquerade forwarding mode #812

Conversation

wyike commented Apr 10, 2024 • edited

wyike commented Apr 10, 2024

wyike Apr 10, 2024 • edited

Choose a reason for hiding this comment

lou-lan Apr 11, 2024 • edited

Choose a reason for hiding this comment

wyike Apr 12, 2024 • edited

Choose a reason for hiding this comment

lubronzhan Apr 10, 2024 • edited

Choose a reason for hiding this comment

wyike Apr 10, 2024 • edited

Choose a reason for hiding this comment

lubronzhan Apr 10, 2024

Choose a reason for hiding this comment

thebsdbox commented Apr 10, 2024

wyike Apr 14, 2024

Choose a reason for hiding this comment

thebsdbox commented Apr 17, 2024

wyike commented Apr 18, 2024 • edited

thebsdbox left a comment

Choose a reason for hiding this comment

wyike commented Apr 10, 2024 •

edited

wyike Apr 10, 2024 •

edited

lou-lan Apr 11, 2024 •

edited

wyike Apr 12, 2024 •

edited

lubronzhan Apr 10, 2024 •

edited

wyike Apr 10, 2024 •

edited

wyike commented Apr 18, 2024 •

edited