cilium / cilium-cli

CLI to install, manage & troubleshoot Kubernetes clusters running Cilium
https://cilium.io
Apache License 2.0
417 stars 210 forks source link

Expect specific command exit codes in connectivity tests #174

Open ti-mo opened 3 years ago

ti-mo commented 3 years ago

Some connectivity tests (e.g. https://github.com/cilium/cilium-cli/pull/158) make curl fail with (expected) DNS resolution errors.

---------------------------------------------------------------------------------------------------------------------
πŸ”Œ [pod-to-world-toFQDNs] Testing cilium-test/client-68c6675687-xkv85 -> google.com:443...
---------------------------------------------------------------------------------------------------------------------
βŒ› The following command is expected to fail...
βœ… curl command "curl -w %{local_ip}:%{local_port} -> %{remote_ip}:%{remote_port} = %{response_code}\n --show-error --silent --fail --show-error --connect-timeout 5 --output /dev/null https://google.com" failed as expected: command terminated with exit code 28
βœ… [pod-to-world-toFQDNs] cilium-test/client-68c6675687-xkv85 (10.0.0.22) -> google.com (google.com)

We should be able to narrow down the exit codes of the underlying test programs to ensure we don't get any false negatives with regards to test outcomes. For example, it's not acceptable for a test to fail on DNS resolution when a TCP connection failure was expected.

jrajahalme commented 3 years ago

Good point, should work well with curl that has well defined exit codes. ping seems to differentiate between these cases as well:

$ ping -c1 -w1 1.2.3.4
PING 1.2.3.4 (1.2.3.4) 56(84) bytes of data.

--- 1.2.3.4 ping statistics ---
1 packets transmitted, 0 received, 100% packet loss, time 0ms

vagrant@runtime:~/go/src/github.com/cilium/cilium-cli$ echo $?
1
vagrant@runtime:~/go/src/github.com/cilium/cilium-cli$ ping -c1 -w1 foo.bar.net.com
ping: foo.bar.net.com: Name or service not known
vagrant@runtime:~/go/src/github.com/cilium/cilium-cli$ echo $?
2

Not sure how universal the ping return codes are, however.

jrajahalme commented 3 years ago

Maybe we should keep the test fail expectations on the current, more abstract level, and translate them to command specific exit code expectations under the hood?

github-actions[bot] commented 1 day ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.