Merge `dpservice-go` and `dpservice-cli` into `dpservice` project and simplify testing setup

[afritzler]

Why?

Currently we are generating the Golang bindings from the dpservice proto file in a seperate project (dpservice-go). This dpservice-go project is then used inside dpservice-cli to create a CLI for interacting with the dpservice. Each project has it's own GH lifecycle and release code in place.

This causes some very bad side-effects when running end to end tests:

• Getting the latest (matching CLI) has to be done in the dpservice project via an own set of scripts
• This process is very error prune and might break as we have to keep all 3 projects in sync
• We have to do the housekeeping 3x across all projects
• We have to maintain Github actions across all 3 projects

Proposed solution

Move dpservice-go into a folder called go inside the dpservice project. This place will hold the generated golang protobuf files.

Move dp-service-cli into a folder called cli. This will be the home of the CLI packages. The goreleaser job will release the CLI packages as part of the dpservice release. That way we ensure the compatibility of the CLI with the underlying dpservice code base.

As a side note: the prometheus exported can be also be placed inside the dpservice project as it is tightly coupled with the dpservice. That way we can ensure functional integrity of the exported code.

[PlagueCZ]

I would just comment that the download script was always planned to be dropped once the project goes public, because then we will simply download the latest release package via a simple ADD docker command. I was planning to remove the script this month.

This will only cover your first reason for this change of course.

[afritzler]

The point is, there should be no need to download any artefacts. Instead you should install in the Makefile all necessary tools into a gitinored bin folder to run all your tests. You can find an example here: https://github.com/ironcore-dev/ironcore/blob/main/Makefile#L486-L488

The installation would be actually even easier if the CLI code would be part of the project itself as you then can ensure that you always use the correct version and also ensure that if something breaks you have an immediate prove in the test run.

[PlagueCZ]

Not arguing about making version matching easier, it would. The separation came from the fact, that dpservice-go is also needed to compile metalnet project.

[afritzler]

In your go.mod you just add the path github.com/ironcore-dev/dpservice/cli and you are done.

Golang doesn't care if the package is nested in a C project or not.

[guvenc]

Thanks for the enhancement suggestions @afritzler

It is always appealing to consolidate all "related" projects in a single repo in the favor of making things "easier" but this kind of approach comes also with its disadvantages and therefore needs to be carefully evaluated.

I can see the added value of integrating dpservice-go under a go directory indpservice as these go bindings are directly generated from the proto file residing in dpservice repo. (With a thin client wrapper over the GRPC API)

But this comes with the disadvantage of dealing with two different language toolings in a single repo. dpservice build system needs to "ignore" this directory. We will still need to maintain separate github action files for dpservice and go bindings part. Existing complexity is actually just shifted but not removed.

As I can imagine that dpservice-go can become part of thedpservice repo. (Because the advantages of moving it may outweigh the advantages of keeping it in a different repo) I am not really in the favor of "moving" everything to dpservice repo as the dpservice-cli and dpdk-exporter projects have the justification of being an independent repo.

dpservice-cli

• dpservice-cli gets very seldom changes and it will still need to get a new version with every release of the dpservice repo which gets quite often releases and new versions., if we move dpservice-cli t to dpservice.
• We really value to have a separate version for the golang dpservice cli.
• Having a focused repo for dpservice-cli makes also sense as dpservice-cli is not a library as the dpservice-go, it is a standalone application.
• As the repos are public now, moving/copying the binary from a public repo or from a sub directory during CI/CD has almost no difference regarding the ease of development.

dpdk-exporter

• This piece of code is actually %99 dpdk generic and can be used by other dpdk applications. (We plan to make the dpservice specific part "pluggable") Making it a seperate repo brings also the benefit of making it usable by other parties. (Hence promoting SAP's opensource engagement)

Apart from these discussions, we need to re-establish the status quo of the CI/CD pipelines in the dpservice and metalnet. The CI/CD pipelines were already developed and they worked fine so it should be easy first to make them run again so the colleagues can continue to work. As a second step, we can start to consolidate the repos where it really makes sense.

[PlagueCZ]

Adding a note here, since I encountered this situation when doing an update to the gRPC protocol.

If dpservice-cli and dpservice-go are separate respos, there can be no single PR to do that. First, dpservice needs to update dpdk.proto, implement the server part, then such PR needs to be merged and only after this dpservice-cli can be updated and merged.

This presents a problem, because dpservice's test suite uses dpservice-cli to work. Thus there can be no tests enabled for a newly introduced feature that changes the protocol. It then requires two separate PRs on dpservice side (one that changes dpdk.proto, the other to enable tests).

This two-step approach is doable, because we still have a C gRPC client, so tests can at least be run manually by the developer and reviewer, but it's not automatic.

Of course all the negatives mentioned by Guvenc may still outweigh this single benefit.

[guvenc]

552

[PlagueCZ]

Repositories dpservice-cli, dpservice-go and dpdk-prometheus-exporter now moved to dpservice.

[guvenc]

As mentioned by @PlagueCZ, this issue can be closed now.