feat(kuma-cp) synchronization of Dataplanes over KDS #847
Conversation
# Conflicts: # app/kuma-cp/cmd/run.go # pkg/config/app/kuma-cp/config.go # pkg/config/app/kuma-cp/kuma-cp.defaults.yaml
# Conflicts: # app/kuma-cp/cmd/run.go # pkg/config/kds/config.go # pkg/kds/reconcile/snapshot_generator.go # pkg/kds/server/components.go # pkg/kds/server/kds.go # pkg/kds/server/server.go # pkg/kds/server/server_test.go # pkg/kds/types.go # pkg/test/kds/verifier/context.go # pkg/test/kds/verifier/executables.go
# Conflicts: # go.sum
# Conflicts: # app/kumactl/pkg/install/k8s/control-plane/templates_vfsdata.go
app/kuma-cp/cmd/run.go
Outdated
| @@ -117,8 +121,8 @@ func newRunCmdWithOpts(opts runCmdOpts) *cobra.Command { | |||
| runLog.Error(err, "unable to set up Clusters server") | |||
| return err | |||
| } | |||
| if err := kds_server.SetupServer(rt); err != nil { | |||
| runLog.Error(err, "unable to set up KDS server") | |||
| if err := kds_global.SetupServer(rt); err != nil { | |||
There was a problem hiding this comment.
It's not exactly a server. Maybe SetupSync? SetupComponent?
Also, shouldn't this be other way around? It's local that connects to global as a client to fetch policies, but it's placed in case config_core.Global
There was a problem hiding this comment.
This is Global that connects to Local in order to fetch Dataplanes. I'm planning to implement Local -> Global in the next PR, because Local by default doesn't know about Global and it gets Global IP from the first DiscoveryRequest from Global.
So it will look like this:
- Global starts KDSSink for every Local for fetching Dataplanes (this PR)
- Local start KDSSink for the first request from Global for fetching policies (next PR)
SetupComponent sounds good.
| backoffTime = 5 * time.Second | ||
| ) | ||
|
|
||
| type ComponentFactory func(log logr.Logger) k8s_manager.Runnable |
There was a problem hiding this comment.
k8s_manager.Runnable? Shouldn't this be Component?
There was a problem hiding this comment.
I use component.Manager to operate with multiple Component as with a single one. After your LB changes Manager doesn't implement Component interface anymore. That's why I changed the interface to Runnable just to make it work.
Probably I have to create a new entity CompositeComponent and pass it to ResilientComponent, instead of passing Manager.
pkg/kds/global/components.go
Outdated
| return nil | ||
| } | ||
| cluster := clusterTag(rs.GetItems()[0]) | ||
| forEach(rs.GetItems()).apply(addPrefixToName(cluster)) |
There was a problem hiding this comment.
just so we are on the same page:
- dataplanes have to be prefixed when sync from remote to global
- policies do not need to be prefixed when sync from global to remote
- all stuff synchronized should be put in one namespace (do we have permissions to default?)
There was a problem hiding this comment.
Does this logic works in this way?
There was a problem hiding this comment.
- yes, when Global fetches Dataplanes from Remotes it adds clusterName as a prefix
- yes,
pkg/kds/global/components.gocreates Component just for Global, policies won't be prefixed when sync global to remote. It will be another code inpkg/kds/local/components.go - It works in e2e test, I guess we have permissions
pkg/kds/global/components.go
Outdated
| addPrefixToName := func(prefix string) func(r model.Resource) { | ||
| return func(r model.Resource) { | ||
| newName := fmt.Sprintf("%s.%s", prefix, r.GetMeta().GetName()) | ||
| // method Sync takes into account only 'Name' and 'Mesh' that why we can set name like this |
There was a problem hiding this comment.
I don't quite get this comment. Can you elaborate?
There was a problem hiding this comment.
I meant Sync doesn't take into account Version, CreationTime and ModificationTime that's why we can replace the whole ResourceMeta object before Sync
pkg/kds/global/components.go
Outdated
| return r.GetSpec().(*mesh_proto.Dataplane).GetNetworking().GetInbound()[0].GetTags()[mesh_proto.ClusterTag] | ||
| } | ||
|
|
||
| addPrefixToName := func(prefix string) func(r model.Resource) { |
There was a problem hiding this comment.
how about simpler impl. Instead of closures that return functions and extra forEach apply etc.
func addPrefixToNames(items, prefix)
func addSuffixToNames(items, suffix)
as a separate functions
| } | ||
| cluster := clusterTag(rs.GetItems()[0]) | ||
| forEach(rs.GetItems()).apply(addPrefixToName(cluster)) | ||
| if k8sStore { |
There was a problem hiding this comment.
at least explain with comment why we need this
| @@ -38,7 +35,7 @@ var _ = Describe("Test Local and Global", func() { | |||
| _ = clusters.DeleteKuma() | |||
| }) | |||
|
|
|||
| It("Should deploy Local and Global on 2 clusters", func() { | |||
| It("should deploy Local and Global on 2 clusters and sync dataplanes", func() { | |||
There was a problem hiding this comment.
Remote and Global
There was a problem hiding this comment.
I didn't merge master yet, so I have a lot of "Local" usages in this PR
# Conflicts: # app/kuma-cp/cmd/run.go # app/kumactl/pkg/install/k8s/control-plane/templates_vfsdata.go # pkg/config/kds/config.go # pkg/kds/reconcile/snapshot_generator.go # pkg/kds/server/components.go # pkg/kds/server/grpc.go # pkg/kds/server/kds.go # pkg/kds/server/server_test.go # pkg/kds/types.go # pkg/test/kds/verifier/context.go # pkg/test/kds/verifier/executables.go # test/e2e/kuma_deploy_global_remote_test.go
Summary
Current PR introduces an ability to synchronize Dataplanes between Locals and Global over KDS.
It turned out it is simple to implement Dataplane synchronization first and then synchronize other resources on top of it (https://github.com/Kong/kuma/issues/804).
upd: Store resources in dedicated namespace if Global has k8s-store.
Limitations:
Issues resolved
Fix https://github.com/Kong/kuma/issues/806
Documentation