Support multi-module releases in go-control-plane #714

alecholmez · 2023-06-05T15:27:10Z

This PR introduces support for multi-module releases in go-control-plane following the plan of action outlined in issue #391.

A few things have happened:

Bumped modules to Go 1.20 (to inherit support for workspaces)
Introduced a go workspace that orchestrates all the subcomponents of this repo
Initialized new modules for ./envoy and ./contrib containing the generated go stubs
Renamed build/ to scripts/ so we have a bucket for more general repo automation

When this PR is merged, a tag will be cut at v0.11.2 but a new tag will be introduced with envoy/v1.26.2. closes #391

valerian-roche

I'm not familiar with go workspaces, but overall lgtm
Nice to see independent versioning through this though

valerian-roche · 2023-06-05T19:45:16Z

contrib/go.mod

+
+go 1.20
+
+replace github.com/envoyproxy/go-control-plane/envoy => ../envoy


My understanding from go.work is that it should no longer be needed to add those replace commands
Is that actually not the case?

I actually couldn't get go mod tidy to play nice without them. I can mess around with it a bit more but figured this was fine for now

I was able to drop the replace from ./envoy but that one still complained so I left it in.

Maybe this is an indication that we don't have the right structure? Since the "envoy" and "contrib" protobufs are bound to the same Envoy release, does it make sense to allow the possibility for a program to import different versions of them?

How hard would it be to move the contrib protobufs to "envoy/contrib"?

And I suppose that a similar question arises for the "./ratelimit" directory.

heres a branch of Contour using the above branch: https://github.com/sunjayBhatia/contour/tree/use-go-control-plane-multi-module

(the above go-control-plane branch also copies the ratelimit module to a package in the new api module)

so this branch of Contour works with this PR's branch (plus this commit sunjayBhatia@4be6f62): sunjayBhatia/contour@dc8fa81

i think either approach will work just fine, though the upgrade/transition UX might be cleaner (no possibility for users complaining about ambiguous package paths if they are requiring mismatched versions) if the new modules have a new package path

(the above go-control-plane branch also copies the ratelimit module to a package in the new api module)

I'll check later, but me recollection is that the ratelimit protos don't come from the envoy repository. So they probably need to be versioned differently and it might not make sense for them to be in the same module as the envoy api.

.gitignore

jpeach · 2023-06-06T04:11:10Z

contrib/go.mod

@@ -0,0 +1,23 @@
+module github.com/envoyproxy/go-control-plane/contrib
+
+go 1.20


JFYI, we should be prepared for some pushback on this. It's possible that some consumer of this module will be using earlier Go versions (see #271 for example).

I can try and roll back to 1.18 but that is a bare minimum requirement for workspaces. Wondering if we should see if it's that problematic and roll back if people complain.

+1 please don't use 1.20. In grpc-go, which uses this, we support 1.18+ and also haven't made any changes AFAIK that would prevent even 1.17 or maybe 1.16 from working (e.g. no use of any or generics). IMO this (and the use of new language features) should stay at the oldest version you can reasonably handle supporting.

@alecholmez it sounds like this is the most contentious point within this change set.

jpeach · 2023-06-06T04:12:26Z

Initialized new modules for ./envoy and ./contrib containing the generated go stubs

We probably want to do the same for ./ratelimit too?

dfawley · 2023-06-08T16:43:04Z

go.work

@@ -0,0 +1,10 @@
+go 1.20


Sorry for the dumb question...what do go workspaces actually do for you? Someone wanted to add a go.work file to our repo, but I didn't fully understand how it would help. We have several modules and don't use a workspace, and things generally work fine.

I think they are more useful in local development etc. when you have modules side-by-side rather than a module hierarchy like we have here, you should mainly be able to remove the replace directives but it doesn't seem (from my playing around with it as well) that we get that here because of the package/module structure

e.g. to get the other modules in to actually use the toher modules in this repo all the way, had to do this: sunjayBhatia@4be6f62

so I'm not sure how much the workspace is getting us

Is the decision to ultimately remove the workspace? I'm fine with that

github-actions · 2023-07-10T04:06:21Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

github-actions · 2023-07-17T04:06:38Z

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

alecholmez · 2023-12-20T03:22:03Z

Hey all, been away for quite a while. Starting to catch back up on this project. Did we ever come to a consensus on the way forward here? I have some cycles I can spend to push this across so we can get onto a more regular release cycle.

mmorel-35 · 2024-07-24T10:27:44Z

Hi @alecholmez ,
You might want to rebase your work.
The go version has changed.
Have you considered defining a module per extension in contrib instead of a contrib module ?

valerian-roche · 2024-08-04T22:45:43Z

I merged head, but my understanding is that we need to create the tags for the sub-modules first in order to allow proper importing. Trying to import in a project with the current setup, I get

 % go mod tidy
go: downloading github.com/envoyproxy/go-control-plane v0.12.1-0.20240804223412-e9538224ae87
go: github.com/envoyproxy/[email protected] requires
	github.com/envoyproxy/go-control-plane/[email protected]: invalid version: unknown revision 000000000000

In this case this PR would need:

first create the changes within the submodules, and tag them
this would require to follow some specific order, to add the different steps (envoy, then contrib and ratelimit, then the rest)
update tooling to allow consistency. I'm still unclear how we can maintain consistency in tagging with a multi-module repository. We can auto-tag envoy and contrib when syncing from envoy, but we'd need to still manually update all the other modules

go.work

Signed-off-by: Alec Holmes <[email protected]>

Signed-off-by: Valerian Roche <[email protected]>

alecholmez requested review from jpeach, valerian-roche, kyessenov, dfawley and sunjayBhatia June 5, 2023 15:27

alecholmez force-pushed the multi-module-release branch from 7686dfb to 12c81b1 Compare June 5, 2023 15:28

alecholmez assigned sunjayBhatia Jun 5, 2023

alecholmez mentioned this pull request Jun 5, 2023

sotw: Ordered ADS mode #544

Merged

valerian-roche previously approved these changes Jun 5, 2023

View reviewed changes

alecholmez dismissed valerian-roche’s stale review via 9ae8c06 June 5, 2023 21:07

alecholmez force-pushed the multi-module-release branch from 12c81b1 to 9ae8c06 Compare June 5, 2023 21:07

alecholmez requested a review from valerian-roche June 5, 2023 21:08

jpeach reviewed Jun 6, 2023

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

jpeach reviewed Jun 6, 2023

View reviewed changes

alecholmez force-pushed the multi-module-release branch 2 times, most recently from 0ab1e02 to 1b350d3 Compare June 6, 2023 15:00

dfawley reviewed Jun 8, 2023

View reviewed changes

jpeach mentioned this pull request Jun 14, 2023

RFC: Sync Envoy protobufs to a nested proto module. #723

Closed

sunjayBhatia mentioned this pull request Jun 29, 2023

api: generate Go protobufs to a module subdirectory envoyproxy/envoy#28041

Closed

github-actions bot added the stale label Jul 10, 2023

github-actions bot closed this Jul 17, 2023

alecholmez reopened this Jul 17, 2023

alecholmez added no stalebot and removed stale labels Jul 17, 2023

alecholmez mentioned this pull request Dec 20, 2023

Release schedule for this project? #832

Closed

valerian-roche mentioned this pull request May 8, 2024

Support ext_authz: Added support for populating the tls_session for extensions.filters.network.ext_authz.v3.ExtAuthz #936

Closed

mmorel-35 reviewed Aug 5, 2024

View reviewed changes

go.work Show resolved Hide resolved

Support multi-module releases in go-control-plane

d1247aa

Signed-off-by: Alec Holmes <[email protected]>

mmorel-35 force-pushed the multi-module-release branch 2 times, most recently from 36e2e63 to a360f6f Compare August 30, 2024 07:56

Align go.mod with latest updates

69a8bde

Signed-off-by: Valerian Roche <[email protected]>

mmorel-35 force-pushed the multi-module-release branch from a360f6f to 69a8bde Compare August 30, 2024 07:59

mmorel-35 requested review from jpeach, phlax, dfawley, mmorel-35 and craigjbass August 30, 2024 08:12

mmorel-35 approved these changes Aug 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support multi-module releases in go-control-plane #714

Support multi-module releases in go-control-plane #714

alecholmez commented Jun 5, 2023 •

edited

Loading

valerian-roche left a comment

valerian-roche Jun 5, 2023

alecholmez Jun 5, 2023

alecholmez Jun 5, 2023

jpeach Jun 6, 2023

jpeach Jun 6, 2023

sunjayBhatia Jun 9, 2023

sunjayBhatia Jun 9, 2023

sunjayBhatia Jun 9, 2023

sunjayBhatia Jun 9, 2023

jpeach Jun 10, 2023 •

edited

Loading

jpeach Jun 6, 2023

alecholmez Jun 6, 2023 •

edited

Loading

dfawley Jun 8, 2023

craigjbass Dec 22, 2023

jpeach commented Jun 6, 2023

dfawley Jun 8, 2023

sunjayBhatia Jun 8, 2023

sunjayBhatia Jun 9, 2023

alecholmez Dec 30, 2023

github-actions bot commented Jul 10, 2023

github-actions bot commented Jul 17, 2023

alecholmez commented Dec 20, 2023

mmorel-35 commented Jul 24, 2024

valerian-roche commented Aug 4, 2024


		go 1.20

		replace github.com/envoyproxy/go-control-plane/envoy => ../envoy

		@@ -0,0 +1,23 @@
		module github.com/envoyproxy/go-control-plane/contrib

		go 1.20

Support multi-module releases in go-control-plane #714

Are you sure you want to change the base?

Support multi-module releases in go-control-plane #714

Conversation

alecholmez commented Jun 5, 2023 • edited Loading

valerian-roche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpeach Jun 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alecholmez Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpeach commented Jun 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 10, 2023

github-actions bot commented Jul 17, 2023

alecholmez commented Dec 20, 2023

mmorel-35 commented Jul 24, 2024

valerian-roche commented Aug 4, 2024

alecholmez commented Jun 5, 2023 •

edited

Loading

jpeach Jun 10, 2023 •

edited

Loading

alecholmez Jun 6, 2023 •

edited

Loading