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

Minor improvements concerning the tunnel feature determination for connectivity tests #2247

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

giorio94
Copy link
Member

Please refer to the individual commit messages for additional details.

So that it is easier to reuse the same logic from external hooks.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Let's improve the consistency in the determination of the tunnel feature,
and ensure that also in case of native routing mode we retrieve the correct
tunnel protocol. Indeed, certain features (e.g., egress gateway) still rely
on the configured tunnel protocol even if the main routing mode is native.
While being there, let's also increase the test coverage of this logic.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94
Copy link
Member Author

Kind / Kind Installation and Connectivity Test failed due to #2204

Copy link
Member

@asauber asauber left a comment

Choose a reason for hiding this comment

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

These changes look good.

That being said, why is this not using the more global Feature Set parser? https://github.com/cilium/cilium-cli/blob/main/utils/features/features.go#L219

Nevermind, I see that there's a separate codepath based on Cilium version.

@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 Jan 23, 2024
@tklauser tklauser merged commit 4698dd7 into cilium:main Jan 23, 2024
13 checks passed
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.

None yet

3 participants