v3 migration - clusters, endpoints #1312
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
So far this approach is great. We'll support v2 and v3 and smoothly migrate. |
nickolaev
reviewed
Dec 14, 2020
| envoy_api_v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" | ||
| envoy_core_v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" | ||
| envoy_cluster "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" | ||
| envoy_core "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" |
There was a problem hiding this comment.
As agreed, let's always use a numbered version of the envoy imports so envoy_core_v3 etc.
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
jakubdyszkiewicz
force-pushed
the
chore/upgrade-v3
branch
from
2f93a82
to
e1e142e
Compare
jakubdyszkiewicz
changed the title
nickolaev
approved these changes
Dec 15, 2020
| Configure(cluster *envoy_api.Cluster) error | ||
| } | ||
| envoy_api_v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" | ||
| envoy_api "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" |
There was a problem hiding this comment.
This seems inconsistent with the below v2 and v3 pointing out.
Shall e just use envoy_api_v3? I don't mind either way, but maybe being explicit will make supporting these through thime slightly easier.
| "github.com/kumahq/kuma/pkg/core/xds" | ||
| "github.com/kumahq/kuma/pkg/util/proto" | ||
| "github.com/kumahq/kuma/pkg/xds/envoy" | ||
| envoy_metadata "github.com/kumahq/kuma/pkg/xds/envoy/metadata/v2" | ||
| tls_v2 "github.com/kumahq/kuma/pkg/xds/envoy/tls/v2" |
There was a problem hiding this comment.
I would assume that under v2 folder you don't mix versions so tls_v2 could just be tls.
| "github.com/kumahq/kuma/pkg/xds/envoy" | ||
| envoy_metadata "github.com/kumahq/kuma/pkg/xds/envoy/metadata/v3" | ||
| "github.com/kumahq/kuma/pkg/xds/envoy/tls" | ||
| tls_v3 "github.com/kumahq/kuma/pkg/xds/envoy/tls/v3" |
There was a problem hiding this comment.
Same here, tls instead of tls_v3
| envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" | ||
| envoy_grpc_credential_v3 "github.com/envoyproxy/go-control-plane/envoy/config/grpc_credential/v3" | ||
| envoy_tls_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" | ||
| envoy_type_matcher_v3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" |
There was a problem hiding this comment.
everything here is v3, so no need to explicitly envoy_tls_v3
| @@ -32,7 +32,7 @@ func (g InboundProxyGenerator) Generate(ctx xds_context.Context, proxy *model.Pr | |||
|
|
|||
| // generate CDS resource | |||
| localClusterName := envoy_names.GetLocalClusterName(endpoint.WorkloadPort) | |||
| clusterBuilder := envoy_clusters.NewClusterBuilder(). | |||
| clusterBuilder := envoy_clusters.NewClusterBuilder(envoy_common.APIV2). | |||
There was a problem hiding this comment.
Should you put a todo on these so we don't accidentally miss something when we get to configure the version?
|
I'll fix the imports in the next PR |
mergify bot
pushed a commit
that referenced
this pull request
Dec 16, 2020
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com> (cherry picked from commit 9df49a1) # Conflicts: # pkg/xds/envoy/clusters/v2/health_check_configurer.go
jakubdyszkiewicz
pushed a commit
that referenced
this pull request
Dec 18, 2020
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cluster / Endpoints / TLS / Metadata V3 migration
This PR enables generation of V3 config for Cluster and Endpoints
Concept of V3 migration
Most of the go-control-plane V2 imports is in
pkg/xds/envoyand are structures are hidden behind builders.Then, builders are used in proxy generators which don't really care about the actual type, they care that it is a resource that can be added to
ResourceSetwhich will be put into the cache. You will have to now pass the API Version to the builder as a mandatory parameter so the builder can choose which version of the resource it will return.V2 -> V3 migration will result in the copy-pasting of a massive amount of the code, but at least here it is hidden behind builders, so the copy-paste will be in one place and it will be easy to remove in the future.
Of course, it's not that obvious, V2 imports are spread around a code
some of the tricky parts will be
but here is a good start for the actual V3 config generation