Skip to content

Add configurable timeout to netcat calls with 10s default #78370

Merged
gongomgra merged 1 commit intobitnami:mainfrom
mati865:add-timeout-to-netcat
Mar 13, 2025
Merged

Add configurable timeout to netcat calls with 10s default #78370
gongomgra merged 1 commit intobitnami:mainfrom
mati865:add-timeout-to-netcat

Conversation

@mati865
Copy link
Contributor

@mati865 mati865 commented Feb 26, 2025

Description of the change

Without any timeout, netcat can get stuck forever (until Kubernetes kills unhealthy Pod) when configured address and port combination is wrong, or the connection gets stuck.
For example nc -z bitnami.com 12345 doesn't seem to ever finish.

Benefits

In the case when an attempt gets stuck, the timeout will allow further attempts to be performed, instead of waiting until Pod is killed.

Possible drawbacks

When the timeout is too short, valid configurations might falsely show as broken. Thus, I chose 10s which should be long enough.

Applicable issues

Additional information

Default 10s timeout is very conservative and should not cause spurious failures, users are advised to shorten it according to their needs.

@github-actions github-actions bot added the triage Triage is needed label Feb 26, 2025
@github-actions github-actions bot requested a review from javsalgar February 26, 2025 12:48
@mati865 mati865 force-pushed the add-timeout-to-netcat branch from d63aada to 3d66450 Compare February 26, 2025 13:04
@javsalgar javsalgar added verify Execute verification workflow for these changes in-progress labels Feb 27, 2025
@github-actions github-actions bot removed the triage Triage is needed label Feb 27, 2025
@github-actions github-actions bot removed the request for review from javsalgar February 27, 2025 09:51
@github-actions github-actions bot requested a review from gongomgra February 27, 2025 09:51
@gongomgra
Copy link
Contributor

@mati865 thanks for your contribution! What do you think about adding a new parameter to the <asset>-env.sh file for this new setting?

@mati865
Copy link
Contributor Author

mati865 commented Mar 3, 2025

@gongomgra I considered this from the beginning, but opted for an easy fix/workaround to avoid spending too much time investigating the scripts' interdependencies.

If this is just a matter of adding it to one more script per chart, I can certainly look into that.

@gongomgra
Copy link
Contributor

@mati865 you can take a look to a recent PR for pgbouncer adding a new environment variable definition at #75939

@mati865 mati865 force-pushed the add-timeout-to-netcat branch from 3d66450 to fe39aca Compare March 4, 2025 14:30
Without any timeout, netcat can get stuck forever (until Kubernetes
kills unhealthy Pod) when configured address and port combination is
wrong, or the connection gets stuck.
For example `nc -z bitnami.com 12345` doesn't seem to ever finish.

In the case when an attempt gets stuck, the timeout will allow further
attempts to be performed, instead of waiting until Pod is killed.

Signed-off-by: Mateusz Mikuła <oss@mateuszmikula.dev>
@mati865 mati865 force-pushed the add-timeout-to-netcat branch from fe39aca to f465deb Compare March 4, 2025 14:31
@mati865 mati865 changed the title Add 10s timeout to netcat calls Add configurable timeout to netcat calls with 10s default Mar 4, 2025
@mati865
Copy link
Contributor Author

mati865 commented Mar 4, 2025

@gongomgra thank you for the pointers, updated commit, description and title. The diff of the changes since your initial review is available at: https://github.com/bitnami/containers/compare/3d664506660480823bbcc01c6582e3256f038a0b..fe39acaf1defd3c57b5d7a3c9d287d32d7315447
The second force-push is for commit name because I forgot to update it.

I have used 10s as the default value, so this change is backwards compatible.

Copy link
Contributor

@gongomgra gongomgra left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@gongomgra gongomgra merged commit ee01f55 into bitnami:main Mar 13, 2025
13 checks passed
@mati865 mati865 deleted the add-timeout-to-netcat branch March 13, 2025 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

solved verify Execute verification workflow for these changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants