fix(xds): accelerate universal dp XDS generation

[Icarus9913]

In the XDS callback chain, the dataplane sync tracker callback relies on the dataplane object to generate the xds configurations. However, the dataplane object creation happens in dataplane liftcycle callback which is latter than the sync tracker callback. Consequetenly, just revert these 2 order and create the dataplane object before the sync call.

Reference issue: #11135

Checklist prior to review

• Link to relevant issue as well as docs and UI issues --
• This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as a image registry) and it will work on Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS --
• Tests (Unit test, E2E tests, manual test on universal and k8s) --
  • Don't forget ci/ labels to run additional/fewer tests
• Do you need to update UPGRADE.md? --
• Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label) --

[lukidzi]

@jakubdyszkiewicz Do you know if that order was important? I think it was visible after this fix: #11094

[jakubdyszkiewicz]

It makes sense, but it does not fully fix the problem.

We retrieve DPP for which we generate XDS from the MeshContext to avoid additional queries. MeshContext is cached (by default for 1s). This means that between creation of DPP and running XDS watchdog you need to invalidate the cache to pick up new DPP. This may not be the case very often.
If you were testing this on a single DPP you always had invalid cache, because no DPPs connected = no cached MeshContext.

[Icarus9913]

It makes sense, but it does not fully fix the problem.

We retrieve DPP for which we generate XDS from the MeshContext to avoid additional queries. MeshContext is cached (by default for 1s). This means that between creation of DPP and running XDS watchdog you need to invalidate the cache to pick up new DPP. This may not be the case very often. If you were testing this on a single DPP you always had invalid cache, because no DPPs connected = no cached MeshContext.

Yeah, you're right.
Once the Dataplane resource was created, the xds Sync would try to retrieve this Dataplane object by ReadOnlyResourceManager which uses the cache to get the MeshContext first.

kuma/pkg/core/resources/manager/cache.go

Lines 93 to 117 in e21c751

	func (c *cachedManager) List(ctx context.Context, list model.ResourceList, fs ...store.ListOptionsFunc) error {
	tenantID, err := c.tenants.GetID(ctx)
	if err != nil {
	return err
	}
	opts := store.NewListOptions(fs...)
	if !opts.IsCacheable() {
	return fmt.Errorf("filter functions are not allowed for cached store")
	}
	cacheKey := fmt.Sprintf("LIST:%s:%s:%s", list.GetItemType(), opts.HashCode(), tenantID)
	obj, found := c.cache.Get(cacheKey)
	if !found {
	// There might be a situation when cache just expired and there are many concurrent goroutines here.
	// We should only let one fill the cache and let the rest of them wait for it. Otherwise we will be repeating expensive work.
	mutex := c.mutexFor(cacheKey)
	mutex.Lock()
	obj, found = c.cache.Get(cacheKey)
	if !found {
	// After many goroutines are unlocked one by one, only one should execute this branch, the rest should retrieve object from the cache
	c.metrics.WithLabelValues("list", string(list.GetItemType()), "miss").Inc()
	if err := c.delegate.List(ctx, list, fs...); err != nil {
	mutex.Unlock()
	return err
	}
	c.cache.SetDefault(cacheKey, list.GetItems())

The default cache expiration is 1s:

kuma/pkg/config/app/kuma-cp/kuma-cp.defaults.yaml

Line 71 in e21c751

expirationTime: 1s # ENV: KUMA_STORE_CACHE_EXPIRATION_TIME

So, this PR doesn't fix the issue totally.

[Icarus9913]

Maybe we should set an interval between dataplane_lifecycle and goroutine dataplane_sync_tracker.

kuma/pkg/xds/server/callbacks/dataplane_lifecycle.go

Lines 130 to 134 in e21c751

	err := manager.Upsert(ctx, d.resManager, core_model.MetaToResourceKey(md.Resource.GetMeta()), proxyResource(md.GetProxyType()), func(existing core_model.Resource) error {
	if err := d.validateUpsert(ctx, md.Resource); err != nil {
	return errors.Wrap(err, "you are trying to override existing proxy to which you don't have an access.")
	}
	return existing.SetSpec(md.Resource.GetSpec())

Since only the universal DP created in the Strean flow:

kuma/pkg/xds/server/callbacks/dataplane_lifecycle.go

Lines 70 to 72 in e21c751

	if md.Resource == nil {
	return nil
	}

So I guess maybe we could sleep Cache ExpirationTime after we created the Universal DP object.
What do you think ? @jakubdyszkiewicz

[jakubdyszkiewicz]

Yeah, I think sleeping after creating Universal DP for cache expiration time makes sense.

Btw. it's important to do this only when we create the resource in lifecycle. On Kube, Dataplane object is created by the controller, not by the callback