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

fix: locality loadbalance doc #101

Merged
merged 1 commit into from
Dec 30, 2024
Merged

fix: locality loadbalance doc #101

merged 1 commit into from
Dec 30, 2024

Conversation

cr7258
Copy link
Contributor

@cr7258 cr7258 commented Dec 29, 2024

No description provided.

@kmesh-bot
Copy link
Collaborator

Welcome @cr7258! It looks like this is your first PR to kmesh-net/website 🎉

@netlify Netlify
Copy link

netlify bot commented Dec 29, 2024 •
edited
Loading

✅ Deploy Preview for kmesh-net ready!

Name Link
🔨 Latest commit 97b2dab
🔍 Latest deploy log https://app.netlify.com/sites/kmesh-net/deploys/677164fe67d5db0008fa1483
😎 Deploy Preview https://deploy-preview-101--kmesh-net.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@cr7258 cr7258 force-pushed the fix-locality-loadbalance-doc branch from 301f6e6 to 97b2dab Compare December 29, 2024 15:04
@hzxuzhonghu
Copy link
Member

THank you @cr7258

hzxuzhonghu
hzxuzhonghu reviewed Dec 30, 2024
View reviewed changes
content/en/docs/userguide/locality_loadbalance.md
@@ -288,12 +290,13 @@ $ Hello version: region.zone1.subzone2, instance: helloworld-region.zone1.subzon

- Relabel the locality of the ambient-worker3 same as the worker2 and test.
```
kubectl label node ambient-worker3 topology.kubernetes.io/region=region
Copy link
Member

Choose a reason for hiding this comment

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

why do you remove this region label

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the region label has already been added in the beginning:

website/content/en/docs/userguide/locality_loadbalance.md

Lines 83 to 85 in 97b2dab

kubectl label node ambient-worker3 topology.kubernetes.io/region=region
kubectl label node ambient-worker3 topology.kubernetes.io/zone=zone2
kubectl label node ambient-worker3 topology.kubernetes.io/subzone=subzone3

Here we want to change the locality of ambient-worker3, so we only need to adjust the zone and subzone labels.

hzxuzhonghu
hzxuzhonghu approved these changes Dec 30, 2024
View reviewed changes
Copy link
Member

@hzxuzhonghu hzxuzhonghu left a comment

Choose a reason for hiding this comment

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

/lgtm

This makes it work

@kmesh-bot kmesh-bot added the lgtm label Dec 30, 2024
@kmesh-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kmesh-bot kmesh-bot merged commit c111dd0 into kmesh-net:main Dec 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hzxuzhonghu hzxuzhonghu hzxuzhonghu approved these changes

@nlgwcy nlgwcy Awaiting requested review from nlgwcy

Assignees

@hzxuzhonghu hzxuzhonghu

Labels
approved lgtm size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cr7258 @kmesh-bot @hzxuzhonghu