test/system: For pasta port forwarding tests don't bind socat server #24064
Conversation
The various pasta port forwarding tests run a socat server inside a container, then connect to it from a socat client on the host. Currently we have the server bind to the same specific address within the container as we connect to on the host. That's not quite what we want. For "tap" tests where the traffic goes over pasta's L2 link to the container it's fine, though unnecessary. For "loopback" tests where traffic is forwarded by pasta at the L4 socket level, however, it's not quite right. In this case the address used is either 127.0.0.1 or ::. That's correct and as needed for the host side address we're connecting to. However on the container side, this only works because of an odd and arguably undesirable behaviour of pasta: we use the fact that we have an L4 socket within the container to make such "spliced" L4 connections appear as if they come from loopback within the container. A container will generally expect it's loopback address to be only accessible from within the container, and this odd behaviour may be changed in pasta in future. In any case, the binding of the container side server is unnecessary, so simply remove it. Link: containers#24045 Signed-off-by: David Gibson <[email protected]>
openshift-ci
bot
added
the
do-not-merge/release-note-label-needed
|
@dgibson: GitHub didn't allow me to request PR reviews from the following users: adelton. Note that only containers members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this: 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-sigs/prow repository. |
openshift-ci
bot
added
release-note-none
and removed
do-not-merge/release-note-label-needed
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgibson, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Well, again, this didn't work by mistake: I purposefully bound socat's sockets here to check exactly this behaviour as I intended it. Anyway, it makes sense to change it, of course, and we'll need this to be merged in Podman before we can merge the change for #24045 in passt, so (assuming it does something): LGTM |
Ok, that wasn't actually obvious to me. I assumed it had been copied from the equivalent logic in the passt test suite, and that that had been copied from the outbound cases which bind to
|
|
/lgtm |
The various pasta port forwarding tests run a socat server inside a container, then connect to it from a socat client on the host. Currently we have the server bind to the same specific address within the container as we connect to on the host.
That's not quite what we want. For "tap" tests where the traffic goes over pasta's L2 link to the container it's fine, though unnecessary. For "loopback" tests where traffic is forwarded by pasta at the L4 socket level, however, it's not quite right. In this case the address used is either 127.0.0.1 or ::. That's correct and as needed for the host side address we're connecting to. However on the container side, this only works because of an odd and arguably undesirable behaviour of pasta: we use the fact that we have an L4 socket within the container to make such "spliced" L4 connections appear as if they come from loopback within the container. A container will generally expect it's loopback address to be only accessible from within the container, and this odd behaviour may be changed in pasta in future.
In any case, the binding of the container side server is unnecessary, so simply remove it.
Link: #24045
Does this PR introduce a user-facing change?