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

Drop cilium endpoint verification. #2205

Merged
merged 2 commits into from
Jan 9, 2024
Merged

Drop cilium endpoint verification. #2205

merged 2 commits into from
Jan 9, 2024

Conversation

viktor-kurchenko
Copy link
Contributor

@viktor-kurchenko viktor-kurchenko commented Dec 21, 2023

Looks like Cilium endpoint verification is no longer needed because Cilium should be already provisioned correctly in the environment before deploying the connectivity apps, and these tests already validate that the k8s deployment resource is ready before continuing.

This should also unblock connectivity tests for Customers that have disableEndpointCRD: "true" property configured.

connectivity/check/wait.go Outdated Show resolved Hide resolved
internal/cli/cmd/connectivity.go Outdated Show resolved Hide resolved
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.

Looks great, thanks for the updates

@viktor-kurchenko viktor-kurchenko changed the title Skip cilium endpoint verification flag. Skip cilium endpoint verification. Dec 27, 2023
@viktor-kurchenko viktor-kurchenko removed the request for review from michi-covalent January 5, 2024 07:37
@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 5, 2024
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

I have a concern below about the behaviour of the testsuite with this proposal.

connectivity/check/deployment.go Outdated Show resolved Hide resolved
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 5, 2024
@joestringer joestringer dismissed their stale review January 5, 2024 21:30

No need to block on my feedback. We can discuss a little more but I'm fine with the solution in principle.

@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 8, 2024
@joestringer
Copy link
Member

I think it would be useful to include the context in the description that: This is no longer needed because Cilium should be already provisioned correctly in the environment before deploying the connectivity apps, and these tests already validate that the k8s deployment resource is ready before continuing.

@viktor-kurchenko viktor-kurchenko changed the title Skip cilium endpoint verification. Drop cilium endpoint verification. Jan 9, 2024
@tklauser tklauser merged commit 28a5c9a into main Jan 9, 2024
13 checks passed
@tklauser tklauser deleted the pr/vk/endpoint/crd/skip branch January 9, 2024 09:48
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