-
Notifications
You must be signed in to change notification settings - Fork 49
components/metallb: Update to v0.1.0-789-g85b7a46a #885
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Do we have an e2e test to cover ingress requests on Packet which would test that automatically?
I don't have an experience with setting up BGP on Packet, so testing may take me a bit longer.
Doesn't look like we have that. I can look into adding that. In any case I wanted people to play around with things manually. To deploy MetalLB on Packet all you have to do is ensure you don't disable BGP for the worker pool and include a component section with your EIP: component "metallb" {
address_pools = {
default = ["147.1.2.3/32"]
}
} (Use the code from this PR for |
38f0fe0
to
1d46d2e
Compare
This is a "snapshot" of metallb/metallb#593 at commit 85b7a46a. This updates MetalLB to a more recent release which contains a more stable implementation of the peer autodiscover feature. Fixes #785.
1d46d2e
to
ee55a2e
Compare
I've tested this manually and it worked for me 👍 |
Thanks @invidian. I assume you were able to test and things went fine? |
I'm taking a quick look at adding an integration test to verify ingress on Packet. EDIT: Doing so would require figuring out how to deal with Packet EIPs in an automated way (especially considering the fact we often have multiple CI builds running in parallel). I've opened #888 for tracking this and we'll leave it out of this PR. |
Yes, sorry for the delay, it works nicely 👍 |
This PR updates the
metallb
component to a more recent "snapshot" of the peer autodiscovery feature. A main purpose of the update is to stabilize the BGP sessions we use on Packet.As part of this update I've copy-pasted the manifests from the upstream ones as much as possible to make future updates easier. In some cases this resulted in fields being reordered.
On preliminary testing things look good to me, however I've opened the PR to get reviews ASAP since this update is kind of urgent. We should examine the changes carefully, and ideally at least one more person should manually verify things look sane before we merge this.