Improve EG Gateway xDS & startup Reliability (custom k8s health prob)

[alexwo]

The proposed enhancement involves modifying the controller's "ready" status to accurately reflect the completion and synchronization of xds discovery processes.

Specifically, the "ready" status indicator can transition to "true" when the xDS discovery has fully completed to store it's initial snapshot or when there is no reconciliation required. (empty or new deployment).

This can ensure that envoy proxies are always in sync with latest xDS service and that an EG that has started is able to reconcile.

-> If there is nothing to reconcile -> ready = true
-> if there are changes to reconcile, wait for xDS to complete -> ready = true
-> other wise -> ready = false

Currently, there may be certain cases where xDS is not completely synchronized at startup, which could cause new Envoy proxies to work with an incomplete xDS.

This can provide better guarantees that an operational EG consistently maintains an updated xDS, potentially also can allow avoiding situations where instances startup but fail during the initial reconcile.

Leader Election and multiple instances use case:
Will improve consistency in environments where multiple instances of EG run simultaneously by ensuring they start only once xDS server has persisted the latest state snapshot. #1953

[arkodg]

makes sense, a workaround for this until this is implemented is to wake up slowly i.e. set initialDelaySeconds to a higher value

[alexwo]

please assign to me

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

[aoledk]

makes sense, a workaround for this until this is implemented is to wake up slowly i.e. set initialDelaySeconds to a higher value

@arkodg But currently EG hasn't exposed initialDelaySeconds via Chart Values in

gateway/charts/gateway-helm/templates/envoy-gateway-deployment.yaml

Lines 66 to 71 in 78fe57a

	readinessProbe:
	httpGet:
	path: /readyz
	port: 8081
	initialDelaySeconds: 5
	periodSeconds: 10

[arkodg]

I was referring to initialDelaySeconds for envoy proxy (data plane) so it gets enough time to receive xds before receiving traffic (from external LB) once signaling its ready

[aoledk]

Since EG is xDS resources provider and envoy is consumer, setting longer initialDelaySeconds for envoy can't prevent envoy from receiving incomplete xDS resources when EG is starting. This will lead to frequent envoy listener draining.

Workaround here maybe set longer initialDelaySeconds for EG to make sure envoy always consume complete xDS resources from EG.

[aoledk]

@arkodg As a workaround, could we make this EG readiness initialDelaySeconds configurable via helm chart. I can help with this.

[arkodg]

@aoledk if EG's cache is not ready yet but the xds server is ready, we send an empty response

gateway/internal/xds/cache/snapshotcache.go

Line 202 in 92760c8

// If no snapshot has been generated yet, we can't do anything, so don't mess with this request.

will this cause the proxy listeners to drain ?

[aoledk]

@arkodg if cache is not ready at all (non-exist), EG will return nil and not set snapshot for envoy, only set snapshot will trigger sending xDS resources to envoy. So envoy will use its current active xDS resources instead of empty xDS resources, no listener drain.

if cache is partly ready (starting EG is still reconciling objects), EG will set partly snapshot for envoy. Then envoy will receive partly xDS resources to replace its current active complete xDS resources, maybe leading to listener drain ¹, specially when there are ClientTrafficPolicies not be reconciled.

gateway/internal/xds/cache/snapshotcache.go

Lines 202 to 214 in 33fceb0

	// If no snapshot has been generated yet, we can't do anything, so don't mess with this request.
	// go-control-plane will respond with an empty response, then send an update when a snapshot is generated.
	if s.lastSnapshot[cluster] == nil {
	return nil
	}
	_, err := s.GetSnapshot(nodeID)
	if err != nil {
	err = s.SetSnapshot(context.TODO(), nodeID, s.lastSnapshot[cluster])
	if err != nil {
	return err
	}
	}

Footnotes

1. https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/listeners/listener_filters#filter-chain-only-update ↩

[arkodg]

ah, so afaik the part ready case shouldnt happen because there is one big reconciler,

gateway/internal/provider/kubernetes/controller.go

Line 160 in 33fceb0

func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) {

so if we reconcile once, we should have state of the world of all resources, until then, there are no resources and no xds o/p in the cache. Is there an incorrect assumption made here ?

[aoledk]

@arkodg It's my mistake, and you're right, partly ready xDS resources will never be generated under this big reconciler.

[arkodg]

thanks for cross checking and brainstorming the edge cases @aoledk !

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

[arkodg]

can we close this @alexwo

[guydc]

To consider: move the current healthcheck listener and filter chain from static config (bootstrap) to dynamic config generated by EG. This would mean that readdiness checks would only pass once the proxy was programmed at least once.
cc @arkodg, @liorokman @alexwo

[arkodg]

great idea @guydc , big +1

[zirain]

+1, this will also reduce the size of bootstrap configuration.