Skip to content

Conversation

jrajahalme
Copy link
Member

Start serving xDS resources to Envoy as soon as policies for all restored endpoints have been computed, rather than waiting for them to be fully restored.

@jrajahalme jrajahalme requested review from a team as code owners December 7, 2024 14:37
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 7, 2024
@github-actions github-actions bot added the sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. label Dec 7, 2024
@jrajahalme jrajahalme added area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. release-note/misc This PR makes changes that have no direct user impact. and removed sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. labels Dec 7, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 7, 2024
@jrajahalme jrajahalme marked this pull request as draft December 7, 2024 14:37
@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from a61a122 to f3c7e69 Compare December 9, 2024 11:13
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from f3c7e69 to 8dd87a8 Compare December 9, 2024 11:43
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from 8dd87a8 to 4d36bda Compare December 9, 2024 13:54
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from 4d36bda to ac91218 Compare December 18, 2024 09:47
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from ac91218 to acae7a4 Compare December 18, 2024 11:18
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from acae7a4 to 608932e Compare December 18, 2024 11:39
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from 608932e to c1e09bb Compare December 20, 2024 18:51
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from c1e09bb to 60eb197 Compare December 20, 2024 20:05
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from 60eb197 to 3872308 Compare December 20, 2024 21:09
Copy link
Contributor

@doniacld doniacld left a comment

Choose a reason for hiding this comment

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

Great work! 👏🏽 LGTM with the last commit

Copy link
Member

@christarazi christarazi 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! A few comments to address.

Compute endpoint policy before scheduling bpf regeneration. This may
allow for faster initial proxy policy sync with Envoy and thus make
'toFQDNs' policies more responsive during Cilium Agent restarts.

With this endpoint's desiredPolicy may already be computed when the
endpoint is queued for regeneration. This has two consequences:

1. The version handle of the desired policy may not have been closed when
   endpoint is removed while it is waiting to be regenerated. Thus we need
   to mark the desired policy as 'Ready' when removing the endpoint to avoid
   warning logs.

2. Endpoint's policy may get updated while it is waiting to get
   regererated. If this happens the policy needs to be recomputed when
   endoint regeneration is started.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
…ation

Only pre-compute endpoint policy on first regeneration. This avoids extra
work if the endpoint policy gets updated while it is waiting to be
regenerated.

Start serving xDS resources to Envoy as soon as the policies of all
restored endopoints have been computed, rather than waiting for them to
have been fully restored. This allows Envoy to get policy updates sooner
on restart.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Add an optional stacktrace to VersionHandle, which is set if debug is
enabled. This is logged if a VersionHandle finalizer is needed for
cleanup and helps debug unclosed version handles.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Log the calling file and line in EndpointPolicy.Detach if the policy was
not marked as Ready at the time of the Detach call. This helps finding
out where the problem may lie.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme force-pushed the endpoint-policy-before-restoration branch from c45575c to f2be250 Compare January 11, 2025 18:04
@jrajahalme
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 14, 2025
@jrajahalme jrajahalme added release-blocker/1.17 This issue will prevent the release of the next version of Cilium. needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch kind/bug This is a bug in the Cilium logic. and removed affects/v1.17 This issue affects v1.17 branch labels Jan 14, 2025
@jrajahalme jrajahalme enabled auto-merge January 14, 2025 13:00
@jrajahalme jrajahalme added this pull request to the merge queue Jan 14, 2025
Merged via the queue into cilium:main with commit eb8072e Jan 14, 2025
66 checks passed
@jrajahalme jrajahalme deleted the endpoint-policy-before-restoration branch January 14, 2025 13:10
@rastislavs rastislavs mentioned this pull request Jan 21, 2025
45 tasks
@rastislavs rastislavs added backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. and removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Jan 21, 2025
@github-actions github-actions bot added backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. and removed backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. labels Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/v1.14 This issue affects v1.14 branch affects/v1.15 This issue affects v1.15 branch affects/v1.16 This issue affects v1.16 branch area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.17 This issue will prevent the release of the next version of Cilium. release-note/misc This PR makes changes that have no direct user impact.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants