Skip to content

Conversation

ArthurChiao
Copy link
Contributor

@ArthurChiao ArthurChiao commented Feb 25, 2020

ref #10160

Signed-off-by: ArthurChiao arthurchiao@hotmail.com


This change is Reviewable

@ArthurChiao ArthurChiao requested a review from a team as a code owner February 25, 2020 02:59
@maintainer-s-little-helper
Copy link

Release note label not set, please set the appropriate release note.

@coveralls
Copy link

coveralls commented Feb 25, 2020

Coverage Status

Coverage increased (+0.007%) to 45.519% when pulling 968de865a7e87298df7e44cf3661a62bcc52d49f on ctripcloud:bird_guide into 43f7824 on cilium:master.

@ArthurChiao ArthurChiao changed the title add getting started guide for BIRD WIP: add getting started guide for BIRD Feb 25, 2020
@pchaigno pchaigno added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. pending-review and removed pending-review labels Feb 25, 2020
@aanm aanm added pending-review release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Feb 25, 2020
@aanm
Copy link
Member

aanm commented Feb 25, 2020

test-docs-please

@ArthurChiao ArthurChiao changed the title WIP: add getting started guide for BIRD add getting started guide for BIRD Feb 26, 2020
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Thanks @ArthurChiao! This is great 👍

I haven't tested it yet, but I found a few nits (see below) and a couple warnings returned by Sphinx when I run make render-docs to see the Read the docs rendering. I have written these as GitHub suggestions whenever possible, in case that makes it easier for you to integrate and then just rebase :-)

Could you also add this guide to Documentation/gettingstarted/index.rst, under Advanced Networking?

@maintainer-s-little-helper
Copy link

Commit 9cd69cf215cf3a87e23a3d9696461bbeab3f63c1 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/contributing/#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 Feb 26, 2020
@maintainer-s-little-helper
Copy link

Commits 9cd69cf215cf3a87e23a3d9696461bbeab3f63c1, 3e22ddc3ad742fe86566be4f19ccdd4b5a799b2a do not contain "Signed-off-by".

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

3 similar comments
@maintainer-s-little-helper
Copy link

Commits 9cd69cf215cf3a87e23a3d9696461bbeab3f63c1, 3e22ddc3ad742fe86566be4f19ccdd4b5a799b2a do not contain "Signed-off-by".

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

@maintainer-s-little-helper
Copy link

Commits 9cd69cf215cf3a87e23a3d9696461bbeab3f63c1, 3e22ddc3ad742fe86566be4f19ccdd4b5a799b2a do not contain "Signed-off-by".

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

@maintainer-s-little-helper
Copy link

Commits 9cd69cf215cf3a87e23a3d9696461bbeab3f63c1, 3e22ddc3ad742fe86566be4f19ccdd4b5a799b2a do not contain "Signed-off-by".

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

@maintainer-s-little-helper
Copy link

Commits 9cd69cf215cf3a87e23a3d9696461bbeab3f63c1, 3e22ddc3ad742fe86566be4f19ccdd4b5a799b2a, 5ae1bb146765074c247ddb5a3b6707d5325628dc, 45490bdea80e9e6c78c2d1ad169830a95e7dd5d8 do not contain "Signed-off-by".

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

2 similar comments
@maintainer-s-little-helper
Copy link

Commits 9cd69cf215cf3a87e23a3d9696461bbeab3f63c1, 3e22ddc3ad742fe86566be4f19ccdd4b5a799b2a, 5ae1bb146765074c247ddb5a3b6707d5325628dc, 45490bdea80e9e6c78c2d1ad169830a95e7dd5d8 do not contain "Signed-off-by".

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

@maintainer-s-little-helper
Copy link

Commits 9cd69cf215cf3a87e23a3d9696461bbeab3f63c1, 3e22ddc3ad742fe86566be4f19ccdd4b5a799b2a, 5ae1bb146765074c247ddb5a3b6707d5325628dc, 45490bdea80e9e6c78c2d1ad169830a95e7dd5d8 do not contain "Signed-off-by".

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

@ArthurChiao
Copy link
Contributor Author

hi @pchaigno , big thanks for your advises!

I have resolved them except two:

  1. two pictures seems too large applying scale 70%
  2. whether should i add sed commands to replace the placeholders in bird.conf

@maintainer-s-little-helper
Copy link

Commits 2899518813f2c09ed83c2b26044d9f8325eac295, 6f688cb145abdd9b239b0a9637edcadcbcdff26f, 4fdc010f06d3980563940a66edf16666750e3c6a do not contain "Signed-off-by".

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

2 similar comments
@maintainer-s-little-helper
Copy link

Commits 2899518813f2c09ed83c2b26044d9f8325eac295, 6f688cb145abdd9b239b0a9637edcadcbcdff26f, 4fdc010f06d3980563940a66edf16666750e3c6a do not contain "Signed-off-by".

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

@maintainer-s-little-helper
Copy link

Commits 2899518813f2c09ed83c2b26044d9f8325eac295, 6f688cb145abdd9b239b0a9637edcadcbcdff26f, 4fdc010f06d3980563940a66edf16666750e3c6a do not contain "Signed-off-by".

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

@maintainer-s-little-helper
Copy link

Commits 2899518813f2c09ed83c2b26044d9f8325eac295, 6f688cb145abdd9b239b0a9637edcadcbcdff26f, 4fdc010f06d3980563940a66edf16666750e3c6a, 577200131e61f8b76e6281f5504e0a3ae7d06090, 87514dbfa03e7e8009c8047f8c828942c5f5935d do not contain "Signed-off-by".

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

3 similar comments
@maintainer-s-little-helper
Copy link

Commits 2899518813f2c09ed83c2b26044d9f8325eac295, 6f688cb145abdd9b239b0a9637edcadcbcdff26f, 4fdc010f06d3980563940a66edf16666750e3c6a, 577200131e61f8b76e6281f5504e0a3ae7d06090, 87514dbfa03e7e8009c8047f8c828942c5f5935d do not contain "Signed-off-by".

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

@maintainer-s-little-helper
Copy link

Commits 2899518813f2c09ed83c2b26044d9f8325eac295, 6f688cb145abdd9b239b0a9637edcadcbcdff26f, 4fdc010f06d3980563940a66edf16666750e3c6a, 577200131e61f8b76e6281f5504e0a3ae7d06090, 87514dbfa03e7e8009c8047f8c828942c5f5935d do not contain "Signed-off-by".

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

@maintainer-s-little-helper
Copy link

Commits 2899518813f2c09ed83c2b26044d9f8325eac295, 6f688cb145abdd9b239b0a9637edcadcbcdff26f, 4fdc010f06d3980563940a66edf16666750e3c6a, 577200131e61f8b76e6281f5504e0a3ae7d06090, 87514dbfa03e7e8009c8047f8c828942c5f5935d do not contain "Signed-off-by".

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

@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 Feb 26, 2020
@ArthurChiao
Copy link
Contributor Author

Indeed i missed the hidden ones just now @pchaigno

ref cilium#10160

Signed-off-by: ArthurChiao <arthurchiao@hotmail.com>
@pchaigno
Copy link
Member

test-docs-please

@borkmann
Copy link
Member

@ArthurChiao can this be backported to 1.7 as well, meaning, is it confirmed to work with 1.7 so it could go into the stable doc? Thx

@ArthurChiao
Copy link
Contributor Author

ArthurChiao commented Feb 27, 2020

@borkmann yes, i have tested Cilium 1.7.0 + bird 2.0.6 (but just tested one node), the scheme works ok.

Actually in my understanding, Cilium and bird works independently, they collaborate through kernel routing tables/facilities, so as long as they do not break their kernel conventions, each of them could be upgraded/downgraded transparently - or even, be replaced by their alternatives (that's how we rolling-replaced kube-router with bird).

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

This is awesome!

Did you consider whether it is feasible to deploy BIRD as a DaemonSet rather than configuring each node? Just wondering if there's an easier configuration path there.

Either way, the nice thing about this guide is that it explains the various settings that must be made and at least provides the first round of documentation. That's valuable in itself.

Full disclosure: I didn't try this guide out.

@ArthurChiao
Copy link
Contributor Author

ArthurChiao commented Feb 27, 2020

@joestringer that's nice way for automation, but at present there are many challenges that may threaten the availability (both bird itself and the whole cluster) instead of strengthen it, such as:

  1. bird daemon on each node has different configurations - with respect to your BGP peering scheme, these differences may include router ID, PodCIDR, peer ASN, peer IP, peer password, etc. I assume Daemonset is good at handling services with much the same configuration (e.g. via configmap)?
  2. if Daemonset encountered some problems (e.g. problematic yaml), all bird daemons may exit and could not be re-launched, in this case, all announced routes will be withdrawn, then not only new Pods could not be deployed, but also all running Pods will be disconnected - the whole cluster suffers.

For these reason, we prefer standalone deployment of bird currently. However, i will consider your suggested way, thanks!

@aanm aanm requested a review from qmonnet February 27, 2020 10:18
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. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants