Add commatrix tests #28634
Closed
Add commatrix tests #28634
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
liornoy
force-pushed
the
add-commatrix-tests
branch
2 times, most recently
from
fae4daa
to
c7f1dd5
Compare
yanjoseph
reviewed
Mar 4, 2024
| g "github.com/onsi/ginkgo/v2" | ||
| o "github.com/onsi/gomega" | ||
|
|
||
| "github.com/liornoy/node-comm-lib/commatrix" |
There was a problem hiding this comment.
any discussion made WRT a suitable host for the code ?
There was a problem hiding this comment.
we're considering moving it to openshift/library-go.
this is pointing to my own repo temporarily just to see first that the library does what we want it to do.
|
Job Failure Risk Analysis for sha: c7f1dd5
|
liornoy
force-pushed
the
add-commatrix-tests
branch
from
c7f1dd5
to
e8143e7
Compare
|
/retest |
|
Job Failure Risk Analysis for sha: e8143e7
|
liornoy
force-pushed
the
add-commatrix-tests
branch
2 times, most recently
from
6f97a9e
to
782a7a8
Compare
|
Job Failure Risk Analysis for sha: 782a7a8
|
liornoy
force-pushed
the
add-commatrix-tests
branch
from
782a7a8
to
f62655c
Compare
|
Job Failure Risk Analysis for sha: f62655c
|
liornoy
force-pushed
the
add-commatrix-tests
branch
2 times, most recently
from
1bb2289
to
8a5422a
Compare
|
Job Failure Risk Analysis for sha: 8a5422a
|
liornoy
force-pushed
the
add-commatrix-tests
branch
from
8a5422a
to
a45a87b
Compare
|
Job Failure Risk Analysis for sha: a45a87b
|
|
|
||
| nodesComDeatils := []types.ComDetails{} | ||
| for _, n := range nodesList.Items { | ||
| cds, err := ssutil.CreateComDetailsFromNode(oc, &n, tcpfile, udpfile) |
There was a problem hiding this comment.
To be more clear on what this is doing let's consider changing to ss.ToComDetails or ss.ConvertToComDetails
| cs, err := clientutil.New(kubeconfig) | ||
| o.Expect(err).ToNot(o.HaveOccurred()) | ||
|
|
||
| g.By("fetching open ports on the cluster") |
There was a problem hiding this comment.
Let's be more clear here, this is fetching open ports on the nodes with ss
liornoy
force-pushed
the
add-commatrix-tests
branch
from
a45a87b
to
2ddcc2b
Compare
openshift-merge-robot
added
the
needs-rebase
Add a test for the 'comMatrix' (Communication Matrix) library. This library generates a description of the ports to which the cluster listens for ingress traffic. The test currently validates that the Communication Matrix generated by the library covers all the open ports on the nodes. It does so by comparing the output of the 'ss' command on the nodes. Note: the test also manully creates endpointslices that represent host services that don't have one natively. Some of them are static and known ports. See more here: https://github.com/openshift/enhancements/blob/master/dev-guide/host-port-registry.md Others are host services with random ports that we create the endpointslices from the given "ss" output for that specific service (rpc, and crio, ovnkube). The test create the relevant artifacts under the "commatrix" dir. the artifacts that can be found are: 1. The communicatiom matrix. 2. The ss entries we compare against. 3. Diff of 1, and 2. 4. Diff of 2, and 1. 5. Raw tcp 'ss' output on nodes. 6. Raw udp 'ss' output on nodes. Signed-off-by: Lior Noy <[email protected]>
liornoy
force-pushed
the
add-commatrix-tests
branch
from
2ddcc2b
to
ea5c916
Compare
openshift-merge-robot
removed
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: liornoy The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@liornoy: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Job Failure Risk Analysis for sha: ea5c916
|
| if err != nil { | ||
| return nil, fmt.Errorf("failed writing to file: %s", err) | ||
| } | ||
| _, err = tcpfile.Write([]byte(fmt.Sprintf("node: %s\n%s", node.Name, strings.Join(ssOutFilteredUDP, "\n")))) |
There was a problem hiding this comment.
Both UDP and TCP are printed to TCP file
|
closing this in favor of the updated test #28706 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Coverage tests,
Please only consider the second commit: "Refactor commatrix tests"
The first one is kept for reference and will be squashed before merge.
Also - the node-comm-lib will eventually be moved to a more suitable place
e.g. openshift/library-go