fix(kuma-cp) subscription finalizer #2444
Conversation
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
| @@ -123,7 +123,7 @@ func NewUpsertOpts(fs ...UpsertFunc) UpsertOpts { | |||
| return opts | |||
| } | |||
|
|
|||
| func Upsert(manager ResourceManager, key model.ResourceKey, resource model.Resource, fn func(resource model.Resource), fs ...UpsertFunc) error { | |||
There was a problem hiding this comment.
Is there a way to give a better name to the fn parameter or to type the func? It's tricky to give a good name because it often relies on side effect but maybe shouldUpdate(res)
|
|
||
| // CandidateForDisconnect is an indicator if the data plane proxy should be | ||
| // disconnected in the near future | ||
| bool candidate_for_disconnect = 7; |
There was a problem hiding this comment.
Connection tracking seems like an implementation detail, not something that should be exposed as art of the API.
| dataplaneResource = "" | ||
| } else { | ||
| dataplaneResource = opts.appYaml | ||
| } |
There was a problem hiding this comment.
Why don't we want to use the resource that kubectl rendered?
There was a problem hiding this comment.
We have 2 ways to run DPP on Universal:
- Create resource using
kumactl apply -f ...and runkuma-dp - Pass resource directly to
kuma-dplikekuma-dp run --dataplane-file ...
Rendering is supported only for the latter flow
| err := manager.Upsert(f.rm, core_model.MetaToResourceKey(item.GetMeta()), upsertInsight, func(_ core_model.Resource) bool { | ||
| upsertInsight.GetSpec().(kuma_interfaces.Insight).UpdateSubscription(insight.GetLastSubscription()) | ||
| return true | ||
| }) |
There was a problem hiding this comment.
Is there a way that we an avoid persisting this state, by keeping an in-memory candidate list? There's 2 issues that I'm concerned about (1) exposing this state in the API and (2) write amplification on the data store.
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru> # Conflicts: # api/mesh/v1alpha1/dataplane_insight.pb.go # api/system/v1alpha1/zone_insight.pb.go # pkg/xds/server/callbacks/dataplane_status_sink_test.go # test/framework/universal_cluster.go
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
|
closing in favor of #2526 |
Summary
This is not exactly Solution 2 from #2320, slightly different approach and doesn't require real disconnections.
SubscriptionFinalizer is a new component that runs on the leader Kuma CP and every N second marks all Insights as a candidate for further disconnect. Those Insights that have connected DPPs (or Zone CPs) the "candidate" status will be unset almost immediately in StatusSink. So it's required 2*N second to finalize the subscription of disconnected DPP (or Zone CP).
Current implementation implies KDS traffic every N second. I think it's fine, but could be improved if we change
kds.GenerateSnapshot. Instead of comparing the whole Resource (with Meta) we can check for equality only Resource.Spec, having previously setCandidateForDisconnectto false.Full changelog
idleTimeoutproperty for kuma-cp configIssues resolved
Fix #2320
Documentation
Testing
Backwards compatibility
backport-to-stablelabel if the code is backwards compatible. Otherwise, list breaking changes.