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

Option for MetalLB to talk BGP #6383

Merged
merged 5 commits into from
Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions inventory/sample/group_vars/k8s-cluster/addons.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,11 @@ metallb_enabled: false
# - "10.5.1.50-10.5.1.99"
# protocol: "layer2"
# auto_assign: false
# metallb_protocol: "bgp"
# metallb_peers:
# - peer_address: 192.0.2.1
# peer_asn: 64512
# my_asn: 4200000000
# - peer_address: 192.0.2.2
# peer_asn: 64513
# my_asn: 4200000000
10 changes: 6 additions & 4 deletions roles/kubernetes-apps/metallb/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
MetalLB hooks into your Kubernetes cluster, and provides a network load-balancer implementation.
In short, it allows you to create Kubernetes services of type "LoadBalancer" in clusters that
don't run on a cloud provider, and thus cannot simply hook into paid products to provide load-balancers.
This addon aims to automate [this](https://metallb.universe.tf/concepts/layer2/).
It deploys MetalLB into Kubernetes and sets up a layer 2 load-balancer.
This addon aims to automate [MetalLB in layer 2 mode](https://metallb.universe.tf/concepts/layer2/)
and/or [MetalLB in BGP mode][https://metallb.universe.tf/concepts/bgp/].
Copy link
Contributor

Choose a reason for hiding this comment

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

@Miouge1 has a good point.
Here says we can enable both modes with and/or word but actually metallb_protocol can accept a single protocol only.
It is nice to have more discussion.

/lgtm cancel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

metallb_additional_address_pools expect a protocol for each additional address pool, so "and/or" is correct as far as kubespray goes, but I've removed both instances of "and/".
Like @EppO said having both at once doesn't really make any practical sense.

It deploys MetalLB into Kubernetes and sets up a layer 2 and/or BGP load-balancer.

## Install

In the default, MetalLB is not deployed into your Kubernetes cluster.
You can override the defaults by copying the contents of this file to somewhere in inventory/mycluster/group_vars
such as inventory/mycluster/groups_vars/k8s-cluster/addons.yml and updating metallb_enabled option to `true`.
You can override the defaults by copying the contents of roles/kubernetes-apps/metallb/defaults/main.yml
to somewhere in inventory/mycluster/group_vars such as inventory/mycluster/groups_vars/k8s-cluster/addons.yml
and updating metallb_enabled option to `true`.
In addition you need to update metallb_ip_range option on the addons.yml at least for suiting your network
environment, because MetalLB allocates external IP addresses from this metallb_ip_range option.
6 changes: 6 additions & 0 deletions roles/kubernetes-apps/metallb/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
when:
- metallb_ip_range is not defined or not metallb_ip_range

- name: Kubernetes Apps | Check BGP peers for MetalLB
fail:
msg: "metallb_peers is mandatory when metallb_protocol is bgp"
when:
- metallb_protocol == 'bgp' and metallb_peers is not defined

- name: Kubernetes Apps | Check AppArmor status
command: which apparmor_parser
register: apparmor_status
Expand Down
8 changes: 8 additions & 0 deletions roles/kubernetes-apps/metallb/templates/metallb-config.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ metadata:
name: config
data:
config: |
{% if metallb_protocol == 'bgp' %}
peers:
{% for peer in metallb_peers %}
Copy link
Contributor

Choose a reason for hiding this comment

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

If metallb_protocol is bgp, is metallb_peers mandatory to be specified?
If so, it is better to implement check to verify metallb_peers is specified in the case at roles/kubernetes-apps/metallb/tasks/main.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not immediately clear if peers are mandatory for MetalLB, the documentation does say you need peer-address, peer-asn, and my-asn though, and as you noted the template modification I made makes metallb_peers a requirement.
I've added the check as requested.

Copy link
Contributor

Choose a reason for hiding this comment

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

when using BGP, peers is mandatory otherwise MetalLB doesn't know what router to connect to in order to advertise the routes

For a basic configuration featuring one BGP router and one IP address range, you need 4 pieces of information:
- The router IP address that MetalLB should connect to,
- The router’s AS number,
- The AS number MetalLB should use,
- An IP address range expressed as a CIDR prefix.

So I agree with @oomichi, we better add a validation check for metallb_peers if metalLB is running in BGP mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A check for metallb_peers was added to roles/kubernetes-apps/metallb/tasks/main.yml in f185879

- peer-address: {{ peer.peer_address }}
peer-asn: {{ peer.peer_asn }}
my-asn: {{ peer.my_asn }}
{% endfor %}
{% endif %}
address-pools:
- name: loadbalanced
protocol: {{ metallb_protocol }}
Expand Down