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

Use different containerd sock address in tests #9415

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kiashok
Copy link
Contributor

@kiashok kiashok commented Nov 22, 2023

If the containerd/containerd/defaults.DefaultAddress is used , it causes failures if containerd is already running on the default address and the tests are run without the 'no-daemon' flag.
It would be good to use a different containerd sock address for integration tests

@dcantah
Copy link
Member

dcantah commented Nov 22, 2023

The address is configurable via the flag, can't we just pass the flag when we run our tests? (or locally when a user is running the tests they can provide the flag)

@kiashok
Copy link
Contributor Author

kiashok commented Nov 23, 2023

The address is configurable via the flag, can't we just pass the flag when we run our tests? (or locally when a user is running the tests they can provide the flag)

Hmm isn't the -address flag for the tests mostly to know which sock address to connect to? It would be nice to have parity in how the test daemon is started when the '--no-daemon' flag is not specified. That is start on a different non-default sock address for the tests. like npipe:\containerd-containerd-test
Was this change intended to make it to 1.6 as well - f318947 ? Is there any reason we want to have it only in main and 1.7?

@kiashok
Copy link
Contributor Author

kiashok commented Nov 23, 2023

The address is configurable via the flag, can't we just pass the flag when we run our tests? (or locally when a user is running the tests they can provide the flag)

Hmm isn't the -address flag for the tests mostly to know which sock address to connect to? It would be nice to have parity in how the test daemon is started when the '--no-daemon' flag is not specified. That is start on a different non-default sock address for the tests. like npipe:\containerd-containerd-test Was this change intended to make it to 1.6 as well - f318947 ? Is there any reason we want to have it only in main and 1.7?

Also, tests like the following try to get the ttrpc address from the "address" set in TestMain() which is a unique test only sock address in 1.6 which makes more sense:

@kiashok
Copy link
Contributor Author

kiashok commented Nov 23, 2023

If the test address is now same as the default address for the platform and if this is already open, tests would fail while trying to open same pipe

@kiashok kiashok force-pushed the fixIntegrationClientAddr branch 2 times, most recently from 40054df to 18a48d8 Compare November 23, 2023 04:15
@dcantah
Copy link
Member

dcantah commented Nov 23, 2023

Hmm isn't the -address flag for the tests mostly to know which sock address to connect to?

It may be (I'm reviewing on mobile so didn't do great due diligence 😅). The description made it seem like thats the addr it'd listen on if they were launching the daemon

@ruiwen-zhao
Copy link
Member

/retest

Signed-off-by: Kirtana Ashok <kiashok@microsoft.com>
@k8s-ci-robot
Copy link

PR needs rebase.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants