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.

Sign up for GitHub

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

Jump to bottom

toFQDNs: Add documention and metrics for fqdn identities #33237

Merged
merged 5 commits into from
Jun 25, 2024
Merged

toFQDNs: Add documention and metrics for fqdn identities #33237

merged 5 commits into from
Jun 25, 2024

Conversation

gandro
Copy link
Member

@gandro gandro commented Jun 18, 2024

This PR:

@gandro gandro added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. area/fqdn Affects the FQDN policies feature labels Jun 18, 2024
@github-actions github-actions bot added the sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. label Jun 18, 2024
@gandro gandro changed the title Pr/gandro/dns identities metrics toFQDNs: Add documention and metrics for fqdn identities Jun 18, 2024
@gandro gandro force-pushed the pr/gandro/dns-identities-metrics branch from 68760c5 to e863cd4 Compare June 18, 2024 16:34
@gandro gandro marked this pull request as ready for review June 18, 2024 16:37
@gandro gandro requested review from a team as code owners June 18, 2024 16:37
@gandro gandro added the release-blocker/1.16 This issue will prevent the release of the next version of Cilium. label Jun 18, 2024
@gandro
Copy link
Member Author

gandro commented Jun 19, 2024

/test

@gandro gandro mentioned this pull request Jun 19, 2024
Closed
lambdanis
lambdanis approved these changes Jun 19, 2024
View reviewed changes
Copy link
Contributor

@lambdanis lambdanis left a comment

Choose a reason for hiding this comment

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

Left one comment, but docs good otherwise

@gandro
Copy link
Member Author

gandro commented Jun 19, 2024

Left one comment, but docs good otherwise

Thanks for the review - I think the comment got lost somewhere?

lambdanis
lambdanis approved these changes Jun 19, 2024
View reviewed changes
Copy link
Contributor

@lambdanis lambdanis left a comment

Choose a reason for hiding this comment

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

I think the comment got lost somewhere?

sorry, I don't know how that happened

Documentation/spelling_wordlist.txt Outdated Show resolved Hide resolved
tommyp1ckles
tommyp1ckles reviewed Jun 20, 2024
View reviewed changes
pkg/metrics/metrics.go Show resolved Hide resolved
pkg/fqdn/name_manager.go Outdated Show resolved Hide resolved
@gandro gandro force-pushed the pr/gandro/dns-identities-metrics branch from e863cd4 to 8e20c34 Compare June 20, 2024 08:15
@gandro
Copy link
Member Author

gandro commented Jun 20, 2024

/test

@gandro gandro mentioned this pull request Jun 20, 2024
Closed
@joestringer joestringer added this to the 1.16 milestone Jun 20, 2024
tommyp1ckles
tommyp1ckles approved these changes Jun 21, 2024
View reviewed changes
Copy link
Contributor

@tommyp1ckles tommyp1ckles left a comment

Choose a reason for hiding this comment

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

approved for metrics changes

@squeed
Copy link
Contributor

squeed commented Jun 21, 2024

Nice!

aditighag
aditighag approved these changes Jun 24, 2024
View reviewed changes
Copy link
Member

@aditighag aditighag left a comment

Choose a reason for hiding this comment

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

LGTM, except for a couple of typos

Documentation/operations/upgrade.rst Outdated Show resolved Hide resolved
Documentation/operations/upgrade.rst Outdated Show resolved Hide resolved
@gandro
metrics: Add identity_label_sources metric
a697d60 
This adds a new metric which counts the number of identities per label
source. This allows users to have a bit more precise breakdown of what
types of identities are allocated over the existing `identities`
metrics. For example, the new metric allows users to track precisely how
many identities contain a `fqdn` or `cidr` label, where as the per-type
metric puts them in the same bucket.

There are only about a dozen different label sources, so cardinality of
the metric should be low.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro
metrics: Add fqdn_selectors metric
c11a4f6 
This adds a new simple metric which counts the number of registered
`ToFQDN` selectors. This, in combination with the previously added
`identity_label_sources` metric, allows users to monitor how many `fqdn`
identities are allocated compared to how may `ToFQDN` selectors are
registered. If there are orders of magnitude more identities than
selectors, then this indicates that selectors are overlapping in
different combinations, which can cause the local identity space to
exhaust quickly.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro
docs: Update troubleshooting guides for ToFQDN
6c7fe6f 
This commit updates our docs to use the new `fqdn` identities introduced
by commit 719eb4f - rather than the
previously used `cidr` identities.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro force-pushed the pr/gandro/dns-identities-metrics branch from 8e20c34 to 86e48f2 Compare June 24, 2024 14:59
@gandro
Copy link
Member Author

gandro commented Jun 24, 2024

/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 Jun 25, 2024
doniacld
doniacld approved these changes Jun 25, 2024
View reviewed changes
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.

LGTM, only one typo!

@gandro
docs: Add section on toFQDNs metrics
84b7ced 
This adds a section in the `toFQDNs` troubleshooting guide on how the
identity usage can be monitored. It makes use of the metrics added in
previous commits.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro
docs: Mention new toFQDNs implementation in upgrade notes
40fd37e 
This adds the upgrade notes for the new ``toFQDNs`` implementation. It
mentions upgrade impact and the new metrics added to troubleshoot it.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro force-pushed the pr/gandro/dns-identities-metrics branch from 86e48f2 to 40fd37e Compare June 25, 2024 08:14
@gandro
Copy link
Member Author

gandro commented Jun 25, 2024

/test

@gandro gandro added this pull request to the merge queue Jun 25, 2024
Merged via the queue into cilium:main with commit 638be8f Jun 25, 2024
66 checks passed
@gandro gandro deleted the pr/gandro/dns-identities-metrics branch June 25, 2024 10:30
@joestringer joestringer added the backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. label Jun 28, 2024
@joestringer joestringer mentioned this pull request Jun 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nathanjsweet nathanjsweet nathanjsweet approved these changes

@lambdanis lambdanis lambdanis approved these changes

@tommyp1ckles tommyp1ckles tommyp1ckles approved these changes

@doniacld doniacld doniacld approved these changes

@aditighag aditighag aditighag approved these changes

Assignees

@gandro gandro

Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/fqdn Affects the FQDN policies feature backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.16 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. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
Release blockers
Archived in project
Milestone
1.16
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@gandro @squeed @nathanjsweet @lambdanis @tommyp1ckles @doniacld @aditighag @joestringer