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

Add a note about conflicting node CIDRs #20204 #20208

Merged
merged 1 commit into from
Jun 16, 2022
Merged

Conversation

wokalski
Copy link

@wokalski wokalski commented Jun 15, 2022

Fixes #20204

Signed-off-by: Wojtek Czekalski me@wczekalski.com

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

Fixes: #20204

I am not a great doc writer but I hope this is enough to get the ball rolling. My team collectively spent 1 day debugging this so I hope we can save others from it.

@wokalski wokalski requested a review from a team June 15, 2022 06:54
@wokalski wokalski requested a review from a team as a code owner June 15, 2022 06:54
@wokalski wokalski requested review from twpayne and qmonnet June 15, 2022 06:54
@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 Jun 15, 2022
Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

Thanks! One minor correction needed

Documentation/concepts/networking/ipam/cluster-pool.rst Outdated Show resolved Hide resolved
@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. labels Jun 15, 2022
@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 Jun 15, 2022
@gandro
Copy link
Member

gandro commented Jun 15, 2022

Thanks for addressing the feedback. Could you please squash all commits into a single commit?

Signed-off-by: Wojtek Czekalski <me@wczekalski.com>
@wokalski
Copy link
Author

Could you please squash all commits into a single commit?

Done.

Thanks everyone for the quick turnaround. Such open source leadership makes me really bullish about the project!

@kkourt kkourt requested a review from qmonnet June 16, 2022 07:46
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@qmonnet qmonnet added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 16, 2022
@aanm aanm merged commit 590388d into cilium:master Jun 16, 2022
@wokalski wokalski deleted the patch-1 branch June 16, 2022 21:00
@qmonnet qmonnet added backport-done/1.10 backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.10 labels Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. 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
Development

Successfully merging this pull request may close these issues.

[Docs] By default Cilium doesn't work if node is in 10.0.0.0/24 network
7 participants