cilium-cli icon indicating copy to clipboard operation
cilium-cli copied to clipboard

Add pod-to-pod strict mode tests

Open 3u13r opened this issue 2 years ago • 5 comments

This PR implements the pod-to-pod strict mode tests in the cilium-cli test suite. Currently, there are similar tests inside the legacy vagrant test suite (https://github.com/cilium/cilium/blob/836598a317565d4c53c678bb09173ae9fee5f54b/test/k8s/datapath_configuration.go#L387).

With this PR we could deprecate and remove the vagrant tests if we add strict mode versions to Cilium's conformance CI. The difference is that the vagrant tests control the Kubernetes environment and cilium configuration for each test. Therefore the test coverage regarding different cilium configurations is easier to achieve.

My question is: If the vagrant tests are "legacy", what is the best way to implement potentially disrupting tests which need to perform changes to the K8s environment? It seems like that this test suite was originally developed to implement tests which the user can execute in their production environment without any disruption. I guess this is the discussion you have right now: https://github.com/cilium/design-cfps/pull/9#discussion_r1218651510.

3u13r avatar Sep 01 '23 13:09 3u13r

Thanks for the PR!

I wonder whether it's possible to implement tests w/o causing disruptions to an existing Cilium installation. My main worry that it might cause flakiness for other tests.

Thinking out loud, in the existing WG strict test we stop cilium-operator and remove some artifacts (CE CRDs) to simulate an IPcache delay, so that for a remote CE there is no corresponding IPcache entry, which results in the WG strict feature dropping the packets, as the strict CIDR is set to podCIDR.

The easiest way to simulate such an IPcache delay is to implement the test as a control-plane e2e test, which is currently discussed. TL;DR start two instances of cilium-agent in two connected network namespaces from Go, and then mock / modify one of the agents IPcache updates path to introduce delays (cc @margamanterola / @joamaki / @ti-mo).

I'm not sure when we gonna have such control tests available soon, so we need to decide whether to merge this PR and risk flakiness, or live for now with the legacy Vagrant / ginkgo tests. I'm leaning towards the latter.

brb avatar Sep 04 '23 13:09 brb

@brb Could you elaborate on the flakiness risk? Is there any way we could avoid that risk that doesn't require the control plane tests?

margamanterola avatar Sep 05 '23 07:09 margamanterola

@margamanterola The test needs to scale down cilium-operator replicas count to 0, then removes some CRDs, runs tests, and brings back cilium-operator replica count to a previous. The problem I see is that some CLI test cases might be missing proper wait statements which could result in flakiness (the missing waits are notorious in Cilium / CLI). Another issue is that we assume the CLI connectivity tests won't modify Cilium instances. But if you think that control-plane e2e doesn't happen soon, then we should consider that option.

brb avatar Sep 05 '23 09:09 brb

I see potential flakiness reasons in:

  • the test not resetting the operator scale correctly
  • not being able to concurrently execute tests, which is also not possible for the other network policy tests I guess. The tests in this PR just also ignore any K8s namespace boundary.

If there are waits missing in the other tests we always add verification that everything was restored correctly. We kinda do this already since we first test that the traffic is blocked and then also test that after the operator was scaled up again and the endpoint we previously deleted has been restored, the connection succeeds.

To summarize: we delete one endpoint and explicitly wait for its regeneration. We can still have missing endpoints but I don't think the risk is much higher than when e.g. new pods are spawned.

I mostly ported the test:

  • to migrate away from the legacy tests should they be shut down in the future
  • to get a feel how to develop strict tests to develop a node-to-node strict mode via TDD

If there's another test framework/suite which better fits the requirements of those tests I'm happy to wait.

3u13r avatar Sep 05 '23 09:09 3u13r

@3u13r Are you in Cilium's Slack? Let's discuss it in #development channel (I am martynas there).

brb avatar Sep 07 '23 08:09 brb

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Sep 28 '24 02:09 github-actions[bot]

This pull request has not seen any activity since it was marked stale. Closing.

github-actions[bot] avatar Oct 13 '24 02:10 github-actions[bot]