Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update module from antrea.io/antrea to antrea.io/antrea/v2 #6774

Open
jainpulkit22 opened this issue Oct 28, 2024 · 4 comments
Open

Update module from antrea.io/antrea to antrea.io/antrea/v2 #6774

jainpulkit22 opened this issue Oct 28, 2024 · 4 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@jainpulkit22
Copy link
Contributor

Describe the bug
The antrea module in go.mod should be updated from antrea.io/antrea/v2 since now we are on v2.x

Additional context
If we try to require this in some other repository then it will generate error because go.mod will not parse until the module is antrea.io/antrea/v2.

@jainpulkit22 jainpulkit22 added the kind/bug Categorizes issue or PR as related to a bug. label Oct 28, 2024
@jainpulkit22
Copy link
Contributor Author

cc: @antoninbas @tnqn

@antoninbas
Copy link
Contributor

That's a real issue, but I think there is a deeper issue that should be addressed at the same time: antrea.io/antrea is not really meant to be imported as a module (like k8s.io/kubernetes is not meant to be imported for example). Only a small part of antrea.io/antrea is meant to be imported: APIs and generated clientsets. By forcing consumer projects of the Antrea APIs to import all of antrea.io/antrea, we can create issues in dependency management for these other projects.

In my view, there are 2 solutions that are worth considering:

  1. move Antrea APIs and generated clients to a separate repository / module (e.g., antrea-io/apis), for which we would not have any semantic versioning. Calico has taken that approach.
  2. leverage workspace support introduced in Go 1.18. K8s currently uses a Go workspace: https://github.com/kubernetes/kubernetes/blob/master/go.work

A few characteristics of each approach:

Approach 1:

  • when making an update to an API, antrea-io/apis needs to be updated first, separately from antrea-io/antrea. For minor changes / fixes or for chores (code generation version updates), this may be a bit of a nuisance. For major changes such as the introduction of a new API, we anyway recommend opening separate PRs even today (one for the API change, one for the implementation)
  • we don't actually need to update the package name (and all imports) from antrea-io/antrea to antrea-io/antrea/v2

Approach 2:

  • everything stays in the same repo, so we keep the contribution model the same
  • we still need to update the package name (and all imports) from antrea-io/antrea to antrea-io/antrea/v2
  • APIs need to be imported as antrea-io/antrea/v2/apis, assuming we use apis as the module for APIs inside the workspace

In both cases, some code reorganization is necessary. I don't have a strong preference at the moment. Approach 1 is pretty straightforward, while I am not as familiar with Approach 2 and what it entails. Probably something worth discussing at the next community meeting.

@antoninbas antoninbas added this to the Antrea v2.3 release milestone Oct 28, 2024
@antoninbas antoninbas added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Oct 28, 2024
@jainpulkit22
Copy link
Contributor Author

@antoninbas Approach 1 seems good because only apis are used by consumers, but in general approach2 seems to be more uniform and maintainable in long run, because we need not maintain separate versions and this makes the code(test code as well) more maintainable.

@antoninbas antoninbas self-assigned this Nov 14, 2024
@antoninbas
Copy link
Contributor

After reviewing the 2 solutions above, I have concluded that none of them are really satisfactory:

  • Approach 1 is superior IMO, but does require extra repos (at least one for APIs, and potentially one for the generated clientset?) and will complicate the development process.
  • Approach 2 introduces more complexity, and while workspaces can be used to build "monorepos", that doesn't seem to be the primary use case (it is actually recommended for go.work not to be checked in). I am sure this is a good solution for a project like K8s, but doesn't seem appropriate at our scale.

Therefore, I'd like to suggest the following 2-step approach:

  1. Keep the status quo for projects importing antrea-io/antrea, and update the module name to antrea.io/antrea/v2 to match our major version number
  2. In order to streamline imports, consider introducing new "sub-modules" as part of the same repo (https://github.com/antrea-io/antrea) that can be imported by downstream projects in a convenient fashion. For example, we could have antrea.io/antrea/apis for APIs, and antrea.io/antrea/client for generated clientsets.

Step 1 does require modifying most Go files in this repo (as well as a few others) to update imports from antrea.io/antrea/... to antrea.io/antrea/v2/.... For Go files, a tool such as https://github.com/marwan-at-work/mod works pretty well. Here is the size of the diff:

 906 files changed, 3078 insertions(+), 3078 deletions(-)

Step 2 does also have some caveats:

The advantages of step 2 still exist, but may not be as important as I initially thought / presented. For example, I wrote above:

we can create issues in dependency management for these other projects

While this is somewhat true, and fewer dependencies is always better, especially for an "API" module, dependency pruning (introduced in go 1.17) means that not all Antrea dependencies are added to downstream projects. This is the go.mod I get when I create a module that import antrea.io/antrea, but only consumes APIs:

go 1.23.0

require antrea.io/antrea v1.15.2

require (
	github.com/go-logr/logr v1.4.1 // indirect
	github.com/gogo/protobuf v1.3.2 // indirect
	github.com/google/gofuzz v1.2.0 // indirect
	github.com/json-iterator/go v1.1.12 // indirect
	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
	github.com/modern-go/reflect2 v1.0.2 // indirect
	golang.org/x/net v0.20.0 // indirect
	golang.org/x/text v0.14.0 // indirect
	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
	gopkg.in/inf.v0 v0.9.1 // indirect
	gopkg.in/yaml.v2 v2.4.0 // indirect
	k8s.io/api v0.26.4 // indirect
	k8s.io/apimachinery v0.26.4 // indirect
	k8s.io/klog/v2 v2.100.1 // indirect
	k8s.io/utils v0.0.0-20230209194617-a36077c30491 // indirect
	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
)

These dependencies would presumably stay the same if we were to move Antrea APIs to a separate module.

So in practice, the advantages of step 2 would be:

  1. Better code organization and separation of APIs, with a clear picture of the API-specific dependencies
  2. The ability to set a lower go version directive for the API module compared to the main Antrea module, hence enabling import by projects using an older go version

See https://github.com/vmware-tanzu/vm-operator for an example of a project which uses a separate module for APIs (but in the same source repository).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

2 participants