Skip to content

Commit

Permalink
Fix deletion of svclb DaemonSet when Service is deleted
Browse files Browse the repository at this point in the history
87e1806 removed the OwnerReferences
field from the DaemonSet, which makes sense since the Service may now be
in a different namespace.  Unfortunately, we were relying on
OwnerReferences garbage collection to delete the DameonSet, since we
don't have an OnRemove handler registered for Services.

Fix this by adding an OnRemove handler, and restore the DaemonSet
removal Event that was removed by the same commit.

Signed-off-by: Brad Davidson <[email protected]>
  • Loading branch information
brandond committed Jul 8, 2022
1 parent 86fc940 commit f995fc1
Showing 1 changed file with 19 additions and 2 deletions.
21 changes: 19 additions & 2 deletions pkg/servicelb/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func Register(ctx context.Context,
}

services.OnChange(ctx, ControllerName, h.onChangeService)
services.OnRemove(ctx, ControllerName, h.onChangeService)
nodes.OnChange(ctx, ControllerName, h.onChangeNode)
relatedresource.Watch(ctx, ControllerName+"-watcher",
h.onResourceChange,
Expand Down Expand Up @@ -359,7 +360,16 @@ func (h *handler) deployPod(svc *core.Service) error {
return err
}
objs := objectset.NewObjectSet()
if !h.enabled || svc.Spec.Type != core.ServiceTypeLoadBalancer || svc.Spec.ClusterIP == "" || svc.Spec.ClusterIP == "None" {
if !h.enabled || svc.DeletionTimestamp != nil || svc.Spec.Type != core.ServiceTypeLoadBalancer || svc.Spec.ClusterIP == "" || svc.Spec.ClusterIP == "None" {
name := generateName(svc)
ds, err := h.daemonsets.DaemonSets(h.klipperLBNamespace).Get(context.TODO(), name, meta.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return nil
}
return err
}
h.recorder.Eventf(svc, core.EventTypeNormal, "DeletedDaemonSet", "Deleted LoadBalancer DaemonSet %s/%s", ds.Namespace, ds.Name)
return h.processor.WithOwner(svc).Apply(objs)
}

Expand All @@ -377,7 +387,7 @@ func (h *handler) deployPod(svc *core.Service) error {
// newDaemonSet creates a DaemonSet to ensure that ServiceLB pods are run on
// each eligible node.
func (h *handler) newDaemonSet(svc *core.Service) (*apps.DaemonSet, error) {
name := fmt.Sprintf("svclb-%s-%s", svc.Name, svc.UID[:8])
name := generateName(svc)
oneInt := intstr.FromInt(1)

// If ipv6 is present, we must enable ipv6 forwarding in the manifest
Expand All @@ -394,6 +404,8 @@ func (h *handler) newDaemonSet(svc *core.Service) (*apps.DaemonSet, error) {
Namespace: h.klipperLBNamespace,
Labels: map[string]string{
nodeSelectorLabel: "false",
svcNameLabel: svc.Name,
svcNamespaceLabel: svc.Namespace,
},
},
TypeMeta: meta.TypeMeta{
Expand Down Expand Up @@ -562,3 +574,8 @@ func (h *handler) deleteOldDeployments(svc *core.Service) error {
}
return h.deployments.Deployments(svc.Namespace).Delete(context.TODO(), name, meta.DeleteOptions{})
}

// generateName generates a distinct name for the DaemonSet based on the service name and UID
func generateName(svc *core.Service) string {
return fmt.Sprintf("svclb-%s-%s", svc.Name, svc.UID[:8])
}

0 comments on commit f995fc1

Please sign in to comment.