Skip to content

Backport of Fix issue with peer stream node cleanup. into release/1.15.x#17247

Merged
hashi-derek merged 3 commits intorelease/1.15.xfrom
backport/derekm/NET-3007/fix-peer-stream-cleanup/immensely-well-sunbird
May 8, 2023
Merged

Backport of Fix issue with peer stream node cleanup. into release/1.15.x#17247
hashi-derek merged 3 commits intorelease/1.15.xfrom
backport/derekm/NET-3007/fix-peer-stream-cleanup/immensely-well-sunbird

Conversation

@hc-github-team-consul-core
Copy link
Collaborator

Backport

This PR is auto-generated from #17235 to be assessed for backporting due to the inclusion of the label backport/1.15.

The below text is copied from the body of the original PR.


This commit encompasses a few problems that are closely related due to their proximity in the code.

  1. The peerstream utilizes node IDs in several locations to determine which nodes / services / checks should be cleaned up or created. While VM deployments with agents will likely always have a node ID, agentless uses synthetic nodes and does not populate the field. This means that for consul-k8s deployments, all services were likely bundled together into the same synthetic node in some code paths (but not all), resulting in strange behavior. The Node.Node field should be used instead as a unique identifier, as it should always be populated.

  2. The peerstream cleanup process for unused nodes uses an incorrect query for node deregistration. This query is NOT namespace aware and results in the node (and corresponding services) being deregistered prematurely whenever it has zero default-namespace services and 1+ non-default-namespace services registered on it. This issue is tricky to find due to the incorrect logic mentioned in 1, combined with the fact that the affected services must be co-located on the same node as the currently deregistering service for this to be encountered.

  3. The stream tracker did not understand differences between services in different namespaces and could therefore report incorrect numbers. It was updated to utilize the full service name to avoid conflicts and return proper results.


Overview of commits

@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/derekm/NET-3007/fix-peer-stream-cleanup/immensely-well-sunbird branch 2 times, most recently from 5ccaf4b to c5486f8 Compare May 8, 2023 18:13
@hc-github-team-consul-core hc-github-team-consul-core enabled auto-merge (squash) May 8, 2023 18:13
@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/derekm/NET-3007/fix-peer-stream-cleanup/immensely-well-sunbird branch from a860175 to 0475a1d Compare May 8, 2023 18:13
Copy link
Collaborator

Choose a reason for hiding this comment

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

Auto approved Consul Bot automated PR

@hashi-derek hashi-derek disabled auto-merge May 8, 2023 18:18
@hashi-derek hashi-derek merged commit 3a1ea22 into release/1.15.x May 8, 2023
@hashi-derek hashi-derek deleted the backport/derekm/NET-3007/fix-peer-stream-cleanup/immensely-well-sunbird branch May 8, 2023 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants