Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Failing e2e tests #194

Closed
LionelJouin opened this issue Dec 4, 2023 · 4 comments · Fixed by #198 or #201
Closed

[BUG] Failing e2e tests #194

LionelJouin opened this issue Dec 4, 2023 · 4 comments · Fixed by #198 or #201
Assignees

Comments

@LionelJouin
Copy link
Collaborator

Describe the bug

1. address already in use

I am not sure, but I believe the macvlan interface (type of cni used in the e2e tests) is not fully deleted and since the mac address used is always the same for each e2e tests, the kernel refuses a new macvlan interface to be created with an already existing mac address.

I would propose to remove the hard-coded mac address and generate a random mac address for each tests.

// https://stackoverflow.com/questions/21018729/generate-mac-address-in-go
func generateMacAddress() (net.HardwareAddr, error) {
	buf := make([]byte, 6)
	_, err := rand.Read(buf)
	if err != nil {
		return nil, err
	}

	buf[0] = (buf[0] | 2) & 0xfe // Set local bit, ensure unicast addres

	return buf, nil
}
2. timed out waiting for the condition

This issue is related to Multus and requires at least 2 tests running.
The pods UID retrieved by Multus when adding a network belongs to a pod that was running in the previous tests. That pod has been deleted in the previous test and a new one with exactly the same name and namespace has been created by the current test.

It could probably solved with different way of handling cache in Multus. Otherwise, having the "watch" permission is also solving the problem (a PR exists in Multus: k8snetworkplumbingwg/multus-cni#1171).

logs:

2023-12-02T22:47:33.740071666Z stderr F 2023-12-02T22:47:33Z [verbose] ADD finished CNI request ContainerID:"8eadc68bc8957fc260db7c320af98a8694136a1f266d0736f8266fee529606ae" Netns:"/var/run/netns/cni-7b2611d8-ada2-07ff-1b08-a6b13b6198fc" IfName:"eth0" Args:"IgnoreUnknown=1;K8S_POD_NAMESPACE=ns1;K8S_POD_NAME=tiny-winy-pod;K8S_POD_INFRA_CONTAINER_ID=8eadc68bc8957fc260db7c320af98a8694136a1f266d0736f8266fee529606ae;K8S_POD_UID=c01880d4-5c38-4cf8-b7b4-a7e8dee315e7" Path:"", result: "", err: error configuring pod [ns1/tiny-winy-pod] networking: Multus: [ns1/tiny-winy-pod/c01880d4-5c38-4cf8-b7b4-a7e8dee315e7]: expected pod UID "c01880d4-5c38-4cf8-b7b4-a7e8dee315e7" but got "f164f508-c820-4741-98d7-f1ebe1f93a1c" from Kube API
3. a provisioned pod whose network selection elements do not feature the interface name

This test creates a pod with a network in which the interface name is not included, this interface is added at pod creation successfully since Multus assign an interface name (net).
When adding a new correct interface in the pod annotation, it should not be reconciled since the first interface should not be handled due to the lack name.

I found a few problems regarding this issue:

(3.1.) Sometimes, the order in which the interfaces are handled is changing, so the interface is still added. The order is random since, to create the attachmentsToAdd slice, the loop is iterating over a map.
https://github.com/k8snetworkplumbingwg/multus-dynamic-networks-controller/blob/v0.3.2/pkg/controller/pod.go#L213

(3.2.) If 2 interfaces has to be added and the first one gets added correctly but not the second, the first one will not be added in the network status, but (the interface) will still be added inside the pod.
This happens because the attachmentResults must always be returned even if an error is returned.
https://github.com/k8snetworkplumbingwg/multus-dynamic-networks-controller/blob/v0.3.2/pkg/controller/pod.go#L337

(3.3.) This statement could not catch anything since there is no wait for reconciliation.
https://github.com/k8snetworkplumbingwg/multus-dynamic-networks-controller/blob/v0.3.2/e2e/e2e_test.go#L297

(3.4.) Is it a valid test knowing we updated the network annotation on the fly?
#63

Expected behavior

/

To Reproduce

  1. kubectl apply -f https://raw.githubusercontent.com/k8snetworkplumbingwg/multus-cni/master/e2e/templates/cni-install.yml.j2
  2. kubectl apply -f https://raw.githubusercontent.com/k8snetworkplumbingwg/multus-cni/master/deployments/multus-daemonset-thick.yml
  3. kubectl apply -f manifests/dynamic-networks-controller.yaml
  4. make e2e/test

Environment:

  • multus-dynamic-networks-controller version: 21b6dee (latest)

Additional info / context
/

@maiqueb
Copy link
Collaborator

maiqueb commented Jan 8, 2024

/cc

@maiqueb maiqueb self-assigned this Jan 8, 2024
@LionelJouin
Copy link
Collaborator Author

Hi @maiqueb, I can contribute these changes if needed. 2. timed out waiting for the condition is now solved since the watch permission PR has been merged in Multus.

@maiqueb
Copy link
Collaborator

maiqueb commented Jan 8, 2024

Hi @maiqueb, I can contribute these changes if needed. 2. timed out waiting for the condition is now solved since the watch permission PR has been merged in Multus.

Let's go step by step. Focus on 1 for now - i.e. using a different MAC address in each of the tests. You'll need to change the tests to assert that particular MAC is reported in the status, but that should be easy enough.

Once that's in, we can look where next to focus.

Thanks for the help. Highly appreciated.

@maiqueb
Copy link
Collaborator

maiqueb commented Jan 22, 2024

This issue has only been partly addressed by #201 .

Still, it does really well at improving the CI effectiveness.

@maiqueb maiqueb reopened this Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants