ci: add tests for migration to CiliumEndpointSlice #32268
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
github-actions
bot
added
the
kind/community-contribution
This comment was marked as outdated.
This comment was marked as outdated.
joestringer
reviewed
Apr 30, 2024
jshr-w
force-pushed
the
jshr/ceptocesupgrade
branch
3 times, most recently
from
c7a29a6
to
018e95a
Compare
thorn3r
requested changes
May 15, 2024
There was a problem hiding this comment.
Overall this looks great, thanks for taking this on! I left a few in-line suggestions. You may also want to add steps for merging the sysdumps together and updating the commit status (I believe this is needed in order to prevent skipped from reporting as failed).
Could you also squash your commits?
|
Just realized I jumped the gun a bit and this is still marked as draft 😅 Feel free to retag me for review when it's ready |
No problem! Thank you for your feedback, I'll go through them and update accordingly :) |
This comment was marked as resolved.
This comment was marked as resolved.
maintainer-s-little-helper
bot
added
the
dont-merge/needs-sign-off
jshr-w
force-pushed
the
jshr/ceptocesupgrade
branch
from
d815d83
to
6e133bc
Compare
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-sign-off
jshr-w
force-pushed
the
jshr/ceptocesupgrade
branch
from
6e133bc
to
5927ca1
Compare
|
The connectivity test failures for client-egress-l7/pod-to-world/http-to-one.one.one.one, client-egress-l7-named-port/pod-to-world/http-to-one.one.one.one, pod-to-ingress-service/pod-to-ingress-service observed seem to be related to the Cilium Agent restart, rather than migration to CES. -> #32611 |
jshr-w
force-pushed
the
jshr/ceptocesupgrade
branch
4 times, most recently
from
c79c251
to
d92778b
Compare
|
Hi @thorn3r, I think I'm ready for a re-review! :) |
thorn3r
reviewed
May 24, 2024
joestringer
reviewed
May 24, 2024
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-release-note-label
joestringer
reviewed
May 28, 2024
jshr-w
force-pushed
the
jshr/ceptocesupgrade
branch
3 times, most recently
from
1365756
to
538b222
Compare
joestringer
reviewed
May 29, 2024
jshr-w
force-pushed
the
jshr/ceptocesupgrade
branch
from
0851d09
to
e185587
Compare
jshr-w
force-pushed
the
jshr/ceptocesupgrade
branch
3 times, most recently
from
5c7340a
to
b0e0226
Compare
|
Thanks everyone for all the help and feedback :) I think I'm ready for another review now (cc @thorn3r) |
jshr-w
force-pushed
the
jshr/ceptocesupgrade
branch
2 times, most recently
from
63390c4
to
3ae3c8a
Compare
This commit adds CI to test that the migration from CiliumEndpoint to CiliumEndpointSlice does not disturb long-lived connections. A Kind cluster is set up without CiliumEndpointSlice enabled. Long-lived connections are set up. Then, CES is enabled, the operator is restarted and then the agent, after the CES CRD is created. Then, the connectivity test is run to ensure long-lived connections were not broken. Signed-off-by: jshr-w <shjayaraman@microsoft.com>
jshr-w
force-pushed
the
jshr/ceptocesupgrade
branch
from
3ae3c8a
to
d97bd3f
Compare
|
/test |
|
@viktor-kurchenko @nbusseneau Hi, I think this PR is blocked on codeowner review from ci-structure, would you be able to help with this please? |
viktor-kurchenko
approved these changes
Jun 12, 2024
maintainer-s-little-helper
bot
added
the
ready-to-merge
Comment on lines
+105
to
+110
| uses: ./.github/actions/conn-disrupt-test | ||
| with: | ||
| job-name: ces-enable | ||
| operation-cmd: | | ||
| kubectl patch -n kube-system configmap cilium-config --type merge --patch '{"data":{"enable-cilium-endpoint-slice":"true"}}' | ||
|
|
There was a problem hiding this comment.
I believe the usage of conn-disrupt-test needs adjustment for the changes in #32930.
There was a problem hiding this comment.
Yep, just mentioned the same: https://cilium.slack.com/archives/C069SNDMLA2/p1718286768251579
There was a problem hiding this comment.
The path filter should also be modified so that these changed are detected immediately.
jshr-w
added a commit
to jshr-w/cilium
that referenced
this pull request
Jun 13, 2024
migration test to fail. This PR updates the test to work with the updated test format. The inconsistency was not caught because of the path filters used in the test, so the path filters have been updated to exclude only Documentation/ and test/. Fixes: cilium#32268 Signed-off-by: jshr-w <shjayaraman@microsoft.com>
jshr-w
added a commit
to jshr-w/cilium
that referenced
this pull request
Jun 13, 2024
PR cilium#32930 introduced a change to the conn-disrupt test that caused this migration test to fail. This PR updates the test to work with the updated test format. The inconsistency was not caught because of the path filters used in the test, so the path filters have been updated to exclude only Documentation/ and test/. Fixes: cilium#32268 Signed-off-by: jshr-w <shjayaraman@microsoft.com>
jshr-w
added a commit
to jshr-w/cilium
that referenced
this pull request
Jun 13, 2024
PR cilium#32930 introduced a change to the conn-disrupt test that caused this migration test to fail. This PR updates the test to work with the updated test format. The inconsistency was not caught because of the path filters used in the test, so the path filters have been updated to exclude only Documentation/ and test/. Fixes: cilium#32268 Signed-off-by: jshr-w <shjayaraman@microsoft.com>
8 tasks
jshr-w
added a commit
to jshr-w/cilium
that referenced
this pull request
Jun 14, 2024
PR cilium#32930 introduced a change to the conn-disrupt test that caused this migration test to fail. This PR updates the test to work with the updated test format. The inconsistency was not caught because of the path filters used in the test, so the path filters have been updated to exclude only Documentation/ and test/. Fixes: cilium#32268 Signed-off-by: jshr-w <shjayaraman@microsoft.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 15, 2024
PR #32930 introduced a change to the conn-disrupt test that caused this migration test to fail. This PR updates the test to work with the updated test format. The inconsistency was not caught because of the path filters used in the test, so the path filters have been updated to exclude only Documentation/ and test/. Fixes: #32268 Signed-off-by: jshr-w <shjayaraman@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds CI to validate the migration path from CiliumEndpoint to CiliumEndpointSlice, by checking that the migration does not affect connectivity on a migrated cluster.
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXXline if the commit addresses a particularGitHub issue.
Fixes: <commit-id>tag, thenplease add the commit author[s] as reviewer[s] to this issue.
Fixes: #32057