Repository: metallb/metallb
Pull request #765 (2020-10-24 21:25:03)

Do not set resources limits by default
I've experienced issues in the past where MetalLB was throttled
for 15s and then replying to all previous ARP request at the same time.
MetalLB is the entry point of the cluster, if it's not working everything
else in the cluster is useless, so let's just not set limits.

champtar: @daxmc99 @johananl @rata any opinions 
champtar: Updated and rebased

Patch:
diff --git a/controller/main.go b/controller/main.go
index b0d6c4f9..865c6d47 100644
--- a/controller/main.go
+++ b/controller/main.go
@@ -32,7 +32,6 @@ import (
 
 // Service offers methods to mutate a Kubernetes service object.
 type service interface {
-	Update(svc *v1.Service) (*v1.Service, error)
 	UpdateStatus(svc *v1.Service) error
 	Infof(svc *v1.Service, desc, msg string, args ...interface{})
 	Errorf(svc *v1.Service, desc, msg string, args ...interface{})
@@ -76,19 +75,11 @@ func (c *controller) SetBalancer(l log.Logger, name string, svcRo *v1.Service, _
 		return k8s.SyncStateSuccess
 	}
 
-	var err error
-	if !(reflect.DeepEqual(svcRo.Annotations, svc.Annotations) && reflect.DeepEqual(svcRo.Spec, svc.Spec)) {
-		svcRo, err = c.client.Update(svc)
-		if err != nil {
-			l.Log("op", "updateService", "error", err, "msg", "failed to update service")
-			return k8s.SyncStateError
-		}
-	}
 	if !reflect.DeepEqual(svcRo.Status, svc.Status) {
 		var st v1.ServiceStatus
 		st, svc = svc.Status, svcRo.DeepCopy()
 		svc.Status = st
-		if err = c.client.UpdateStatus(svc); err != nil {
+		if err := c.client.UpdateStatus(svc); err != nil {
 			l.Log("op", "updateServiceStatus", "error", err, "msg", "failed to update service status")
 			return k8s.SyncStateError
 		}
diff --git a/internal/k8s/k8s.go b/internal/k8s/k8s.go
index 5241593a..67d9a917 100644
--- a/internal/k8s/k8s.go
+++ b/internal/k8s/k8s.go
@@ -331,13 +331,6 @@ func (c *Client) ForceSync() {
 	}
 }
 
-// Update writes svc back into the Kubernetes cluster. If successful,
-// the updated Service is returned. Note that changes to svc.Status
-// are not propagated, for that you need to call UpdateStatus.
-func (c *Client) Update(svc *v1.Service) (*v1.Service, error) {
-	return c.client.CoreV1().Services(svc.Namespace).Update(context.TODO(), svc, metav1.UpdateOptions{})
-}
-
 // UpdateStatus writes the protected "status" field of svc back into
 // the Kubernetes cluster.
 func (c *Client) UpdateStatus(svc *v1.Service) error {
diff --git a/manifests/metallb.yaml b/manifests/metallb.yaml
index 0186e498..9b81c99c 100644
--- a/manifests/metallb.yaml
+++ b/manifests/metallb.yaml
@@ -110,7 +110,6 @@ rules:
   - get
   - list
   - watch
-  - update
 - apiGroups:
   - ''
   resources:
@@ -325,10 +324,6 @@ spec:
         - containerPort: 7946
           name: memberlist-udp
           protocol: UDP
-        resources:
-          limits:
-            cpu: 100m
-            memory: 100Mi
         securityContext:
           allowPrivilegeEscalation: false
           capabilities:
@@ -379,10 +374,6 @@ spec:
         ports:
         - containerPort: 7472
           name: monitoring
-        resources:
-          limits:
-            cpu: 100m
-            memory: 100Mi
         securityContext:
           allowPrivilegeEscalation: false
           capabilities:
diff --git a/speaker/main.go b/speaker/main.go
index 549e275e..7679cf55 100644
--- a/speaker/main.go
+++ b/speaker/main.go
@@ -49,7 +49,6 @@ var announcing = prometheus.NewGaugeVec(prometheus.GaugeOpts{
 
 // Service offers methods to mutate a Kubernetes service object.
 type service interface {
-	Update(svc *v1.Service) (*v1.Service, error)
 	UpdateStatus(svc *v1.Service) error
 	Infof(svc *v1.Service, desc, msg string, args ...interface{})
 	Errorf(svc *v1.Service, desc, msg string, args ...interface{})
@@ -200,13 +199,13 @@ func (c *controller) SetBalancer(l gokitlog.Logger, name string, svc *v1.Service
 		return c.deleteBalancer(l, name, "serviceDeleted")
 	}
 
-	l.Log("event", "startUpdate", "msg", "start of service update")
-	defer l.Log("event", "endUpdate", "msg", "end of service update")
-
 	if svc.Spec.Type != "LoadBalancer" {
 		return c.deleteBalancer(l, name, "notLoadBalancer")
 	}
 
+	l.Log("event", "startUpdate", "msg", "start of service update")
+	defer l.Log("event", "endUpdate", "msg", "end of service update")
+
 	if c.config == nil {
 		l.Log("event", "noConfig", "msg", "not processing, still waiting for config")
 		return k8s.SyncStateSuccess


Test patch:
diff --git a/controller/controller_test.go b/controller/controller_test.go
index 491055ee..c699f9f4 100644
--- a/controller/controller_test.go
+++ b/controller/controller_test.go
@@ -65,11 +65,6 @@ type testK8S struct {
 	t                   *testing.T
 }
 
-func (s *testK8S) Update(svc *v1.Service) (*v1.Service, error) {
-	s.updateService = svc
-	return svc, nil
-}
-
 func (s *testK8S) UpdateStatus(svc *v1.Service) error {
 	s.updateServiceStatus = &svc.Status
 	return nil
diff --git a/speaker/bgp_controller_test.go b/speaker/bgp_controller_test.go
index 765f5cae..05d20fd5 100644
--- a/speaker/bgp_controller_test.go
+++ b/speaker/bgp_controller_test.go
@@ -174,10 +174,6 @@ type testK8S struct {
 	t             *testing.T
 }
 
-func (s *testK8S) Update(svc *v1.Service) (*v1.Service, error) {
-	panic("never called")
-}
-
 func (s *testK8S) UpdateStatus(svc *v1.Service) error {
 	panic("never called")
 }