feat: implement collector and analyser for network namespace connectivity #1670

ricardomaraschini · 2024-10-31T20:15:03Z

Description, Motivation and Context

Important

This feature is only supported when running on Linux, all others platforms return an unsupported platform error.

Checks if two network namespaces can talk to each other on UDP and TCP. Its usage is as follows:

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  name: test
spec:
  hostCollectors:
  - networkNamespaceConnectivity:
      collectorName: check-network-connectivity
      fromCIDR: 10.0.0.0/24
      toCIDR: 10.0.1.0/24
  hostAnalyzers:
  - networkNamespaceConnectivity:
      collectorName: check-network-connectivity
      outcomes:
      - pass:
          message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 is working"
      - fail:
          message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 isn't working"

It is also available on a HostPreflight object:

apiVersion: troubleshoot.sh/v1beta2
kind: HostPreflight
metadata:
    name: ec-cluster-preflight
spec:
    collectors:
        - networkNamespaceConnectivity:
            collectorName: check-network-connectivity
            fromCIDR: 10.0.0.0/24
            toCIDR: 10.0.1.0/24
            port: 8888
    analyzers:
        - networkNamespaceConnectivity:
            collectorName: check-network-connectivity
            outcomes:
            - pass:
                message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 is working"
            - fail:
                message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 isn't working"

The analyzer output can be templated as follows:

apiVersion: troubleshoot.sh/v1beta2
kind: HostPreflight
metadata:
    name: ec-cluster-preflight
spec:
    collectors:
        - networkNamespaceConnectivity:
            collectorName: check-network-connectivity
            fromCIDR: 10.0.0.0/24
            toCIDR: 10.0.1.0/24
    analyzers:
        - networkNamespaceConnectivity:
            collectorName: check-network-connectivity
            outcomes:
            - pass:
                message: "Communication between {{ .FromNamespace }}  and {{ .ToNamespace }} is working"
            - fail:
                message: "{{ .ErrorMessage }}"

Tip

If this fails then you may need to enable IP forwarding with:

sysctl -w net.ipv4.ip_forward=1

If it still fails then you may need to configure firewalld to allow the traffic or simply disable it for sake of testing.

firewall-cmd --new-zone=ec --permanent
firewall-cmd --zone=ec --set-target=ACCEPT --permanent
firewall-cmd --zone=ec --add-source=10.0.0.0/24 --permanent
firewall-cmd --zone=ec --add-source=10.0.1.0/24 --permanent
firewall-cmd --reload

or

systemctl stop firewalld

Notes

This is being implemented to be used on our Embedded Cluster.
We have chosen to require the user to provide CIDRs to avoid false positives (firewall rules may vary).
Documentation hasn't been written yet as its content may vary based on how this PR review.
The port property is optional, if none is provide then 8080 is used.

Workflow

                                                                 
  ┌───────────────────────────────────────────────────────────┐  
  │                    DEFAULT NAMESPACE                      │  
  │                                                           │  
  │ ┌────────────┐                             ┌────────────┐ │  
  │ │ From NS    │                             │ To NS      │ │  
  │ │            │                             │            │ │  
  │ │            │                             │            │ │  
  │ │ TCP CLIENT │                             │ UDP SERVER │ │  
  │ │ UDP CLIENT │                             │ TCP SERVER │ │  
  │ │        ▲   │                             │        ▲   │ │  
  │ │   │    │   │                             │  │     │   │ │  
  │ │   │    │   │                             │  │     │   │ │  
  │ │   │    └───┼─────────────────────────────┼──┘     │   │ │  
  │ │   └────────┼─────────────────────────────┼────────┘   │ │  
  │ │  ┌─────────│───────────┐     ┌───────────│─────────┐  │ │  
  │ │  │10.0.0.1 │ 10.0.0.254│     │10.0.1.254 │ 10.0.1.1│  │ │  
  │ │  └─────────│───────────┘     └───────────│─────────┘  │ │  
  │ └────────────┘                             └────────────┘ │  
  │                                                           │  
  └───────────────────────────────────────────────────────────┘

Two namespaces are created (called from and to).
An UDP and a TCP servers are started in the to namespace.
Connections are made to these servers from the from namespace.
Packets traverse out the from namespace through the veth interface and into the to namespace through routing.
First IP of the range is chosen as the internal IP while the last is chosen as the gateway out of the namespace.

Optional Configurations

The collector supports the following additional optional configurations:

Configuration	Default Value	Description
`port`	`8080`	The port to be used by both UDP and TCP connections
`timeout`	`5s`	The timeout for the UDP and TCP connections

Message templating

The following templating variables are available when templating the message of a failure outcome:

Property	Description
`{{ .ErrorMessage }}`	Show all error messages found during the collection
`{{ .FromNamespace }}`	The CIDR provided in the collector `from` property
`{{ .ToNamespace }}`	The CIDR provided in the collector `to` property

Checklist

New and existing tests pass locally with introduced changes.
Tests for the changes have been added (for bug fixes / features)
The commit message(s) are informative and highlight any breaking changes
Any documentation required has been added/updated. For changes to https://troubleshoot.sh/ create a PR here

Does this PR introduce a breaking change?

Yes
No

…vity checks if two network namespaces can talk to each other on udp and tcp. its usage is as follows: ```yaml apiVersion: troubleshoot.sh/v1beta2 kind: SupportBundle metadata: name: test spec: hostCollectors: - networkNamespaceConnectivity: collectorName: check-network-connectivity fromCIDR: 10.0.0.0/24 toCIDR: 10.0.1.0/24 hostAnalyzers: - networkNamespaceConnectivity: collectorName: check-network-connectivity outcomes: - pass: message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 is working" - fail: message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 isn't working" ``` if this fails then you may need to enable `forwarding` with: ```bash sysctl -w net.ipv4.ip_forward=1 ``` if it still fails then you may need to configure firewalld to allow the traffic or simply disable it for sake of testing.

check both protocols even if one fails. this pr commit also introduces a timeout that can be set by the user.

allow users to dump the errors found during the analysis.

banjoh

The collector works well. I was able to perform the tests as described in the PR.

There might be resource leak somewhere. One of the virtual network interfaces is left behind after the collector completes. The screenshot below shows the support-bundle binary waiting for user input. At this point, collection, redaction and analysis have already taken place. Any resources by collectors should have been deleted. Exiting support-bundle removes the network interface suggesting that one of the servers is left running in some go routine.

pkg/namespaces/network-namespace.go

pkg/analyze/host_network_namespace_connectivity.go

ricardomaraschini · 2024-11-05T08:06:49Z

There might be resource leak somewhere. One of the virtual network interfaces is left behind after the collector completes. The screenshot below shows the support-bundle binary waiting for user input. At this point, collection, redaction and analysis have already taken place. Any resources by collectors should have been deleted. Exiting support-bundle removes the network interface suggesting that one of the servers is left running in some go routine.

Interesting, I haven't seen this on my tests and in a simple attempt to reproduce it now. Would you mind sharing the YAML you have used so I can try to reproduce this ?

pkg/namespaces/network-namespace.go

banjoh · 2024-11-05T10:07:58Z

There might be resource leak somewhere. One of the virtual network interfaces is left behind after the collector completes. The screenshot below shows the support-bundle binary waiting for user input. At this point, collection, redaction and analysis have already taken place. Any resources by collectors should have been deleted. Exiting support-bundle removes the network interface suggesting that one of the servers is left running in some go routine.

Interesting, I haven't seen this on my tests and in a simple attempt to reproduce it now. Would you mind sharing the YAML you have used so I can try to reproduce this ?

I used the same spec you have in your description

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  name: test
spec:
  hostCollectors:
  - networkNamespaceConnectivity:
      collectorName: check-network-connectivity
      fromCIDR: 10.0.0.0/24
      toCIDR: 10.0.1.0/24
  hostAnalyzers:
  - networkNamespaceConnectivity:
      collectorName: check-network-connectivity
      outcomes:
      - pass:
          message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 is working"
      - fail:
          message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 isn't working"

I ran support-bundle spec.yaml while watching existing interfaces using ip -s link show

even though the interface pair is deleted everyttime we delete the namespace on my tests we better delete it before we delete the namespace. this comes out of a review comment where some people seem to still be able to see the interface pair even after the namespace is deleted. i.e. better safe than sorry.

ricardomaraschini · 2024-11-05T10:18:02Z

There might be resource leak somewhere. One of the virtual network interfaces is left behind after the collector completes. The screenshot below shows the support-bundle binary waiting for user input. At this point, collection, redaction and analysis have already taken place. Any resources by collectors should have been deleted. Exiting support-bundle removes the network interface suggesting that one of the servers is left running in some go routine.

Interesting, I haven't seen this on my tests and in a simple attempt to reproduce it now. Would you mind sharing the YAML you have used so I can try to reproduce this ?

I used the same spec you have in your description
apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  name: test
spec:
  hostCollectors:
  - networkNamespaceConnectivity:
      collectorName: check-network-connectivity
      fromCIDR: 10.0.0.0/24
      toCIDR: 10.0.1.0/24
  hostAnalyzers:
  - networkNamespaceConnectivity:
      collectorName: check-network-connectivity
      outcomes:
      - pass:
          message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 is working"
      - fail:
          message: "Communication between 10.0.0.0/24 and 10.0.1.0/24 isn't working"
I ran support-bundle spec.yaml while watching existing interfaces using ip -s link show

I ran this more than a thousand times this morning and I could not reproduce. Tried on kernels 6.8.0-47-generic and 5.14.0-427.13.1.el9_4.x86_64 and I haven't seen this behavior not even a single time.

For sake of testing I have raised this commit, can you please check if you can still reproduce this ?

Please let me know what Kernel you are using.

banjoh

LGTM

banjoh · 2024-11-06T09:55:36Z

After your last commit to fix deletion of the interface things look good now

ricardomaraschini added the type::feature New feature or request label Oct 31, 2024

ricardomaraschini marked this pull request as ready for review October 31, 2024 20:18

ricardomaraschini requested a review from a team as a code owner October 31, 2024 20:18

ricardomaraschini added 8 commits October 31, 2024 21:19

chore: rebuild schemas

e10a71e

chore: remove unused property

c66c768

chore: disable namespaces for other platforms

1048513

chore: make sure we timeout temporary servers

d4e4822

Merge branch 'main' into network-namespace-collector-and-analyser

c5e660e

feat: analyzer now supports multi-node collection

5271efd

feat: check both udp and tcp even on failure

4c349a4

check both protocols even if one fails. this pr commit also introduces a timeout that can be set by the user.

feat: add templating to the failure outcome

e837418

allow users to dump the errors found during the analysis.

banjoh requested changes Nov 4, 2024

View reviewed changes

pkg/namespaces/network-namespace.go Outdated Show resolved Hide resolved

pkg/analyze/host_network_namespace_connectivity.go Outdated Show resolved Hide resolved

pkg/analyze/host_network_namespace_connectivity.go Outdated Show resolved Hide resolved

chore: addressing pr comments

e891a89

banjoh reviewed Nov 5, 2024

View reviewed changes

pkg/namespaces/network-namespace.go Outdated Show resolved Hide resolved

chore: fix typo on comment

e15a4fa

banjoh approved these changes Nov 6, 2024

View reviewed changes

ricardomaraschini merged commit e272683 into main Nov 6, 2024
27 checks passed

ricardomaraschini deleted the network-namespace-collector-and-analyser branch November 6, 2024 10:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement collector and analyser for network namespace connectivity #1670

feat: implement collector and analyser for network namespace connectivity #1670

ricardomaraschini commented Oct 31, 2024 •

edited

Loading

banjoh left a comment •

edited

Loading

ricardomaraschini commented Nov 5, 2024 •

edited

Loading

banjoh commented Nov 5, 2024 •

edited

Loading

ricardomaraschini commented Nov 5, 2024

banjoh left a comment

banjoh commented Nov 6, 2024

feat: implement collector and analyser for network namespace connectivity #1670

feat: implement collector and analyser for network namespace connectivity #1670

Conversation

ricardomaraschini commented Oct 31, 2024 • edited Loading

Description, Motivation and Context

Notes

Workflow

Optional Configurations

Message templating

Checklist

Does this PR introduce a breaking change?

banjoh left a comment • edited Loading

Choose a reason for hiding this comment

ricardomaraschini commented Nov 5, 2024 • edited Loading

banjoh commented Nov 5, 2024 • edited Loading

ricardomaraschini commented Nov 5, 2024

banjoh left a comment

Choose a reason for hiding this comment

banjoh commented Nov 6, 2024

ricardomaraschini commented Oct 31, 2024 •

edited

Loading

banjoh left a comment •

edited

Loading

ricardomaraschini commented Nov 5, 2024 •

edited

Loading

banjoh commented Nov 5, 2024 •

edited

Loading