CFP-25694: Move cilium/cilium-cli code into cilium/cilium repository

[michi-covalent]

https://github.com/cilium/cilium/issues/25694

[michi-covalent]

but also the benefits of isolating cilium-cli from the cilium codebase (for release cadence, dev velocity, ensuring compatability, ...)

i'm not too worried about release cadence and ensuring compatibility. i believe this CFP addresses these points reasonably well.

dev velocity is a real concern. however, once we finish migrating cilium-cli to helm mode, most of the cilium-cli development might become:

• adding tests
• adding sysdump tasks
• occasionally adding new sub-commands / flags to accommodate new features

for these things, it might actually be faster to develop everything in cilium/cilium 🤔

merging two repos will most likely reduce the overall number of dependency update pull requests too.

i guess what i'm saying is: dev velocity is a bit difficult to quantify 💭

Should we be instead working towards a future where we can develop tests in cilium/cilium, and in CI the CLI picks them up and runs them against the PR version?

potentially, i need a bit more detail on how this might look like. happy to chat about it.

[joestringer]

One more thought, I like how sysdump isn't too heavily tied to individual cilium versions, because cilium-cli delegates some of the details to Cilium. In the back of my mind, I'm wondering if the solution to this isn't to more tightly couple cilium & cilium-cli, but rather the inverse - further decouple the details so that cilium-cli needs fewer cilium-specific changes over time.

[michi-covalent]

thank you all for your feedback 🙏. let me try to summarize all the discussions here.

at a very high level, there are 2 options that emerged from this CFP:

• option A: merge cilium-cli code into cilium/cilium repository. this is what i originally proposed.
• option B: use cilium-cli's Hook interface to extend cilium connectivity test without merging cilium-cli code into cilium/cilium repo.

based on the discussion in this thread, looks like people are ok with option B. since we have already discussed option A extensively, i'll focus on option B in this comment.

pros

• no need to touch cilium/cilium-cli repo at all.

cons / open questions

• from jibi: One problem I see with that approach is that it still doesn't solve the current cilium-cli CI issue, as it's possible to miss breakages until the CLI changes are tested in the cilium/cilium workflows.

  maybe we could add one workflow in cilium/cilium-cli repo that runs against the latest cilium from main branch 👀 then, we just bump cilium/cilium-cli version in cilium/cilium repo whenever there is a new release using renovate.

• from martynas: However, I find it useful that users have means to do an extensive testing in their clusters.

  maybe there is a way to import these tests in cilium/cilium repo from cilium/cilium-cli repo 🤔

• (from michi) if we go with option B, we need general guidelines to clarify when a test should be added directly to cilium/cilium-cli repo, vs when it should go under cilium/cilium repo.

i'll write up a proposal for option B in a separate file 📝

[michi-covalent]

ok i've been sleeping on this for a while now, talking to various people here and there. let me summarize what i'm currently thinking, and what next steps we should take to achieve the original goal of minimizing the feature development overhead for adding new integration tests for cilium features.

tldr

pursue option B: use cilium-cli's Hook interface to add tests.

what's wrong with merging cilium-cli repo to cilium repo?

nothing is wrong per se, and merging cilium-cli repo is certainly one way to make it easier to use the connectivity package. however now i see flaws in my original reasoning for merging cilium-cli repo to cilium repo. my original reasoning went like this:

1. i need a way to add integration tests in cilium/cilium.
2. i like using cilium-cli's connectivity package for writing tests.
3. however i don't like the connectivity package being in a different repository because i need to open a separate pull request if i need to enhance the connectivity package to accommodate what i need to accomplish.
4. therefore let's merge cilium-cli repo into cilium repo.
5. face any unpredictable consequences in terms of disrupting cilium-cli's development and release processes.
6. figure out if we need to maintain cilium-cli's go module name to avoid breaking projects that import cilium-cli.

the main flaw of this reasoning is in 3 where it jumps from "i need to improve the package i'm using for testing" to "let's merge the entire repo to avoid opening separate pull requests for it". if it were any other testing library (like let's say testify), i'd be much more hesitant to copy it over into cilium repo just to avoid having to open separate pull requests.

i acknowledge that the connectivity package is still in a very early stage of development in terms of enabling outside projects to use it as a library. therefore early adopters often find that they need to improve the connectivity package. @jibi has been trailblazing and making many contributions in this area, including:

• https://github.com/cilium/cilium-cli/pull/2050
• https://github.com/cilium/cilium-cli/pull/1986
• https://github.com/cilium/cilium-cli/pull/1978
• https://github.com/cilium/cilium-cli/pull/1977

my naively optimistic assertion is that this situation will improve over time as the connectivity package matures, and we can get to the point where most people can simply use cilium-cli's Hook interface to add new tests without having to polish off rough edges in the connectivity package.

if this assertion is wrong, pain points for using the connectivity package should be easily identifiable. and again i'm optimistically confident in cilium community's ability to come up with pragmatic solutions to address these pain points.

next steps

• add a test in cilium/cilium using cilium-cli's Hook interface to see how it might look like. i'll try to find some to do this myself, but ping me if anybody is interested in experimenting with this.

[michi-covalent]

sorry for the delay here are proof of concept draft PRs:

• https://github.com/cilium/cilium/pull/31077 moves connectivity package from cilium-cli to cilium.
• https://github.com/cilium/cilium-cli/pull/2355 imports back the connectivity package from cilium.

we might decide to move more packages later on but i think this is a good first cut. @brb @tklauser please take a look and let me know if the overall approach looks ok 🚀🙏

[brb]

@michi-covalent Thanks! Could you TL;DR a new workflow when adding new feature / tests to cilium/cilium? In particular, how they will be run by CLI in the existing GH workflows?

[michi-covalent]

yeah so it uses cilium-cli github action https://github.com/cilium/cilium-cli/blob/main/action.yaml to build and install the Cilium CLI from pkg/cilium-cli directory with this main.go: https://github.com/cilium/cilium/blob/5a838dbb895ef3288c9a14f9e997b44bab874105/pkg/cilium-cli/main.go so you can add you tests in pkg/cilium-cli/connectivity/ and they'll magically get picked up in your PR 🪄

[xmulligan]

@michi-covalent is this still up to date with the current ideas?

[michi-covalent]

yes coming soon i promise

[xmulligan]

Ok, thanks. Was actually just checking if you think it is ready to merge or if anything else needs to be added or updated

[joestringer]

One thing I like from the proposed process in https://github.com/cilium/design-cfps/pull/37 (README preview) is this idea that we can merge a CFP once the ideas are agreed in principle, with the status being "implementable". If the CFP is up to date and the relevant stakeholders approve, we are good to do that even if we are still going through the practical ramifications of how we implement this.

Then we can come back later to update the CFP as "experimental" / "stable" if something changes significantly in the design. Those changes would also be PRs and we can always debate the details at that point too. Merging an "implementable" design does not mean we are hard committed to the specific approach, things are always still flexible.

We could trial this process with this PR if you like.

[xmulligan]

I've tried to outline a short status framework here. Practically speaking, if this CFP has the approval of one committer, then it could be merged as implementable.

[michi-covalent]

@brb it would mean a lot to me if you approve this pull request 🥦 https://github.com/cilium/cilium/pull/34178 just got merged 🚀🙏