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

Support MetalLB #466

Merged
merged 4 commits into from
Jun 25, 2024
Merged

Support MetalLB #466

merged 4 commits into from
Jun 25, 2024

Conversation

eaudetcobello
Copy link
Contributor

@eaudetcobello eaudetcobello commented Jun 3, 2024

Testing

  • l2-mode is covered by our integration tests and has been tested manually by deploying an nginx service.
  • bgp mode is not covered by our integration tests and has been tested manually by deploying a bgp router using bird on a multipass vm and verifying that an nginx service on a node can obtain an ip using the bgp protocol.

Example generated templates from values.yaml

Metallb values.yaml:

driver: metallb

l2:
  enabled: true
  # interfaces:
  # - "^eth[0-9]+"
  interfaces:
    - eth1

ipPool:
  # cidrs:
  # - cidr: "10.42.254.176/28"
  cidrs:
    - cidr: "192.168.10.0/24"
    - start: "192.168.11.1"
      end: "192.168.11.254"

bgp:
  enabled: true
  localASN: 64512
  # neighbors:
  # - peerAddress: '10.0.0.60/24'
  #   peerASN: 65100
  #   peerPort: 179
  neighbors:
    - peerAddress: "10.0.0.60"
      peerASN: 65100
      peerPort: 179

Metallb template output:

---
# Source: ck-loadbalancer/templates/metallb/bgp-policy.yaml
apiVersion: "metallb.io/v1beta1"
kind: BGPAdvertisement
metadata:
  name: release-name-ck-loadbalancer
  labels:
    helm.sh/chart: ck-loadbalancer-0.1.1
    app.kubernetes.io/name: ck-loadbalancer
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "0.1.1"
    app.kubernetes.io/managed-by: Helm
spec:
   ipAddressPools:
     - release-name-ck-loadbalancer
---
# Source: ck-loadbalancer/templates/metallb/bgp-policy.yaml
apiVersion: "metallb.io/v1beta2"
kind: BGPPeer
metadata:
  name: release-name-ck-loadbalancer
  labels:
    helm.sh/chart: ck-loadbalancer-0.1.1
    app.kubernetes.io/name: ck-loadbalancer
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "0.1.1"
    app.kubernetes.io/managed-by: Helm
spec:
  myASN: 64512
  neighbors:
    - peerASN: 65100
      peerAddress: 10.0.0.60
      peerPort: 179
---
# Source: ck-loadbalancer/templates/metallb/lb-ip-pool.yaml
apiVersion: "metallb.io/v1beta1"
kind: IPAddressPool
metadata:
  name: release-name-ck-loadbalancer
  labels:
    helm.sh/chart: ck-loadbalancer-0.1.1
    app.kubernetes.io/name: ck-loadbalancer
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "0.1.1"
    app.kubernetes.io/managed-by: Helm
spec:
  addresses:
    - 192.168.10.0/24
---
# Source: ck-loadbalancer/templates/metallb/l2-policy.yaml
apiVersion: "metallb.io/v1beta1"
kind: L2Advertisement
metadata:
  name: release-name-ck-loadbalancer
  labels:
    helm.sh/chart: ck-loadbalancer-0.1.1
    app.kubernetes.io/name: ck-loadbalancer
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "0.1.1"
    app.kubernetes.io/managed-by: Helm
spec:
  ipAddressPools:
    - release-name-ck-loadbalancer
  interfaces:
    - eth1

Potential improvements

When a user gets an IP for a service using bgp-mode=true, a BGPPeer and a BGPAdvertisement resource are created. If the user then sets bgp-mode=false, the BGPPeer and BGPAdvertisement resources will continue existing. When the user sets l2-mode=true, the service will successfully receive a IP using l2 mode.

From my testing this does not cause problems in terms of bugs or functionality but it could be nice to clean up when the config is changed.

@eaudetcobello eaudetcobello changed the title [moonray] Support MetalLB Support MetalLB Jun 3, 2024
@eaudetcobello eaudetcobello force-pushed the KU-895 branch 2 times, most recently from b83b0e0 to 839526a Compare June 4, 2024 19:00
@eaudetcobello eaudetcobello force-pushed the KU-895 branch 2 times, most recently from 501e142 to ce8fac0 Compare June 6, 2024 20:59
@eaudetcobello eaudetcobello marked this pull request as ready for review June 6, 2024 21:23
@eaudetcobello eaudetcobello requested a review from a team as a code owner June 6, 2024 21:23
Copy link
Contributor

@addyess addyess left a comment

Choose a reason for hiding this comment

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

Provided some thoughts to chew on. I think there's a bug in the cilium/l2-policy.yaml

@eaudetcobello
Copy link
Contributor Author

eaudetcobello commented Jun 11, 2024

Let me know if you have any ideas on what to do with gateway-api. Leave the script in hacks/ and run it from update-component-versions.py? @neoaggelos

k8s/manifests/charts/ck-loadbalancer/values.schema.json Outdated Show resolved Hide resolved
k8s/manifests/charts/ck-loadbalancer/values.yaml Outdated Show resolved Hide resolved
build-scripts/hack/util.py Outdated Show resolved Hide resolved
src/k8s/pkg/k8sd/features/metallb/loadbalancer.go Outdated Show resolved Hide resolved
Copy link
Contributor

@neoaggelos neoaggelos left a comment

Choose a reason for hiding this comment

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

In addition, please add a comment with all the metallb images that are used.

k8s kubectl get node -o template='{{ range .items }}{{ .metadata.name }}{{":"}}{{ range .status.images }}{{ "\n- " }}{{ index .names 1 }}{{ end }}{{"\n"}}{{ end }}'

can help with this

@eaudetcobello
Copy link
Contributor Author

MetalLB images:

  • quay.io/metallb/speaker:v0.14.5
  • quay.io/metallb/controller:v0.14.5

@eaudetcobello
Copy link
Contributor Author

I added a description with an example values.yaml and the generated template.

build-scripts/hack/util.py Outdated Show resolved Hide resolved
build-scripts/hack/update-component-versions.py Outdated Show resolved Hide resolved
build-scripts/hack/util.py Outdated Show resolved Hide resolved
k8s/manifests/charts/ck-loadbalancer/values.schema.json Outdated Show resolved Hide resolved
Copy link
Contributor

@louiseschmidtgen louiseschmidtgen left a comment

Choose a reason for hiding this comment

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

Please rebase, there's changes with Contour.

Copy link
Contributor

@louiseschmidtgen louiseschmidtgen left a comment

Choose a reason for hiding this comment

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

Great work Etienne!

One last thing, please add all the images in use by metallb in the PR.

k8s kubectl get node -o template='{{ range .items }}{{ .metadata.name }}{{":"}}{{ range .status.images }}{{ "\n- " }}{{ index .names 1 }}{{ end }}{{"\n"}}{{ end }}'

Just add a comment or add it to your PR description please.
Additionally, please add a var with the image repo and the tag similar to here.

Copy link
Contributor

@louiseschmidtgen louiseschmidtgen left a comment

Choose a reason for hiding this comment

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

Another last comment :)
Please write a card to add some bgp integration tests in the future.

build-scripts/hack/update-component-versions.py Outdated Show resolved Hide resolved
build-scripts/hack/update-component-versions.py Outdated Show resolved Hide resolved
build-scripts/hack/util.py Outdated Show resolved Hide resolved
build-scripts/hack/util.py Outdated Show resolved Hide resolved
@eaudetcobello
Copy link
Contributor Author

eaudetcobello commented Jun 24, 2024

Great work Etienne!

One last thing, please add all the images in use by metallb in the PR.

k8s kubectl get node -o template='{{ range .items }}{{ .metadata.name }}{{":"}}{{ range .status.images }}{{ "\n- " }}{{ index .names 1 }}{{ end }}{{"\n"}}{{ end }}'

Just add a comment or add it to your PR description please. Additionally, please add a var with the image repo and the tag similar to here.

Already done but I can paste it again.

MetalLB images:

quay.io/metallb/speaker:v0.14.5
quay.io/metallb/controller:v0.14.5

@neoaggelos neoaggelos merged commit 1188bdc into main Jun 25, 2024
17 checks passed
@neoaggelos neoaggelos deleted the KU-895 branch June 25, 2024 12:16
louiseschmidtgen pushed a commit that referenced this pull request Jul 4, 2024
* Add metallb chart

* make ck-loadbalancer chart work with cilium or metallb

* implement metallb feature

* moonray use metallb

---------

Co-authored-by: Angelos Kolaitis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants