Fix to avoid adding FoU devices for pods that don't use its egress

[terassyi]

This PR is related: #253

Background

From coil v2.5, coild runs EgressWatcher to watch Egress resources.

When Egress resources are modified(or created, deleted), EgressWatcher reflects its configuration to pods that use its Egress.

What

When reflecting the change to the pod, EgressWatcher picks the wrong network namespace and creates a device for the wrong pod.

Why

EgressWatcher gets the network namespace associated with the target pod by the pods' IP addresses.

coil/v2/pkg/nodenet/pod.go

Line 425 in ea4d2ac

if c.IPv4.Equal(podIPv4) || c.IPv6.Equal(podIPv6) {

In this code, either c.IPv4.Equal(podIPv4) or c.IPv6.Equal(podIPv6) return true in almost case.
In the case of IPv4(or IPv6) single stack, this condition always returns true, and netNsPath is updated in every loop.
As a result, the wrong network namespace is picked.

[masa213f]

As I commented elsewhere, please revise the test of podNetwrok.Update().
The test passes even after making the following changes.

diff --git a/v2/pkg/nodenet/pod.go b/v2/pkg/nodenet/pod.go
index adc5ef4..2fb7a06 100644
--- a/v2/pkg/nodenet/pod.go
+++ b/v2/pkg/nodenet/pod.go
@@ -424,7 +424,7 @@ func (pn *podNetwork) Update(podIPv4, podIPv6 net.IP, hook SetupHook) error {
        for _, c := range podConfigs {
                // When both c.IPvX and podIPvX are nil, net.IP.Equal() returns always true.
                // Avoiding comparing nil to nil, confirm c.IPvX is not nil.
-               if (c.IPv4 != nil && c.IPv4.Equal(podIPv4)) || (c.IPv6 != nil && c.IPv6.Equal(podIPv6)) {
+               if true {
                        netNsPath, err = getNetNsPath(c.HostVethName)
                        if err != nil {
                                return err

[masa213f]

1. Please check the length of result.IPs.

2. Error messages are wrong, aren't they?

Suggested change

	if isDualStack(&conf) {
	if len(result.Interfaces) != 2 {
	t.Error(`len(result.Interfaces) != 2`)
	}
	if !result.IPs[0].Address.IP.Equal(conf.IPv4) {
	t.Errorf(`!result.IPs[0].Address.IP.Equal("%s")`, conf.IPv4)
	}
	if result.IPs[0].Address.IP.To4() == nil {
	t.Error(`!result.IPs[0] version != "4"`)
	}
	if !result.IPs[1].Address.IP.Equal(conf.IPv6) {
	t.Errorf(`!result.IPs[1].Address.IP.Equal("%s")`, conf.IPv6)
	}
	if result.IPs[1].Address.IP.To4() != nil {
	t.Error(`!result.IPs[1] version != "6"`)
	}
	} else {
	if len(result.Interfaces) != 2 {
	t.Error(`len(result.Interfaces) != 2`)
	}
	if conf.IPv4 != nil {
	if !result.IPs[0].Address.IP.Equal(conf.IPv4) {
	t.Errorf(`!result.IPs[0].Address.IP.Equal("%s")`, conf.IPv4)
	}
	if result.IPs[0].Address.IP.To4() == nil {
	t.Error(`!result.IPs[0] version != "4"`)
	}
	} else {
	if !result.IPs[0].Address.IP.Equal(conf.IPv6) {
	t.Errorf(`!result.IPs[0].Address.IP.Equal("%s")`, conf.IPv6)
	}
	if result.IPs[0].Address.IP.To4() != nil {
	t.Error(`!result.IPs[1] version != "6"`)
	}
	}
	}
	if isDualStack(&conf) {
	if len(result.Interfaces) != 2 {
	t.Error(`len(result.Interfaces) != 2`)
	}
	if !result.IPs[0].Address.IP.Equal(conf.IPv4) {
	t.Errorf(`!result.IPs[0].Address.IP.Equal("%s")`, conf.IPv4)
	}
	if result.IPs[0].Address.IP.To4() == nil {
	t.Error(`result.IPs[0] version != "4"`)
	}
	if !result.IPs[1].Address.IP.Equal(conf.IPv6) {
	t.Errorf(`!result.IPs[1].Address.IP.Equal("%s")`, conf.IPv6)
	}
	if result.IPs[1].Address.IP.To4() != nil {
	t.Error(`result.IPs[1] version != "6"`)
	}
	} else {
	if len(result.Interfaces) != 2 {
	t.Error(`len(result.Interfaces) != 2`)
	}
	if conf.IPv4 != nil {
	if !result.IPs[0].Address.IP.Equal(conf.IPv4) {
	t.Errorf(`!result.IPs[0].Address.IP.Equal("%s")`, conf.IPv4)
	}
	if result.IPs[0].Address.IP.To4() == nil {
	t.Error(`result.IPs[0] version != "4"`)
	}
	} else {
	if !result.IPs[0].Address.IP.Equal(conf.IPv6) {
	t.Errorf(`!result.IPs[0].Address.IP.Equal("%s")`, conf.IPv6)
	}
	if result.IPs[0].Address.IP.To4() != nil {
	t.Error(`result.IPs[1] version != "6"`)
	}
	}
	}

[masa213f]

I think one if statement is enough.

Suggested change

	if err == nil {
	t.Fatal("pn.Check must return error because pod2 network doesn't exist")
	} else {
	if err != errNotFound {
	t.Fatal("given error must be errNotFound")
	}
	}
	if err != errNotFound {
	t.Fatal("pn.Check must return errNotFound because pod2 network doesn't exist")
	}

[masa213f]

LGTM