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

[DNM] testing host isolation #4800

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

npinaeva
Copy link
Contributor

#4799 with host isolation tests enabled
hope to see them fail in the exact places from 81f7424#diff-2be1e01ba9cf5c059a4f4f0cd2b974861043381664e7102e3783f48842891d43

npinaeva and others added 9 commits October 23, 2024 12:53
Since the NAD API resource name uses arbitrary name and
doesn't match the kind name
('network-attachment-definitions' vs. 'NetworkAttachmentDefinition'),
the underlying API machinery cannot match between the resource and
kind names, causing NAD objects to not reflect in the NAD informer [1].

To overcome this, populate the NAD fakeclient tracker with given NADs
objects following the example on [2].

This is preliminary change to enable simplifying UDN controller
tests, making test setup more simple and eliminate using the NAD
informer directly to create/delete NADs.

[1] https://github.com/ovn-org/ovn-kubernetes/blob/65c79af35b2c22f90c863debefa15c4fb1f088cb/go-controller/vendor/k8s.io/client-go/testing/fixture.go#L341
[2] ovn-kubernetes@434b059#diff-ae287d8b2b115068905d4b5bf477d0e8cb6586d271fe872ca3b17acc94f21075R1

Signed-off-by: Or Mergi <[email protected]>
Signed-off-by: Dan Winship <[email protected]>
(cherry picked from commit 7bad589)
Signed-off-by: Dan Winship <[email protected]>
(cherry picked from commit 79b6e3f)
Signed-off-by: Nadia Pinaeva <[email protected]>
Authored-by: Dan Winship <[email protected]>
Signed-off-by: Nadia Pinaeva <[email protected]>
It allows listening for systemd events, we use it to track kubelet
restarts.

Signed-off-by: Nadia Pinaeva <[email protected]>
@github-actions github-actions bot added kind/documentation All issues related to documentation area/unit-testing Issues related to adding/updating unit tests area/e2e-testing labels Oct 23, 2024
UDNHostIsolationManager manages the host isolation for user defined
networks.It uses nftables chain "udn-isolation" to only allow
connection to primary UDN pods from kubelet.
It also listens to systemd events to re-apply the rules after kubelet
restart as cgroup matching is used.

Signed-off-by: Nadia Pinaeva <[email protected]>
Replace podIP with Element, add tracking of duplicate elements owned
by different pods.

Signed-off-by: Nadia Pinaeva <[email protected]>
To allow access to open port from the default hostNetwork pods,
add ntf rules to track icmp and tcp/udp/sctp allowed ports.
Add composed key option to nftPodElementsSet.
Add unit tests for nftPodElementsSet and open port handling.

Signed-off-by: Nadia Pinaeva <[email protected]>
UDN host isolation requires a kernel fix. Turn it off by default in
ovnkube.sh (aka in our kind cluster). As soon as the fix becomes
available for github runners (and dev environments), this flag will be
removed from ovnkube.sh (and potentially from the code).
This flag only takes effect when network segmentation is enabled.

Add DISABLE_UDN_HOST_ISOLATION env variable to test workflow, that is
set to true.
Skip host isolation tests based on the value.

Signed-off-by: Nadia Pinaeva <[email protected]>
Signed-off-by: Nadia Pinaeva <[email protected]>
@npinaeva npinaeva mentioned this pull request Oct 23, 2024
@tssurya tssurya added the feature/user-defined-network-segmentation All PRs related to User defined network segmentation label Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/e2e-testing area/unit-testing Issues related to adding/updating unit tests feature/user-defined-network-segmentation All PRs related to User defined network segmentation kind/documentation All issues related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants