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

Add service and node discovery to test resources #127

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

matmerr
Copy link
Contributor

@matmerr matmerr commented Oct 21, 2022

Add some basic wiring to create/destroy services as part of test step, and target desting datapath from pod matrix to nodes, specifically via nodeport.

Known TODOs:

  • Table showing datapath from pod->node is incomplete
  • Including nodes in truth table/simulated runs needs to be built
  • Is connection from pod x/a, to service that has podselector pod=a, considered loopback? This traffic is allowed with a default-deny-all on cilium, although the generated netpol expectation doesn't factor this in and will fail the scenario with these allowed connections

@mattfenwick
Copy link
Owner

Is connection from pod x/a, to service that has podselector pod=a, considered loopback?

Good question, I couldn't find a definitive answer to this one. So I punted, adding the IgnoreLoopback flag 😱

In general, the behavior seems to be undefined here and varies from CNI to CNI

@mattfenwick
Copy link
Owner

mattfenwick commented Oct 25, 2022

Table showing datapath from pod->node is incomplete

Do you have a mockup of what this table should look like? Doesn't have to be generated by cyclonus, if you can just show me a picture then I think everything else in this PR will become clear to me !

@mattfenwick
Copy link
Owner

thanks @matmerr this is exciting ! overall I'm on board with this and it's a great feature to add !

looks like there's still some TODOs you're planning to clear up, so I'll wait to give a more thorough review (spoiler: code already looks good 👍 )

on my end, for some reason I can't run the CNI-specific tests on this PR -- guess I missed some config so I'll look into enabling that

@matmerr
Copy link
Contributor Author

matmerr commented Jan 17, 2023

Do you have a mockup of what this table should look like? Doesn't have to be generated by cyclonus, if you can just show me a picture then I think everything else in this PR will become clear to me !

tbh I don't have an easy solution in mind, just expressing connectivity between nodes and pods in the same table is clunky 😕

Example:

Actual vs expected (last round):
+--------------------+--------------------+-------------+--------------+-----+-----+-----+-----+-----+-----+-----+-----+-----+
|                    | KIND-CONTROL-PLANE | KIND-WORKER | KIND-WORKER2 | X/A | X/B | X/C | Y/A | Y/B | Y/C | Z/A | Z/B | Z/C |
+--------------------+--------------------+-------------+--------------+-----+-----+-----+-----+-----+-----+-----+-----+-----+
| kind-control-plane | .                  | .           | .            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| kind-worker        | .                  | .           | .            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| kind-worker2       | .                  | .           | .            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| x/a                | X                  | X           | X            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| x/b                | X                  | X           | X            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| x/c                | X                  | X           | X            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| y/a                | X                  | X           | X            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| y/b                | X                  | X           | X            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| y/c                | X                  | X           | X            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| z/a                | X                  | X           | X            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| z/b                | X                  | X           | X            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
| z/c                | X                  | X           | X            | .   | .   | .   | .   | .   | .   | .   | .   | .   |
+--------------------+--------------------+-------------+--------------+-----+-----+-----+-----+-----+-----+-----+-----+-----+

A lot of the remaining work is around visualization, definitely needs some mockups but core datapath is usable for validating with external tools. Best to omit the loadbalancer tag as a default from generator until vis is locked down

@mattfenwick
Copy link
Owner

looking good @matmerr !! sorry for the delay getting back to you on this

how close is this to ready to merge ? I'll take another look through but overall it seems like this is getting pretty close ?

@matmerr
Copy link
Contributor Author

matmerr commented Mar 9, 2023

looking good @matmerr !! sorry for the delay getting back to you on this

how close is this to ready to merge ? I'll take another look through but overall it seems like this is getting pretty close ?

In the current state the service/node creation exists, but the ux around test case evaluation and table presentation is still missing. Tagging in @huntergregory who is currently working in this scope.

@mattfenwick
Copy link
Owner

hi @huntergregory checking in , how is this going ? lmk if you're hitting any problems !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants