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

connectivity: test namespace suffix #2680

Merged
merged 1 commit into from
Jul 15, 2024
Merged

connectivity: test namespace suffix #2680

merged 1 commit into from
Jul 15, 2024

Conversation

viktor-kurchenko
Copy link
Contributor

@viktor-kurchenko viktor-kurchenko commented Jul 13, 2024

Always suffix test namespace with concurrent group number.
Unhide --test-concurrency input param.

Thanks @asauber and @michi-covalent for the idea.
However I think these changes might be breakable for users who rely on test namespace (e.g.: in CI or other automation stuff). WDYT?

Always suffix test namespace with concurrent group number.
Unhide `--test-concurrency` input param.

Signed-off-by: viktor-kurchenko <[email protected]>
Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

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

i think it's ok we'll release note it.

Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 15, 2024
@michi-covalent michi-covalent merged commit 66599f2 into main Jul 15, 2024
13 checks passed
@michi-covalent michi-covalent deleted the pr/vk/test/ns branch July 15, 2024 21:09
michi-covalent added a commit to cilium/cilium that referenced this pull request Aug 1, 2024
cilium/cilium-cli#2680 changed the behavior of
--test-namespace flag. It's now treated as a prefix, and the namespaces
always contain "-$index" suffix even if --test-concurrency is set to 1.
Instead of hardcoding the namespace, use app.kubernetes.io/name label to
find the test namespace.

Signed-off-by: Michi Mutsuzaki <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Aug 1, 2024
cilium/cilium-cli#2680 changed the behavior of
--test-namespace flag. It's now treated as a prefix, and the namespaces
always contain "-$index" suffix even if --test-concurrency is set to 1.
Instead of hardcoding the namespace, use app.kubernetes.io/name label to
find the test namespace.

Signed-off-by: Michi Mutsuzaki <[email protected]>
nbusseneau pushed a commit to cilium/cilium that referenced this pull request Aug 3, 2024
[ upstream commit b189c57 ]

cilium/cilium-cli#2680 changed the behavior of
--test-namespace flag. It's now treated as a prefix, and the namespaces
always contain "-$index" suffix even if --test-concurrency is set to 1.
Instead of hardcoding the namespace, use app.kubernetes.io/name label to
find the test namespace.

Signed-off-by: Michi Mutsuzaki <[email protected]>
nbusseneau pushed a commit to cilium/cilium that referenced this pull request Aug 3, 2024
[ upstream commit b189c57 ]

cilium/cilium-cli#2680 changed the behavior of
--test-namespace flag. It's now treated as a prefix, and the namespaces
always contain "-$index" suffix even if --test-concurrency is set to 1.
Instead of hardcoding the namespace, use app.kubernetes.io/name label to
find the test namespace.

Signed-off-by: Michi Mutsuzaki <[email protected]>
Signed-off-by: Nicolas Busseneau <[email protected]>
nbusseneau pushed a commit to cilium/cilium that referenced this pull request Aug 3, 2024
[ upstream commit b189c57 ]

cilium/cilium-cli#2680 changed the behavior of
--test-namespace flag. It's now treated as a prefix, and the namespaces
always contain "-$index" suffix even if --test-concurrency is set to 1.
Instead of hardcoding the namespace, use app.kubernetes.io/name label to
find the test namespace.

Signed-off-by: Michi Mutsuzaki <[email protected]>
nbusseneau pushed a commit to cilium/cilium that referenced this pull request Aug 3, 2024
[ upstream commit b189c57 ]

cilium/cilium-cli#2680 changed the behavior of
--test-namespace flag. It's now treated as a prefix, and the namespaces
always contain "-$index" suffix even if --test-concurrency is set to 1.
Instead of hardcoding the namespace, use app.kubernetes.io/name label to
find the test namespace.

Signed-off-by: Michi Mutsuzaki <[email protected]>
Signed-off-by: Nicolas Busseneau <[email protected]>
nbusseneau pushed a commit to cilium/cilium that referenced this pull request Aug 5, 2024
[ upstream commit b189c57 ]

cilium/cilium-cli#2680 changed the behavior of
--test-namespace flag. It's now treated as a prefix, and the namespaces
always contain "-$index" suffix even if --test-concurrency is set to 1.
Instead of hardcoding the namespace, use app.kubernetes.io/name label to
find the test namespace.

Signed-off-by: Michi Mutsuzaki <[email protected]>
Signed-off-by: Nicolas Busseneau <[email protected]>
nbusseneau pushed a commit to cilium/cilium that referenced this pull request Aug 6, 2024
[ upstream commit b189c57 ]

cilium/cilium-cli#2680 changed the behavior of
--test-namespace flag. It's now treated as a prefix, and the namespaces
always contain "-$index" suffix even if --test-concurrency is set to 1.
Instead of hardcoding the namespace, use app.kubernetes.io/name label to
find the test namespace.

Signed-off-by: Michi Mutsuzaki <[email protected]>
gandro pushed a commit to cilium/cilium that referenced this pull request Aug 6, 2024
[ upstream commit b189c57 ]

cilium/cilium-cli#2680 changed the behavior of
--test-namespace flag. It's now treated as a prefix, and the namespaces
always contain "-$index" suffix even if --test-concurrency is set to 1.
Instead of hardcoding the namespace, use app.kubernetes.io/name label to
find the test namespace.

Signed-off-by: Michi Mutsuzaki <[email protected]>
Signed-off-by: Nicolas Busseneau <[email protected]>
@jpayne3506
Copy link

This did end up breaking our CI/CD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants