Move the connectivity package from cilium-cli#31077
Move the connectivity package from cilium-cli#31077michi-covalent wants to merge 3 commits intomainfrom
Conversation
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
|
/test |
michi-covalent
added
the
release-note/minor
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-release-note-label
michi-covalent
force-pushed
the
pr/michi/majestic-ketchup
branch
6 times, most recently
from
50ff439 to
9a8c261
Compare
|
/test |
michi-covalent
force-pushed
the
pr/michi/majestic-ketchup
branch
7 times, most recently
from
a97ccb8 to
0c1e088
Compare
|
/test |
1 similar comment
|
/test |
michi-covalent
force-pushed
the
pr/michi/majestic-ketchup
branch
9 times, most recently
from
7de93c7 to
9ed4d53
Compare
|
/test |
7 similar comments
|
/test |
|
/test |
|
/test |
|
/test |
|
/test |
|
/test |
|
/test |
|
/test |
1 similar comment
|
/test |
|
removing the temporary commit and marking it ready for review |
|
/test |
Move the connectivity package from cilium/cilium-cli repo. The main goal here is to make "cilium connectivity test" use the local package so that you can develop new features and add tests for the new feature without having to deal with multiple repositories. There is a corresponding pull request in the cilium-cli repo [^1] to import the connectivity package from the cilium repo. See CFP-25694 [^2] for additional details. The following files are direct copies from [email protected] with some minor adjustments like renaming package paths and fixing lint errors: - cilium-cli/cli/connectivity.go [^3] - cilium-cli/connectivity/* [^4] - cilium-cli/cmd/cilium/main.go [^5] [^1]: cilium/cilium-cli#2355 [^2]: cilium/design-cfps#9 [^3]: https://github.com/cilium/cilium-cli/blob/v0.16.4/cli/connectivity.go [^4]: https://github.com/cilium/cilium-cli/tree/v0.16.4/connectivity [^5]: https://github.com/cilium/cilium-cli/blob/v0.16.4/cmd/cilium/main.go Signed-off-by: Michi Mutsuzaki <[email protected]>
Configure all the workflows that depends on cilium-cli to use the local version in /cilium-cli directory so that they use the local connectivity package for testing. Signed-off-by: Michi Mutsuzaki <[email protected]>
The purpose of this workflow is to ensure that the connectivity test continues to work against a released version of cilium when there is a change in cilium-cli/ directory. This workflow is based on kind.yaml in cilium-cli repo [^1]. [^1]: https://github.com/cilium/cilium-cli/blob/main/.github/workflows/kind.yaml Signed-off-by: Michi Mutsuzaki <[email protected]>
|
/test |
|
still need review from @cilium/sig-scalability and @cilium/github-sec 🚀🙏 (it's already approved by build and contributing teams since tobias is a member). apologies the pull request appears very large, but these files are simply copied from cilium-cli repo so you don't need to review the actual test logic: |
aanm
left a comment
aanm
left a comment
There was a problem hiding this comment.
I have some concerns with the way that the code is currenctly structured.
For example,
cilium-cli/cmd/cilium/main.go imports "github.com/cilium/cilium-cli/api".
This dependency, stored in "vendor/", imports "github.com/cilium/cilium/cilium-cli/connectivity".
We are kind of doing a import cycle across repositories 😕
Isn't there a cleaner option? I thought we were planning to use the "normal" cilium-cli and use cilium/cilium connectivity tests as "plugins" or "extensions" on top of cilium-cli.
Also, how do we make sure that the changes introduced in cilium/cilium connectivity tests don't break cilium/cilium-cli CI?
yeah we've been experimenting using the hooks to extend the connectivity package for a while (cc @jibi / @brb). i think in practice it didn't really solve the pain points developers are facing. it's been difficult to define the hook api ahead of time since different tests need to modify different aspects of the connectivity package. so often times you end up having to modify cilium-cli hook api anyways before you can write tests in cilium/cilium repo.
we run cilium connectivity test against a stable release with this workflow => https://github.com/cilium/cilium/pull/31077/files#diff-946ad5f53d9eb272c72747253dc2edef9bcdb5c5d13bc03a7ee20ac343eb8aaa |
Yep, as one who spent a significant amount of time developing the CLI tests last year I can confirm that the hooks didn't resolve the issues. |
|
back to the drawing board. converting to draft. |
|
This pull request has been automatically marked as stale because it |
|
This pull request has not seen any activity since it was marked stale. |
6 participants
3 commits:
cilium connectivity testagainst the latest released version of cilium when there is a change inpkg/cilium-cli.CFP: cilium/design-cfps#9
Test cilium-cli against a stable Cilium releaseworkflow is passing ✅ https://github.com/cilium/cilium/actions/runs/8563015042/job/23467355624?pr=31077