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

Added EKS-to-EKS Clustermesh Preparation guide #32355

Merged
merged 1 commit into from May 14, 2024
Merged

Added EKS-to-EKS Clustermesh Preparation guide #32355

merged 1 commit into from May 14, 2024

Conversation

network-charles
Copy link
Contributor

Added EKS-to-EKS Clustermesh Preparation guide.

@network-charles network-charles requested review from a team as code owners May 4, 2024 17:47
@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 May 4, 2024
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label May 4, 2024
@network-charles network-charles mentioned this pull request May 7, 2024
Open
giorio94
giorio94 approved these changes May 8, 2024
View reviewed changes
Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

Thanks, looks great to me!

I've left just a couple of minor nits inline. I wonder if it could make sense to generalize the Install cluster one/two parts as they are pretty similar, to avoid the duplication and prevent future divergences. WDYT?

Documentation/network/clustermesh/eks-clustermesh-prep.rst Outdated Show resolved Hide resolved
Documentation/network/clustermesh/eks-clustermesh-prep.rst Outdated Show resolved Hide resolved
Documentation/network/clustermesh/eks-clustermesh-prep.rst Outdated Show resolved Hide resolved
Documentation/network/clustermesh/eks-clustermesh-prep.rst Outdated Show resolved Hide resolved
@giorio94
Copy link
Member

giorio94 commented May 8, 2024

I forgot to mention that there's a linting error: https://github.com/cilium/cilium/actions/runs/8952241106/job/24589411130?pr=32355

@maintainer-s-little-helper
Copy link

Commit bbf7ceb does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

1 similar comment
@maintainer-s-little-helper
Copy link

Commit bbf7ceb does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 8, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 8, 2024
@network-charles
Copy link
Contributor Author

@giorio94, I was attempting to squash some selected commits on the eks-clustermesh-prep.rst file but ended up squashing the ones in other files, so I had to close the PR, fix it and re-open the PR again.

I modified the content in clustermesh.rst file so it references eks-clustermesh-prep.rst, let me know what you think.

@giorio94
Copy link
Member

giorio94 commented May 8, 2024

I was attempting to squash some selected commits on the eks-clustermesh-prep.rst file but ended up squashing the ones in other files, so I had to close the PR, fix it and re-open the PR again.

No problem. Could you please squash the two remaining commits together? And remove my Co-authored-by line (it got probably added automatically when accepting a suggestion)?

I modified the content in clustermesh.rst file so it references eks-clustermesh-prep.rst, let me know what you think.

Makes sense, thanks!

@network-charles
Copy link
Contributor Author

No problem. Could you please squash the two remaining commits together? And remove my Co-authored-by line (it got probably added automatically when accepting a suggestion)?

Done

@giorio94 giorio94 added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/clustermesh Relates to multi-cluster routing functionality in Cilium. release-note/misc This PR makes changes that have no direct user impact. labels May 8, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels May 8, 2024
@giorio94
Copy link
Member

giorio94 commented May 8, 2024

/test

@giorio94
Copy link
Member

It seems that Anna is OOO. Would someone else from @cilium/docs-structure have time for a review?

lambdanis
lambdanis reviewed May 13, 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.

Sorry for delay! Looks good to me overall, just left one comment about the spelling wordlist.

Documentation/spelling_wordlist.txt Outdated Show resolved Hide resolved
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels May 13, 2024
@maintainer-s-little-helper
Copy link

Commit df5134b does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 13, 2024
@network-charles
Created the EKS-to-EKS Clustermesh Prep Guide
24620cd 
- I created the prep guide.
- I referenced the guide on index.rst
- I added new words to spelling_wordlist.txt
- I referenced the guide on clustermesh.rst
Signed-off-by: Charles Uneze <charlesniklaus@gmail.com>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 13, 2024
@lambdanis
Copy link
Contributor

/test

@julianwiedmann julianwiedmann added this pull request to the merge queue May 14, 2024
Merged via the queue into cilium:main with commit ff817ee May 14, 2024
61 checks passed
@network-charles network-charles deleted the network-charles-patch-2 branch May 14, 2024 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lambdanis lambdanis lambdanis approved these changes

@giorio94 giorio94 giorio94 approved these changes

Assignees
No one assigned
Labels
area/clustermesh Relates to multi-cluster routing functionality in Cilium. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@network-charles @giorio94 @lambdanis @julianwiedmann