Use envoy xds server type in daily e2e tests

[sunjayBhatia]

Run e2e tests daily against envoy xds server type that uses
go-control-plane.

Once we are confident it is stable, we can flip the default to be the
envoy go-control-plane variant and run the daily tests with the contour
variant until we drop support for it.

[codecov]

Codecov Report

Merging #4100 (a725b08) into main (1ee363b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #4100   +/-   ##
=======================================
  Coverage   77.72%   77.72%           
=======================================
  Files         112      112           
  Lines        9995     9995           
=======================================
  Hits         7769     7769           
  Misses       2041     2041           
  Partials      185      185           

[sunjayBhatia]

looks like envoy has a hard time sometimes connecting to the local contour in the e2e tests in envoy grpc server mode

[sunjayBhatia]

[2021-10-21 21:34:29.342][1][warning][config] [source/common/config/grpc_subscription_impl.cc:118] gRPC config: initial fetch timed out for type.googleapis.com/envoy.config.listener.v3.Listener

[sunjayBhatia]

bumping the bootstrap connect timeout seems to have helped, it's hardcoded at 5s for now, maybe should be configurable

[sunjayBhatia]

going back through git history, seems like a 5s connect timeout was somewhat arbitrarily chosen

begs the question why the go-control-plane version takes longer to connect to, but we could bump it or make it configurable

[sunjayBhatia]

hm, even with bumping the connect timeout, seems fetches fail with the same Envoy log as above

[sunjayBhatia]

default "initial fetch timeout" is 15s, should be enough to fetch initial clusters and listeners, increasing we still get intermittently stuck on the initial fetch

[sunjayBhatia]

when leader election is disabled looks like the first e2e test always fails, we're not responding to the initial Cluster/Listener request at all, subsequent tests pass when the first Contour exits and Envoy reconnects to the instance that comes up for the next test

[sunjayBhatia]

Looks like the state of the world in how Contour sets up xDS etc. is as below, which explains why the e2e tests fail when you just swap over the xDS server type:

• With leader election enabled
  • The leader election process causes the event handler to be fired, even without any create/delete/change events to fire:

    contour/cmd/contour/leadership.go

    Line 71 in 2d52d47

    updateNow()

  • This ensures snapshots are generated for envoy xDS mode, everything "just works" bc the event handler populates the resources/cache the go-control-plane server watches are following
  • Envoy starts up just fine bc it gets the dynamic listener/route config needed to start up and healthcheck properly running in k8s
• With leader election disabled
  • In contour xDS mode, the server has direct access to xDS resources and the stream loop starts off in a state that allows it to respond to the first discovery request immediately, without needing to wait for a dag rebuild/event handler update:

    contour/internal/xds/v3/contour.go

    Line 93 in 2357d7e

    last := -1

  • In envoy xDS mode, nothing actually triggers the event handler on startup, except changes to resources in the cluster
    • This means in the e2e tests, Envoy is stuck waiting for the dynamic configs it needs to start up to ensure the pods are healthy, which never happens since there is no dag rebuild etc. triggered
    • go-control-plane watches updates to resources via snapshot generation, snapshot generation never happens bc event handler is never triggered

[sunjayBhatia]

in general contour startup with the envoy xDS server type is pretty racy, the server initialization has to happen strictly before a dag rebuild happens, otherwise any OnChange calls will not trigger a snapshot to be generated

it happens to work when leader election is enabled bc that coincidentally takes longer typically than xDS server initialization

[youngnick]

This sounds like another reason we need to fix up the initialization process and have a way to ensure the ordering for some goroutines?

[github-actions]

Marking this PR stale since there has been no activity for 14 days. It will be closed if there is no activity for another 30 days.

[github-actions]

Marking this PR stale since there has been no activity for 14 days. It will be closed if there is no activity for another 30 days.

[sunjayBhatia]

recent refactors have made #4100 (comment) obsolete, the event handler is now notified of leader election when it is enabled and disabled through the leadership notifier runnable that triggers it to run an update

[skriss]

LGTM.

One other idea here would be to set up a nightly run of the full E2E suite using the "envoy" control plane, to get regular full coverage on it without blowing up our E2E matrix for PRs.

[sunjayBhatia]

LGTM.

One other idea here would be to set up a nightly run of the full E2E suite using the "envoy" control plane, to get regular full coverage on it without blowing up our E2E matrix for PRs.

yeah i like that better actually, going to update to do that instead of the smoke tests

[skriss]

nice, LGTM.

I'm now thinking maybe we should re-jigger #4126 to also run its tests in this daily run, instead of running a small subset of the E2E's against an Envoy deployment for every PR, for similar reasons as for this one. Can ponder/tackle that separately.