Convert components to a go module

winkingturtle-vmw commented 1 month ago

Proposed Change

As a developer I would like the ability to build diego-release's components (e.g. bbs) as a standalone module So that when I run go install code.cloudfoundry.org/bbs/cmd/bbs@latest and it will install the latest version of the binary

diego-release components were built at a time before go modules existence. It was built with the assumption of GOPATH. As a result, most modules import each other, creating a circular dependencies where bbs needs to import locket and lcoket needs to import bbs. This is because it was easier to use helper libraries that each repo created originally. In order to be able to do this work we need to do the following to break out the circular import and allow each of those modules to be independent.

Here are the list of components that will have to be converted

auction
auctioneer
bbs
buildpackapplifecycle
cacheddownloader
cfdot
diego-ssh
dockerapplifecycle
ecrhelper
executor
fileserver
healthcheck
inigo
localdriver
locket
operationq
rep
route-emitter
volman
vizzini
workpool

For each of the repo above, we need to figure where it's being imported (e.g. bbs). Some of these imports are happening silently in releases that are not showing up in pkg.go.dev.
We then need to figure out why these internal repos are importing diego's repositories. In the case of bbs, most of the consumers are just looking for models or sqlhelpers. In this case, we would extract out the logic into it's own independent repository and update downstream consumers.
For a given repo, we then need to break out circular imports. In the case of bbs, it's importing locket, auctioneer, ecrhelper, executor, inigo, and workpool. Here is a branch for bbs and a branch for locket that removed the circular dependency between the two of them.
After we finish migrating each component, we then need to change the diego-release to import these modules via it's main go-module instead of submodules.

Acceptance criteria

diego-release will no longer have any submodules and I can deploy diego-release with cf-deployment, so that I cf push and run all acceptance tests.

Related links

Similar Discussion https://github.com/cloudfoundry/bbs/pull/97
Related work previously https://github.com/cloudfoundry/bbs/tree/with-go-mod

maxmoehl commented 1 week ago

Can you share a bit more detail about why the cyclic dependencies break what you are trying to achieve? If they are on the package level I would expect that go refuses to build them no matter how the modules are set up. But if they are cyclic on the module level they should not cause issues if I understand the situation correctly.

winkingturtle-vmw commented 5 days ago

@maxmoehl hmm, I am not sure if I follow your question, but let me re-iterate it differently. I am only going to use the following two code.cloudfoundry.org/bbs and code.cloudfoundry.org/locket modules only. These modules are used outside of diego-release, so it can't be internal and will need to have publicly addressable URL. Currently when there is no go-moded version/release of these repos, we are falling on a GOPATH backward compatible and unadvertised path to get the latest version of each. When they are both converted to a module, they import each other (BBS imports Locket and vice versa) and the cycle causes an issue.

That said, you are more than welcome to experiment with other ways of solving this problem. I am only sharing what has been our experience solving this issue thus far.

maxmoehl commented 5 days ago

When they are both converted to a module, they import each other (BBS imports Locket and vice versa) and the cycle causes an issue.

I would like to know which issue this causes, do you have some error message you encountered?

winkingturtle-vmw commented 5 days ago

When we first looked at this it was 3 years ago when we had to migrate to a go.mod and migrate off of GOPATH, so I don't have the specific error messages I was seeing. If we now have the community support behind this, we should re-start that effort and document it along the way for future.

maxmoehl commented 4 days ago

Okay, I'll try to reserve some time to play around and report back with my findings.

cloudfoundry / diego-release