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.

Sign up for GitHub

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

Jump to bottom

metallb does not deallocate a Service's IP on Service type change to non-LB #190

Closed
rsanders opened this issue Mar 10, 2018 · 4 comments
Closed

metallb does not deallocate a Service's IP on Service type change to non-LB #190

rsanders opened this issue Mar 10, 2018 · 4 comments
Assignees
@danderson
Labels
bug

Comments

@rsanders
Copy link

Is this a bug report or a feature request?:

Bug report.

What happened:

Changing the type of a Service with a metallb-assigned IP from LoadBalancer to NodePort does not appear to release the IP allocation to revoke access from outside the cluster. Instead, the Service remains reachable on the metallb-allocated IP address on the Service's port.

What you expected to happen:

I expected the IP address to be returned to the pool; reachability of the Service to be lost; and the status field of the Service to be updated to remove the ingress IP address.

How to reproduce it (as minimally and precisely as possible):

Setup:

metal v0.4.3 running a K8S 1.8.6 cluster. Create a NodePort service. Edit the type to be LoadBalancer. metallb should assign an IP. Test connectivity and check the status field for the assigned IP.

After that has been successful, edit the Service and change type back to NodePort. The allocation will not appear to change, and in my case I was still able to connect to the service on its defined port using the IP address allocated by metallb.

Anything else we need to know?:

No.

Environment:

  • MetalLB version: 0.4.3
  • Kubernetes version: 1.8.6
  • BGP router type/version: using ARP mode
  • OS (e.g. from /etc/os-release): CentOS 7.4 (except for kernel)
  • Kernel (e.g. uname -a): 4.14.12-1.el7.elrepo.x86_64
@danderson danderson added the bug label Mar 10, 2018
@danderson danderson self-assigned this Mar 10, 2018
@danderson
Copy link
Contributor

Thanks for the report!

I see where the bug is in the code, I'll cut 0.4.4 with a fix shortly.

I'm surprised this can happen, because when I was developing MetalLB on k8s 1.8, I think I was unable to change the type of a Service after creation... So the codepath just assumes that if something is not a LoadBalancer, then it never was a LoadBalancer and we don't need to run the deletion logic. Oops!

@danderson
Copy link
Contributor

And out of pure brilliance, I cut 0.4.4 before actually testing my change, and wouldn't you know it, it's broken!

Fixing (and testing before cutting the release this time...), 0.4.5 will have the right stuff.

danderson added a commit that referenced this issue Mar 10, 2018
@danderson
Call deleteBalancer for services of the wrong type. #190
886a19c 
Turns out, you _can_ convert a service from type LoadBalancer to
something else, whereas I thought that the type was immutable after
creation. Oops!
danderson added a commit that referenced this issue Mar 10, 2018
@danderson
Call deleteBalancer for services of the wrong type. #190
955d967 
Turns out, you _can_ convert a service from type LoadBalancer to
something else, whereas I thought that the type was immutable after
creation. Oops!

(cherry picked from commit 886a19c)
danderson added a commit that referenced this issue Mar 10, 2018
@danderson
Actually fix #190.
655a1de 
We can't just delete internal state when an actively managed object
becomes unmanaged, we have to run through the convergence logic to
push the cleared state back up into k8s.

(cherry picked from commit 0574519)
@danderson danderson reopened this Mar 10, 2018
@danderson
Copy link
Contributor

0.4.5 released, with a fix for this. Thank you again for the clear bug report!

@rsanders
Copy link
Author

Thank you for the quick fix!

fedepaol pushed a commit to fedepaol/metallb that referenced this issue Aug 20, 2024
@openshift-merge-bot
Merge pull request metallb#190 from fedepaol/openshift/kuberbac
5c9d92b 
Openshift CI: override the kuberbac proxy image too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@danderson danderson

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@danderson @rsanders