fix go-control-plane instantiation order #5972
Conversation
| // the contents of the Contour xDS caches after the DAG is built. | ||
| snapshotHandler := xdscache_v3.NewSnapshotHandler( | ||
| resources, | ||
| envoy_cache_v3.NewSnapshotCache(false, &contour_xds_v3.Hash, s.log.WithField("context", "snapshotCache")), |
There was a problem hiding this comment.
This is the important change, the snapshot handler is now connected to the snapshot cache before any runnables are started.
| @@ -1,56 +0,0 @@ | |||
| // Copyright Project Contour Authors | |||
There was a problem hiding this comment.
This seemed like an unnecessary layer of indirection between the snapshot handler and snapshot cache. Those two are now directly connected.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #5972 +/- ##
==========================================
+ Coverage 78.59% 78.64% +0.05%
==========================================
Files 139 138 -1
Lines 19638 19625 -13
==========================================
Hits 15434 15434
+ Misses 3901 3888 -13
Partials 303 303
|
skriss
added
the
release-note/small
skriss
requested review from
tsaarni,
stevesloka and
sunjayBhatia
and removed request for
a team
|
Ran through the E2E's a number of times on this PR and #5973 (which includes this change), had a couple seemingly unrelated flakes but overall looks good with many successful runs. I'm going to revert the E2E config change now. |
Previously, the go-control-plane snapshot cache was not instantiated and wired up to the snapshot handler (via a snapshotter) until after the first DAG run was completed and during the xDS server startup. As a result, the first DAG run never populated the go-control plane snapshot cache, so the go-control-plane xDS server had nothing to respond to initial discovery requests with. This moves the wiring of the snapshot handler to the snapshot cache up to where the snapshot handler itself is instantiated, before any runnables are started. It also removes the unneeded snapshotter layer of indirection. The snapshot handler now directly generates a go-control-plane snapshot and stores it in the snapshot cache. Signed-off-by: Steve Kriss <krisss@vmware.com>
skriss
force-pushed
the
exp-go-control-plane-race
branch
from
4b0ba66 to
ed23722
Compare
There was a problem hiding this comment.
Been playing around with this change this morning, adding some log lines in various places to watch contour startup in more detail, and as far as I can tell this looks correct
👍🏽 Should be able to fix the daily build e2e failures that happen on startup of contour. Nice work!
Previously, the go-control-plane snapshot cache was not instantiated and wired up to the snapshot handler (via a snapshotter) until after the first DAG run was completed and during the xDS server startup. As a result, the first DAG run never populated the go-control plane snapshot cache, so the go-control-plane xDS server had nothing to respond to initial discovery requests with.
This moves the wiring of the snapshot handler to the snapshot cache up to where the snapshot handler itself is instantiated, before any runnables are started. It also removes the unneeded snapshotter layer of indirection. The snapshot handler now directly generates a go-control-plane snapshot and stores it in the snapshot cache.