-
Notifications
You must be signed in to change notification settings - Fork 763
Add port monitoring test script #4077
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
Conversation
814c0dc to
a5c15b6
Compare
|
|
||
| load "../helpers/load" | ||
|
|
||
| : "${TEMPLATE:=default}" # Alternative: "docker" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be part of test-templates ?
Anyway we should establish the criteria to clarify what should be in test-templates and what else should be in bats.
Maybe we should also consider rewriting test-templates in bats.
https://lima-vm.io/docs/dev/testing/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should also consider rewriting test-templates in bats.
I think we should, but it will take some time. Until then we can run them in parallel. New tests should be in BATS, I think, if possible with reasonable effort.
I don't have the time to rework the whole test-suite at once, but will add a little bit whenever I have some extra time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be part of
test-templates?
The reason this particular test has a TEMPLATES variable is so that you can use it to test both containerd and docker, as the 2 container engines may have different port binding mechanisms, and they should be tested both. There is no point in running this test with all the templates.
Potentially it could be extended to run with rootful containerd/docker, but I didn't think that was necessary.
When we have more tests we can figure out a set of tags to run different groups under different scenarios, but I think it is still too early to bother with that: https://bats-core.readthedocs.io/en/stable/writing-tests.html#tagging-tests
It verifies that when a container is destroyed, its ports can be reused immediately and are not subject to being freed by a polling loop. See lima-vm#4066. Signed-off-by: Jan Dubois <jan.dubois@suse.com>
a5c15b6 to
3234d11
Compare
AkihiroSuda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
It verifies that when a container is destroyed, its ports can be reused immediately and are not subject to being freed by a polling loop.
I wrote this to validate #4066, but I think we should keep things like this in the repo, just in case we need them again. They also serve as samples to write other similar tests in the future.
It should not be run in CI; it is not totally reliable: there is always a chance that the server will take longer to actually respond to requests after opening the port. The test works around it by retrying once on curl exit code 52, but I have it seen fail once by refusing to connect.
Anyways, running the test on
masterfails:I've been running on the #4066 PR branch multiple times, with both
containerdanddocker, and it seems to be passing regularly: