Files
kube-vip/pkg/services/processor.go
Maximilian Rink 9ff88eba50 fix(lease): do not let a stale cleanup cancel a recreated lease (#1669)
* fix(lease): do not let a stale cleanup cancel a recreated lease

Every object that starts leader election also starts a goroutine that calls
Manager.Delete once its context is cancelled. Manager.Delete looked the lease up
by name only, so it acted on whatever lease held that name at the time it ran,
not the one the caller was given.

When a service is torn down and rebuilt straight away, the replacement lease is
already registered by the time the old cleanup goroutine runs, so the cleanup
cancels the live replacement and removes it from the manager. The service is
then never handled again: its election loop exits, the lease keeps an empty
holderIdentity, and the VIP is never re-advertised.

This is reachable from an ordinary service update. Flipping
externalTrafficPolicy makes serviceChanged cancel the service context and
rebuild it, which reproduced the stuck lease for minutes.

Pass the lease the caller owns to Manager.Delete and ignore a stale caller,
which keeps cleanup scoped to the instance it belongs to. Callers already hold
that lease. Passing nil keeps the previous behaviour of deleting whichever lease
currently holds the name, which is what the existing tests assert.

Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>

* fix(lease): retire a lease when its service is torn down

The instance guard in Delete stops a late cleanup from cancelling a replacement
lease, but it cannot help when the replacement *is* the same instance.

A service teardown cancels the service context and leaves the lease registered,
because the cleanup that removes it is deferred to a goroutine. The rebuild that
follows calls Add, which finds that lease still in the map and hands it straight
back, so the new service context is parented to a lease the pending cleanup is
about to cancel. The service then cycles: acquire, lose, re-acquire, every few
seconds, and never settles.

Add Manager.Retire, and call it from the serviceChanged teardown next to the
svcMap purge, so the lease is out of the map before the replacement context is
built. Add also refuses to hand out a lease whose context is already cancelled,
which closes the same hazard for any other path that cancels a lease directly.

Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>

* fix(lease): keep a common lease alive for the services still using it

Review feedback from Patryk on #1669: Retire cancelled the lease context
outright, so with a common lease a modification of one service would also tear
down every sibling sharing that lease.

Retire was only ever needed for its side effect of getting the lease out of the
map before the rebuild, and Delete already does exactly that once the last
object is gone. Drop Retire and have the teardown path call Delete with its own
object name, so siblings keep the lease alive and the manager API stays
Add/Delete/Get.

TestManager_LeaseLifetimeInvariant replaces the single-scenario test with the
rule for the whole surface: a lease stays usable for exactly as long as at least
one object holds it, and a rebuild afterwards gets a fresh one. It is table
driven over 1, 2 and 4 objects, and the 2 and 4 cases fail against the reviewed
behaviour with "lease was cancelled with N object(s) still holding it".

Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>

---------

Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
2026-08-06 21:46:31 +02:00

426 lines
15 KiB
Go

package services
import (
"context"
"errors"
"fmt"
log "log/slog"
"reflect"
"sync"
"time"
"github.com/kube-vip/kube-vip/pkg/arp"
"github.com/kube-vip/kube-vip/pkg/bgp"
"github.com/kube-vip/kube-vip/pkg/election"
"github.com/kube-vip/kube-vip/pkg/endpoints"
"github.com/kube-vip/kube-vip/pkg/endpoints/providers"
"github.com/kube-vip/kube-vip/pkg/instance"
"github.com/kube-vip/kube-vip/pkg/kubevip"
"github.com/kube-vip/kube-vip/pkg/lease"
"github.com/kube-vip/kube-vip/pkg/metrics"
"github.com/kube-vip/kube-vip/pkg/networkinterface"
"github.com/kube-vip/kube-vip/pkg/node"
"github.com/kube-vip/kube-vip/pkg/route"
"github.com/kube-vip/kube-vip/pkg/servicecontext"
"github.com/kube-vip/kube-vip/pkg/utils"
"github.com/kube-vip/kube-vip/pkg/vip"
"github.com/kube-vip/kube-vip/pkg/wireguard"
"github.com/prometheus/client_golang/prometheus"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/client-go/kubernetes"
)
type Processor struct {
config *kubevip.Config
lbClassFilter func(svc *v1.Service, config *kubevip.Config) bool
svcMap sync.Map
// Keeps track of all running instances
ServiceInstances []*instance.Instance
mutex sync.Mutex
bgpServer *bgp.Server
clientSet *kubernetes.Clientset
rwClientSet *kubernetes.Clientset
intfMgr *networkinterface.Manager
arpMgr *arp.Manager
leaseMgr *lease.Manager
// nodeLabelManager is the manager for the node labels
nodeLabelManager node.Labeler
electionMgr *election.Manager
// TunnelMgr manages multiple WireGuard tunnels (one per service VIP)
TunnelMgr *wireguard.TunnelManager
routeMgr *route.Manager
}
// labelManager is the interface for the node label manager to add/remove labels
func NewServicesProcessor(config *kubevip.Config, bgpServer *bgp.Server,
clientSet *kubernetes.Clientset, rwClientSet *kubernetes.Clientset,
intfMgr *networkinterface.Manager, arpMgr *arp.Manager, nodeLabelManager node.Labeler,
electionMgr *election.Manager, leaseMgr *lease.Manager, routeMgr *route.Manager) *Processor {
lbClassFilterFunc := lbClassFilter
if config.LoadBalancerClassLegacyHandling {
lbClassFilterFunc = lbClassFilterLegacy
}
return &Processor{
config: config,
lbClassFilter: lbClassFilterFunc,
ServiceInstances: []*instance.Instance{},
bgpServer: bgpServer,
clientSet: clientSet,
rwClientSet: rwClientSet,
intfMgr: intfMgr,
arpMgr: arpMgr,
leaseMgr: leaseMgr,
nodeLabelManager: nodeLabelManager,
electionMgr: electionMgr,
TunnelMgr: wireguard.NewTunnelManager(),
routeMgr: routeMgr,
}
}
func (p *Processor) AddOrModify(ctx context.Context, event watch.Event, serviceFunc *Callback, forcedOnly bool, wg *sync.WaitGroup) error {
svc, ok := event.Object.(*v1.Service)
if !ok {
return fmt.Errorf("unable to parse Kubernetes services from API watcher")
}
timer := prometheus.NewTimer(metrics.ServiceReconcileDuration.WithLabelValues(svc.Namespace))
defer timer.ObserveDuration()
if forcedOnly && svc.Annotations[kubevip.ForcePerServiceElection] != "true" ||
!forcedOnly && svc.Annotations[kubevip.ForcePerServiceElection] == "true" {
return nil
}
// We only care about LoadBalancer services
if svc.Spec.Type != v1.ServiceTypeLoadBalancer {
return nil
}
// Check if we ignore this service
if svc.Annotations[kubevip.LoadbalancerIgnore] == "true" {
log.Info("ignore annotation for kube-vip", "service name", svc.Name)
return nil
}
// Check the loadBalancer class
if p.lbClassFilter(svc, p.config) {
return nil
}
// The Service annotation is cluster-wide while nftables state is local to
// each node. Reconcile stale per-Service chains on every node after a table
// migration, even when this kube-vip pod is not the Service leader.
if svc.Annotations[kubevip.EgressNftablesTable] != "" {
if err := p.cleanupStaleEgressNftablesChains(svc); err != nil {
log.Warn("failed to clean stale nftables egress chains", "service", svc.Name, "namespace", svc.Namespace, "err", err)
}
}
svcAddresses, svcHostnames := instance.FetchServiceAddresses(svc)
// We only care about LoadBalancer services that have been allocated an address
if len(svcAddresses) <= 0 && len(svcHostnames) <= 0 {
s, err := p.waitForAddress(ctx, svc)
if err != nil {
return fmt.Errorf("failed to get updated LB addresses for service %s/%s: %w", svc.Namespace, svc.Name, err)
}
svc = s
}
svcInstance := instance.FindServiceInstance(svc, p.ServiceInstances)
var err error
if svcInstance == nil {
svcInstance, err = instance.NewInstance(ctx, svc, p.config, p.intfMgr, p.arpMgr, p.routeMgr, p.nodeLabelManager, wg)
if err != nil {
metrics.ServiceReconcileErrorsTotal.WithLabelValues(svc.Namespace, svc.Name, "new_instance").Inc()
return fmt.Errorf("unable to create instance for service %s/%s", svc.Namespace, svc.Name)
}
p.ServiceInstances = append(p.ServiceInstances, svcInstance)
p.updateActiveServicesMetric()
}
_, usesCommonLease := svc.Annotations[kubevip.ServiceLease]
if usesCommonLease && svc.Spec.ExternalTrafficPolicy != v1.ServiceExternalTrafficPolicyTypeCluster {
metrics.ServiceReconcileErrorsTotal.WithLabelValues(svc.Namespace, svc.Name, "invalid_config").Inc()
return fmt.Errorf("annotation %q cannot be used with service traffic policy other than %q, service %s/%s",
kubevip.ServiceLease, v1.ServiceExternalTrafficPolicyTypeCluster, svc.Namespace, svc.Name)
}
svcCtx, err := p.getServiceContext(svc.UID)
if err != nil {
metrics.ServiceReconcileErrorsTotal.WithLabelValues(svc.Namespace, svc.Name, "service_context").Inc()
return fmt.Errorf("failed to get service context: %w", err)
}
svcCtx = p.dropCancelledServiceContext(svc.UID, svcCtx)
// The modified event should only be triggered if the service has been modified (i.e. moved somewhere else)
if event.Type == watch.Modified {
shouldGarbageCollect := false
if svcInstance != nil {
shouldGarbageCollect = serviceChanged(svcInstance, svc)
}
if shouldGarbageCollect {
for _, addr := range svcAddresses {
// log.Debugf("(svcs) Retrieving local addresses, to ensure that this modified address doesn't exist: %s", addr)
f, err := vip.GarbageCollect(p.config.Interface, addr, p.intfMgr)
if err != nil {
log.Error("(svcs) cleaning existing address error", "err", err)
}
if f {
log.Warn("(svcs) already found existing config", "address", addr, "adapter", p.config.Interface)
}
}
// This service has been modified, but it was also active.
if svcCtx != nil {
log.Warn("(svcs) The load balancer has changed, cancelling original load balancer")
//Set it to inactive
svcCtx.Cancel()
if err := p.deleteService(ctx, svc.UID); err != nil {
metrics.ServiceReconcileErrorsTotal.WithLabelValues(svc.Namespace, svc.Name, "delete_service").Inc()
log.Error("(svc) unable to remove", "service", svc.UID)
}
// in theory this should never fail
p.svcMap.Delete(svc.UID)
// Drop this service from its lease now, so the replacement context
// below is not parented to a lease the pending cleanup is about to
// cancel. A lease shared with other services stays alive for them.
ns, name := lease.ServiceName(svc)
leaseID := lease.NewID(p.config.LeaderElectionType, ns, name)
p.leaseMgr.Delete(leaseID, lease.ServiceNamespacedName(svc), nil)
// Reset the the svcCtx when it was garbage collected
// As the next function will create a new context when nil
svcCtx = nil
p.updateActiveServicesMetric()
}
}
}
ips, hostnames := instance.FetchServiceAddresses(svc)
log.Debug("(svcs) has been added/modified with addresses", "service name", svc.Name, "ips", ips, "hostnames", hostnames)
if svcCtx == nil {
ns, name := lease.ServiceName(svc)
leaseID := lease.NewID(p.config.LeaderElectionType, ns, name)
lease := p.leaseMgr.Add(ctx, leaseID)
svcCtx = servicecontext.New(lease.Ctx)
p.svcMap.Store(svc.UID, svcCtx)
}
// this goroutine starts service handling function (with or without leaderelection)
if !svcCtx.IsWatched {
wg.Go(func() {
watchWg := sync.WaitGroup{}
defer func() {
// wait for the sub-goroutines and tag service as not watched
watchWg.Wait()
svcCtx.IsWatched = false
}()
watchWg.Go(func() {
// start if service is not already watched/handled
// signal endpoints goroutine we are ready to start and run service handling function
log.Info("(svcs) service function starting", "uid", svc.UID)
err = serviceFunc.Run(svcCtx, svc, wg)
if err != nil {
log.Error(err.Error())
if errors.Is(err, &utils.PanicError{}) {
// cancel service context on panic error
// TODO: should we quit kube-vip altogether here?
svcCtx.Cancel()
}
}
log.Info("(svcs) service function done", "uid", svc.UID)
})
// this goroutine will watch endpoints for the service
watchWg.Go(func() {
// create provider and start watching the endpoints
var provider providers.Provider
if p.config.EnableEndpoints {
provider = providers.NewEndpoints()
} else {
provider = providers.NewEndpointslices()
}
if err = p.watchEndpoint(svcCtx, p.config.NodeName, svc, provider); err != nil {
log.Error(err.Error())
}
})
})
// tag service as watched
svcCtx.IsWatched = true
}
if !p.config.EnableServicesElection {
log.Debug("Service now active", "name", svc.Name, "uid", svc.UID)
}
return nil
}
func (p *Processor) waitForAddress(ctx context.Context, svc *v1.Service) (*v1.Service, error) {
addressCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()
ticker := time.NewTicker(time.Second)
for {
select {
case <-addressCtx.Done():
return nil, fmt.Errorf("failed to wait for the service LB address: %w", ctx.Err())
case <-ticker.C:
s, err := p.clientSet.CoreV1().Services(svc.Namespace).Get(addressCtx, svc.Name, metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("failed to get updated service data: %w", err)
}
addrs, hostnames := instance.FetchServiceAddresses(s)
if len(addrs) > 0 || len(hostnames) > 0 {
return s, nil
}
}
}
}
func (p *Processor) Delete(event watch.Event, forcedOnly bool) error {
svc, ok := event.Object.(*v1.Service)
if !ok {
return fmt.Errorf("(svcs) unable to parse Kubernetes services from API watcher")
}
if forcedOnly && svc.Annotations[kubevip.ForcePerServiceElection] != "true" ||
!forcedOnly && svc.Annotations[kubevip.ForcePerServiceElection] == "true" {
return nil
}
svcCtx, err := p.getServiceContext(svc.UID)
if err != nil {
return fmt.Errorf("(svcs) unable to get context: %w", err)
}
if svcCtx != nil {
// We only care about LoadBalancer services
if svc.Spec.Type != v1.ServiceTypeLoadBalancer {
return nil
}
// We can ignore this service
if svc.Annotations[kubevip.LoadbalancerIgnore] == "true" {
log.Info("(svcs) ignore annotation for kube-vip", "service name", svc.Name)
return nil
}
// If no leader election is enabled, delete routes here
if !p.config.EnableLeaderElection && !p.config.EnableServicesElection &&
p.config.EnableRoutingTable && svcCtx.HasConfiguredNetworks() {
if errs := endpoints.ClearRoutes(svc, &p.ServiceInstances, p.routeMgr); len(errs) == 0 {
svcCtx.ConfiguredNetworks.Clear()
}
}
if !p.config.EnableServicesElection {
// If this is an active service then and additional leaderElection will handle stopping
err = p.deleteService(svcCtx.Ctx, svc.UID)
if err != nil {
log.Error(err.Error())
}
}
// Calls the cancel function of the context
log.Warn("(svcs) The load balancer was deleted, cancelling context", "namespace", svc.Namespace, "name", svc.Name, "uid", svc.UID)
svcCtx.Cancel()
p.svcMap.Delete(svc.UID)
p.updateActiveServicesMetric()
}
log.Info("(svcs) deleted", "service name", svc.Name, "namespace", svc.Namespace)
return nil
}
func (p *Processor) Stop() {
for _, instance := range p.ServiceInstances {
for _, cluster := range instance.Clusters {
cluster.Stop()
}
}
}
func (p *Processor) getServiceContext(uid types.UID) (*servicecontext.Context, error) {
svcCtx, ok := p.svcMap.Load(uid)
if !ok {
return nil, nil
}
ctx, ok := svcCtx.(*servicecontext.Context)
if !ok {
return nil, fmt.Errorf("failed to cast service context pointer - UID: %s", uid)
}
return ctx, nil
}
// dropCancelledServiceContext discards a service context whose context has already been
// cancelled, removing it from svcMap and returning nil so that callers create a fresh one.
//
// This matters because the in-memory lease and the service context are removed independently.
// The cleanup goroutine started by StartServicesLeaderElection calls leaseMgr.Delete once
// svcCtx.Ctx is done, and Manager.Delete drops the lease entirely when its last object goes
// away. Several paths cancel the service context without also removing it from svcMap - for
// example the deferred close(stopChan) in watchEndpoint, and the utils.PanicError branch in
// AddOrModify.
//
// If such a cancelled context were reused, AddOrModify would skip its `if svcCtx == nil`
// branch and therefore never call leaseMgr.Add again, so StartServicesLeaderElection would
// fail with "no existing lease found" on every subsequent event and the VIP would never be
// advertised again.
func (p *Processor) dropCancelledServiceContext(uid types.UID, svcCtx *servicecontext.Context) *servicecontext.Context {
if svcCtx == nil || svcCtx.Ctx.Err() == nil {
return svcCtx
}
p.svcMap.Delete(uid)
return nil
}
func serviceChanged(i *instance.Instance, svc *v1.Service) bool {
svcAddresses, svcHostnames := instance.FetchServiceAddresses(svc)
originalServiceAddresses, originalServiceHostnames := instance.FetchServiceAddresses(i.ServiceSnapshot)
// Service addresses changed
return !reflect.DeepEqual(originalServiceAddresses, svcAddresses) ||
// Service hostnames changed
!reflect.DeepEqual(originalServiceHostnames, svcHostnames) ||
// ExternalTrafficPolicy changed
svc.Spec.ExternalTrafficPolicy != i.ServiceSnapshot.Spec.ExternalTrafficPolicy ||
// IP stack configuration changed
!reflect.DeepEqual(svc.Spec.IPFamilies, i.ServiceSnapshot.Spec.IPFamilies) ||
*svc.Spec.IPFamilyPolicy != *i.ServiceSnapshot.Spec.IPFamilyPolicy ||
// DDNS was disabled/enabled
svc.Annotations[kubevip.ServiceDDNS] != i.ServiceSnapshot.Annotations[kubevip.ServiceDDNS]
}
func (p *Processor) updateActiveServicesMetric() {
counts := map[string]int{}
for _, inst := range p.ServiceInstances {
if inst.ServiceSnapshot != nil {
counts[inst.ServiceSnapshot.Namespace]++
}
}
metrics.ActiveServices.Reset()
for ns, count := range counts {
metrics.ActiveServices.WithLabelValues(ns).Set(float64(count))
}
}