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

Listener pod ncat fix #1773

Closed
wants to merge 5 commits into from

Conversation

vthapar
Copy link
Contributor

@vthapar vthapar commented Dec 17, 2024

No description provided.

skitt and others added 4 commits December 16, 2024 17:03
This ensures that the Busybox compatible nc script is used instead of
ncat.

Signed-off-by: Stephen Kitt <[email protected]>
Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.29.0 to 0.31.0.
- [Commits](golang/crypto@v0.29.0...v0.31.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
`ncat -w` defaults to seconds but gives a warning when unit
not explicitly specified. This causes errors in msg exchanged
between listener and connect pods.

Explicitly specify unit as seconds when passing arg to `ncat -w`
to avoid the warning and resulting errors.

Signed-off-by: Vishal Thapar <[email protected]>
@submariner-bot
Copy link

🤖 Created branch: z_pr1773/vthapar/listener-pod-ncat-fix

`-s` and `-p` flags are not supported with listen mode
with ncat.

Note: This should be handled by `nc` script but doesn't seem
to be working. Once it works, revert this change so it works
with older versions of nettest/ncat.

Signed-off-by: Vishal Thapar <[email protected]>
Copy link
Member

@skitt skitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we’re switching to ncat syntax, it’s safer to invoke ncat directly — otherwise in the next PR after this, once images are rebuilt with the nc compatibility wrapper first on the path, this will fail again.

@@ -278,7 +278,7 @@ func (np *NetworkPod) buildTCPCheckListenerPod() {
"for i in $(seq 1 $BUFS_NUM);" +
" do echo [dataplane] listener says $SEND_STRING;" +
" done" +
" | nc -w $CONN_TIMEOUT -l -v -p $LISTEN_PORT -s 0.0.0.0 >/dev/termination-log 2>&1",
" | nc -w ${CONN_TIMEOUT}s -l -v 0.0.0.0 $LISTEN_PORT >/dev/termination-log 2>&1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
" | nc -w ${CONN_TIMEOUT}s -l -v 0.0.0.0 $LISTEN_PORT >/dev/termination-log 2>&1",
" | ncat -w ${CONN_TIMEOUT}s -l -v 0.0.0.0 $LISTEN_PORT >/dev/termination-log 2>&1",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to see if it fails or not. If it works, I'd revert to nc syntax so it works with older images, as you pointed out earlier. It will also be a good way to see if script works or not.

@@ -328,7 +328,7 @@ func (np *NetworkPod) buildTCPCheckConnectorPod() {
"for in in $(seq 1 $BUFS_NUM);" +
" do echo [dataplane] connector says $SEND_STRING; done" +
" | for i in $(seq $CONN_TRIES);" +
" do if nc -v $REMOTE_IP $REMOTE_PORT -w $CONN_TIMEOUT;" +
" do if nc -v $REMOTE_IP $REMOTE_PORT -w ${CONN_TIMEOUT}s;" +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
" do if nc -v $REMOTE_IP $REMOTE_PORT -w ${CONN_TIMEOUT}s;" +
" do if ncat -v $REMOTE_IP $REMOTE_PORT -w ${CONN_TIMEOUT}s;" +

@vthapar vthapar closed this Dec 17, 2024
@submariner-bot
Copy link

🤖 Closed branches: [z_pr1773/vthapar/listener-pod-ncat-fix]

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.

3 participants