Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix local mesh gateway with peering discovery chains. #15690

Merged
merged 2 commits into from
Dec 7, 2022

Conversation

hashi-derek
Copy link
Member

@hashi-derek hashi-derek commented Dec 6, 2022

Prior to this patch, discovery chains with peer-targets would not properly honor the mesh gateway mode for two reasons.

  1. An incorrect target upstream ID was used to lookup the mesh gateway mode. To fix this, the parent upstream uid is now used instead of the discovery-chain-target-uid to find the intended mesh gateway mode.

  2. The watch for local mesh gateways was never initialized for discovery chains. To fix this, the discovery chains are now scanned, and a local GW watch is spawned if: the mesh gateway mode is local and the target is a peering connection.

@github-actions github-actions bot added the theme/envoy/xds Related to Envoy support label Dec 6, 2022
Prior to this patch, discovery chains with peers would not
properly honor the mesh gateway mode for two reasons.

1. An incorrect target upstream ID was used to lookup the
mesh gateway mode. To fix this, the parent upstream uid is
now used instead of the discovery-chain-target-uid to find
the intended mesh gateway mode.

2. The watch for local mesh gateways was never initialized
for discovery chains. To fix this, the discovery chains are
now scanned, and a local GW watch is spawned if: the mesh
gateway mode is local and the target is a peering connection.
@hashi-derek hashi-derek force-pushed the derekm/NET-1762/fix-localmgw-failovers branch from a421813 to 8aff79e Compare December 6, 2022 17:21
@vercel
Copy link

vercel bot commented Dec 6, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated
consul ⬜️ Ignored (Inspect) Dec 6, 2022 at 5:26PM (UTC)
consul-ui-staging ⬜️ Ignored (Inspect) Dec 6, 2022 at 5:26PM (UTC)

@hashi-derek hashi-derek requested review from a team and freddygv and removed request for a team December 6, 2022 18:24
@hashi-derek hashi-derek marked this pull request as ready for review December 6, 2022 18:24
Copy link
Contributor

@freddygv freddygv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work figuring this out! I left a comment below about where the discovery chain inspection is happening.

By the way, would be good to update state_test.go to verify this is working. Could add something like this to verifySnapshot of stage0 and stage1 of newConnectProxyCase.

if meshGatewayProxyConfigValue == structs.MeshGatewayModeLocal {
	require.True(t, snap.ConnectProxy.WatchedLocalGWEndpoints.IsWatched("dc1"))
	_, ok := snap.ConnectProxy.WatchedLocalGWEndpoints.Get("dc1")
	require.False(t, ok)
}

There is an upstream there called api-failover-to-peer which is impacted by this issue. I tried adding this block to the main branch and the test failed.

agent/proxycfg/upstreams.go Outdated Show resolved Hide resolved
agent/proxycfg/upstreams.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/envoy/xds Related to Envoy support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants