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

Topology-aware hinting docs #1920

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Topology-aware hinting docs #1920

wants to merge 8 commits into from

Conversation

kflynn
Copy link
Member

@kflynn kflynn commented Feb 3, 2025

Originally from @deusxanima in #1579, thanks! 🙂

  • Adding feature documentation and task instructions for topology aware hints
  • cleaned up previously committed errors and modified phrasing as recommended by Flynn.
  • fixed minor typo
  • fixed missing code block notation on Hints.ForZones
  • Tweaks plus callouts for stable features in 2.16+

@kflynn kflynn requested a review from alpeb February 3, 2025 02:39
@kflynn
Copy link
Member Author

kflynn commented Feb 3, 2025

Forced DCO because Alen was a Buoyant employee when he wrote this...

@travisbeckham
Copy link
Collaborator

This is super nitpicky, but when we did the site rebuild, we switched all frontmatter to yaml. Prior to that, it was a mix of toml and yaml. I'll let you decide if we want to keep this consistent.

@kflynn
Copy link
Member Author

kflynn commented Feb 3, 2025

Nice catch! I didn't notice that at all.

Copy link
Collaborator

@travisbeckham travisbeckham left a comment

Choose a reason for hiding this comment

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

I left some minor comments on the 2.17 docs, which would also apply to all other versions.

the capacity of the nodes in the zone (which can be a concern when using
horizontal pod autoscaling, as the HPA may start nodes in the "wrong" zone);
and
- The Kubernetes endpoint slice controller does not take...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to be completed?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦‍♂️ Yes. Yes it does.

proxy logs for the relevant service. The logs should show a stream of messages
similar to the ones below:

```text
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd add text {class=disable-copy} here.


{{< note >}}

If you're using a [stable distribution](/releases/) of Linkerd, it may have
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we're already linking to Releases at the end of this paragraph, should we just remove this link?


{{< /note >}}

To get started with topology aware hints take a look at the [enabling topology
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we show the 2 notes and then end with this sentence? Linking off to the task page seems like a good way to end the doc.

---
title: Topology Aware Hints
description: |-
Linkerd's implementation of Kubernetes topology aware hints enables endpoint consumption based on a node's zone label.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only 2 spaces needed for indent.


{{< note >}}

If you're using a [stable distribution](/releases/) of Linkerd, it may have
Copy link
Collaborator

Choose a reason for hiding this comment

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

(See comment from task page)

---
title: Topology Aware Hints
description: |-
Enable topology aware hints to allow Linkerd to intelligently choose same-zone endpoints
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only 2 spaces needed for indent.

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