cmd/k8s-operator: recover missing egress EndpointSlices (#21174)

* cmd/k8s-operator: move egress EndpointSlice write back into gated provision

PR #20347 moved the EndpointSlice createOrUpdate outside of provision,
causing it to run on every reconcile. This resulted in racing egress-eps on
the EndpointSlice, sometimes causing the TailscaleEgressSvcConfigured to
become stuck as False with the Service not fully updated. Gate it again so
it only runs when a reprovision is required.

Updates #20916

Signed-off-by: Becky Pauley <becky@tailscale.com>

* cmd/k8s-operator: recover missing egress EndpointSlices

Add a watch for EndpointSlices in the egress-services reconciler so a
deleted slice re-triggers a Service reconcile directly. Treat a Service
whose expected per-family EndpointSlice is missing as not up to date so it
re-enters provision and recreates the slice.

Also sort endpoints by Pod UID before writing them in the egress-eps
reconciler, so an unchanged set of ready Pods cannot result in a different
order and trigger an unnecessary Update.

Updates #20916

Signed-off-by: Becky Pauley <becky@tailscale.com>

---------

Signed-off-by: Becky Pauley <becky@tailscale.com>
This commit is contained in:
BeckyPauley authored and GitHub committed 2026-09-21 11:21:57 +01:00
1 parent 3014ad828e
commit 7bf76690f0
6 files changed
+333 -45

No files matched your search

+7
View File
@@ -11,6 +11,7 @@ import (
"fmt"
"net/netip"
"reflect"
"slices"
"strings"
"go.uber.org/zap"
@@ -18,6 +19,7 @@ import (
discoveryv1 "k8s.io/api/discovery/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -130,6 +132,11 @@ func (er *egressEpsReconciler) Reconcile(ctx context.Context, req reconcile.Requ
},
})
}
// Endpoints must be in a deterministic order to avoid triggering unnecessary extra reconciles
// (see tailscale/tailscale#20916). Sort by Pod UID (Hostname), which is stable per Pod.
slices.SortFunc(newEndpoints, func(a, b discoveryv1.Endpoint) int {
return strings.Compare(ptr.Deref(a.Hostname, ""), ptr.Deref(b.Hostname, ""))
})
// Note that Endpoints are being overwritten with the currently valid endpoints so we don't need to explicitly
// run a cleanup for deleted Pods etc.
eps.Endpoints = newEndpoints
+105
View File
@@ -237,6 +237,111 @@ func TestTailscaleEgressEndpointSlices(t *testing.T) {
})
}
// TestEgressEndpointSliceEndpointsSorted verifies that endpoints are written in a deterministic (Hostname/UID
// sorted) order and that a second reconcile over an unchanged ready set does not rewrite the slice (a needless
// write would re-trigger this reconciler via its own EndpointSlice watch - see tailscale/tailscale#20916).
func TestEgressEndpointSliceEndpointsSorted(t *testing.T) {
clock := tstest.NewClock(tstest.ClockOpts{})
svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
UID: types.UID("1234-UID"),
Annotations: map[string]string{
AnnotationTailnetTargetFQDN: "foo.bar.ts.net",
AnnotationProxyGroup: "foo",
},
},
Spec: corev1.ServiceSpec{
ExternalName: "placeholder",
Type: corev1.ServiceTypeExternalName,
Ports: []corev1.ServicePort{{Name: "http", Protocol: "TCP", Port: 80}},
},
Status: corev1.ServiceStatus{
Conditions: []metav1.Condition{
condition(tsapi.EgressSvcConfigured, metav1.ConditionTrue, "", "", clock),
condition(tsapi.EgressSvcValid, metav1.ConditionTrue, "", "", clock),
},
},
}
port := randomPort()
cm := configMapForSvc(t, svc, port)
eps := &discoveryv1.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "operator-ns",
Labels: map[string]string{
LabelParentName: "test",
LabelParentNamespace: "default",
labelSvcType: typeEgress,
labelProxyGroup: "foo",
},
},
AddressType: discoveryv1.AddressTypeIPv4,
}
fc := fake.NewClientBuilder().
WithScheme(tsapi.GlobalScheme).
WithObjects(svc, cm).
WithStatusSubresource(svc).
Build()
zl, err := zap.NewDevelopment()
if err != nil {
t.Fatal(err)
}
er := &egressEpsReconciler{Client: fc, logger: zl.Sugar(), tsNamespace: "operator-ns"}
mustCreate(t, fc, eps)
// Two ready Pods whose UIDs sort in the opposite order to their creation, so a stable sort is observable.
for _, p := range []struct {
uid string
ip string
}{{"pod-b", "10.0.0.2"}, {"pod-a", "10.0.0.1"}} {
pod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: p.uid,
Namespace: "operator-ns",
Labels: pgLabels("foo", nil),
UID: types.UID(p.uid),
},
Status: corev1.PodStatus{PodIPs: []corev1.PodIP{{IP: p.ip}}},
}
mustCreate(t, fc, pod)
s := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: p.uid,
Namespace: "operator-ns",
Labels: pgSecretLabels("foo", kubetypes.LabelSecretTypeState),
},
}
stBs := serviceStatusForPodIPs(t, svc, p.ip, "", port)
mak.Set(&s.Data, egressservices.KeyEgressServices, stBs)
mustCreate(t, fc, s)
}
expectReconciled(t, er, "operator-ns", "foo")
got := &discoveryv1.EndpointSlice{}
if err := fc.Get(t.Context(), types.NamespacedName{Name: "foo", Namespace: "operator-ns"}, got); err != nil {
t.Fatalf("getting EndpointSlice: %v", err)
}
if len(got.Endpoints) != 2 {
t.Fatalf("expected 2 endpoints, got %d", len(got.Endpoints))
}
if h0, h1 := *got.Endpoints[0].Hostname, *got.Endpoints[1].Hostname; h0 != "pod-a" || h1 != "pod-b" {
t.Errorf("endpoints not sorted by Hostname: got [%q, %q], want [\"pod-a\", \"pod-b\"]", h0, h1)
}
// A second reconcile over the unchanged ready set must not rewrite the slice.
rvBefore := got.ResourceVersion
expectReconciled(t, er, "operator-ns", "foo")
after := &discoveryv1.EndpointSlice{}
if err := fc.Get(t.Context(), types.NamespacedName{Name: "foo", Namespace: "operator-ns"}, after); err != nil {
t.Fatalf("getting EndpointSlice: %v", err)
}
if after.ResourceVersion != rvBefore {
t.Errorf("EndpointSlice was rewritten on a no-op reconcile: resourceVersion %s -> %s", rvBefore, after.ResourceVersion)
}
}
func configMapForSvc(t *testing.T, svc *corev1.Service, p uint16) *corev1.ConfigMap {
t.Helper()
ports := make(map[egressservices.PortMap]struct{})
@@ -88,12 +88,6 @@ func (esrr *egressSvcsReadinessReconciler) Reconcile(ctx context.Context, req re
return res, nil
}
// If an EndpointSlice for an expected family is missing, we mark the Service as NotReady.
//
// Setting the NotReady condition here is also used for best-effort recovery. The
// egress-svcs-reconciler does not watch EndpointSlices, so a deleted EndpointSlice is only
// recreated when this status change re-triggers a Service reconcile.
//
// TODO(beckypauley): refactor so EndpointSlice recovery is not dependent on Service status.
clusterIPSvc, err := getSingleObject[corev1.Service](ctx, esrr.Client, esrr.tsNamespace, crl)
if err != nil {
err = fmt.Errorf("error retrieving ClusterIP Service: %w", err)
+58 -38
View File
@@ -191,7 +191,25 @@ func (esr *egressSvcsReconciler) maybeProvision(ctx context.Context, svc *corev1
if clusterIPSvc == nil {
clusterIPSvc = esr.clusterIPSvcForEgress(crl)
}
// Detect which IP families the ClusterIP Service supports to determine which
// EndpointSlices must exist.
addrTypes, err := addrTypesForClusterIPSvc(clusterIPSvc)
if err != nil {
return err
}
upToDate := svcConfigurationUpToDate(svc, lg)
if upToDate && clusterIPSvc.Name != "" {
// The configuration can still match while an expected-family EndpointSlice
// is missing - check expected EndpointSlices exist.
missing, err := esr.missingEndpointSliceFamilies(ctx, clusterIPSvc, addrTypes)
if err != nil {
return err
}
if missing {
lg.Info("an expected EndpointSlice is missing, reprovisioning")
upToDate = false
}
}
provisioned := true
if !upToDate {
if clusterIPSvc, provisioned, err = esr.provision(ctx, svc.Annotations[AnnotationProxyGroup], svc, clusterIPSvc, lg); err != nil {
@@ -203,10 +221,6 @@ func (esr *egressSvcsReconciler) maybeProvision(ctx context.Context, svc *corev1
return nil
}
if err := esr.ensureEndpointSlices(ctx, svc, clusterIPSvc, lg); err != nil {
return err
}
// Update ExternalName Service to point at the ClusterIP Service.
clusterDomain := retrieveClusterDomain(esr.tsNamespace, lg)
clusterIPSvcFQDN := fmt.Sprintf("%s.%s.svc.%s", clusterIPSvc.Name, clusterIPSvc.Namespace, clusterDomain)
@@ -243,40 +257,6 @@ func addrTypesForClusterIPSvc(clusterIPSvc *corev1.Service) ([]discoveryv1.Addre
return addrTypes, nil
}
// ensureEndpointSlices ensures that EndpointSlices exist for the egress service
// for each IP family supported by the cluster, and that their ports are up to
// date.
func (esr *egressSvcsReconciler) ensureEndpointSlices(ctx context.Context, svc, clusterIPSvc *corev1.Service, lg *zap.SugaredLogger) error {
crl := egressSvcEpsLabels(svc, clusterIPSvc)
// Only create EndpointSlices for IP families supported by the cluster.
addrTypes, err := addrTypesForClusterIPSvc(clusterIPSvc)
if err != nil {
return err
}
for _, addrType := range addrTypes {
eps := &discoveryv1.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-%s", clusterIPSvc.Name, strings.ToLower(string(addrType))),
Namespace: esr.tsNamespace,
Labels: crl,
},
AddressType: addrType,
Ports: epsPortsFromSvc(clusterIPSvc),
}
if _, err := createOrUpdate(ctx, esr.Client, esr.tsNamespace, eps, func(e *discoveryv1.EndpointSlice) {
e.Labels = eps.Labels
e.AddressType = eps.AddressType
e.Ports = eps.Ports
for _, p := range e.Endpoints {
p.Conditions.Ready = nil
}
}); err != nil {
return fmt.Errorf("error ensuring %s EndpointSlice: %w", addrType, err)
}
}
return nil
}
func (esr *egressSvcsReconciler) provision(ctx context.Context, proxyGroupName string, svc, clusterIPSvc *corev1.Service, lg *zap.SugaredLogger) (*corev1.Service, bool, error) {
lg.Infof("updating configuration...")
usedPorts, err := esr.usedPortsForPG(ctx, proxyGroupName)
@@ -374,6 +354,29 @@ func (esr *egressSvcsReconciler) provision(ctx context.Context, proxyGroupName s
return nil, false, fmt.Errorf("error ensuring ClusterIP Service: %v", err)
}
}
crl := egressSvcEpsLabels(svc, clusterIPSvc)
addrTypes, err := addrTypesForClusterIPSvc(clusterIPSvc)
if err != nil {
return nil, false, err
}
for _, addrType := range addrTypes {
eps := &discoveryv1.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-%s", clusterIPSvc.Name, strings.ToLower(string(addrType))),
Namespace: esr.tsNamespace,
Labels: crl,
},
AddressType: addrType,
Ports: epsPortsFromSvc(clusterIPSvc),
}
if _, err := createOrUpdate(ctx, esr.Client, esr.tsNamespace, eps, func(e *discoveryv1.EndpointSlice) {
e.Labels = eps.Labels
e.AddressType = eps.AddressType
e.Ports = eps.Ports
}); err != nil {
return nil, false, fmt.Errorf("error ensuring %s EndpointSlice: %w", addrType, err)
}
}
cm, cfgs, err := egressSvcsConfigs(ctx, esr.Client, proxyGroupName, esr.tsNamespace)
if err != nil {
@@ -798,6 +801,23 @@ func tailnetSvcName(extNSvc *corev1.Service) string {
return fmt.Sprintf("%s-%s", extNSvc.Namespace, extNSvc.Name)
}
// missingEndpointSliceFamilies reports whether any EndpointSlice expected for the given ClusterIP Service (one per addrType)
// does not exist. Slices are looked up by name rather than labels so an orphaned EndpointSlice cannot satisfy the check.
func (esr *egressSvcsReconciler) missingEndpointSliceFamilies(ctx context.Context, clusterIPSvc *corev1.Service, addrTypes []discoveryv1.AddressType) (bool, error) {
for _, addrType := range addrTypes {
eps := &discoveryv1.EndpointSlice{}
name := fmt.Sprintf("%s-%s", clusterIPSvc.Name, strings.ToLower(string(addrType)))
err := esr.Get(ctx, types.NamespacedName{Namespace: esr.tsNamespace, Name: name}, eps)
if apierrors.IsNotFound(err) {
return true, nil
}
if err != nil {
return false, fmt.Errorf("error retrieving %s EndpointSlice: %w", addrType, err)
}
}
return false, nil
}
// epsPortsFromSvc takes the ClusterIP Service created for an egress service and
// returns its Port array in a form that can be used for an EndpointSlice.
func epsPortsFromSvc(svc *corev1.Service) (ep []discoveryv1.EndpointPort) {
+161
View File
@@ -23,6 +23,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
tsoperator "tailscale.com/k8s-operator"
tsapi "tailscale.com/k8s-operator/apis/v1alpha1"
"tailscale.com/kube/egressservices"
"tailscale.com/tstest"
@@ -449,6 +450,123 @@ func TestTailscaleEgressServicesDualStack(t *testing.T) {
})
}
// TestEgressServiceGatedProvision verifies that egress provisioning is gated on
// up-to-date configuration. If an expected EndpointSlice is
// missing, provision must recreate it, without depending on a configuration change
// and without reshuffling the ClusterIP Service ports.
func TestEgressServiceGatedProvision(t *testing.T) {
pg := &tsapi.ProxyGroup{
TypeMeta: metav1.TypeMeta{Kind: "ProxyGroup", APIVersion: "tailscale.com/v1alpha1"},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
UID: types.UID("1234-UID"),
},
Spec: tsapi.ProxyGroupSpec{
Replicas: pointer.To[int32](3),
Type: tsapi.ProxyGroupTypeEgress,
},
}
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: pgEgressCMName("foo"),
Namespace: "operator-ns",
},
}
zl, err := zap.NewDevelopment()
if err != nil {
t.Fatal(err)
}
clock := tstest.NewClock(tstest.ClockOpts{})
// Mark the ProxyGroup Available so validateClusterResources does not strip
// the EgressSvcConfigured condition.
tsoperator.SetProxyGroupCondition(pg, tsapi.ProxyGroupAvailable, metav1.ConditionTrue, "foo", "foo", pg.Generation, clock, zl.Sugar())
fc := fake.NewClientBuilder().
WithScheme(tsapi.GlobalScheme).
WithObjects(pg, cm).
WithStatusSubresource(pg, &corev1.Service{}).
WithInterceptorFuncs(interceptor.Funcs{
Create: clusterIPInterceptor("10.96.0.1"),
}).
Build()
esr := &egressSvcsReconciler{
Client: fc,
logger: zl.Sugar(),
clock: clock,
tsNamespace: "operator-ns",
}
svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
UID: types.UID("1234-UID"),
Annotations: map[string]string{
AnnotationTailnetTargetFQDN: "foo.bar.ts.net.",
AnnotationProxyGroup: "foo",
},
},
Spec: corev1.ServiceSpec{
ExternalName: "placeholder",
Type: corev1.ServiceTypeExternalName,
Ports: []corev1.ServicePort{
{Protocol: "TCP", Port: 80},
},
},
}
mustCreate(t, fc, svc)
// First reconcile: provisions the single-stack (IPv4) EndpointSlice and sets
// EgressSvcConfigured=True.
expectReconciled(t, esr, "default", "test")
name := findGenNameForEgressSvcResources(t, fc, svc)
v4Name := fmt.Sprintf("%s-ipv4", name)
v6Name := fmt.Sprintf("%s-ipv6", name)
expectMissing[discoveryv1.EndpointSlice](t, fc, "operator-ns", v6Name)
gotSvc := &corev1.Service{}
if err := fc.Get(t.Context(), client.ObjectKey{Namespace: "default", Name: "test"}, gotSvc); err != nil {
t.Fatalf("getting Service: %v", err)
}
if cond := tsoperator.GetServiceCondition(gotSvc, tsapi.EgressSvcConfigured); cond == nil || cond.Status != metav1.ConditionTrue {
t.Fatalf("EgressSvcConfigured condition not True after first reconcile: %+v", cond)
}
// Second reconcile: the service is up to date (same single family), so the
// gate must skip provision and not rewrite the IPv4 EndpointSlice.
v4Before := mustGetEndpointSlice(t, fc, v4Name)
expectReconciled(t, esr, "default", "test")
v4After := mustGetEndpointSlice(t, fc, v4Name)
if v4Before.ResourceVersion != v4After.ResourceVersion {
t.Errorf("IPv4 EndpointSlice was rewritten on a no-op reconcile: resourceVersion %s -> %s", v4Before.ResourceVersion, v4After.ResourceVersion)
}
// If an expected EndpointSlice is missing while the config is up to date, the
// missingEndpointSliceFamilies check must flip the gate false so provision reruns
// and recreates the slice, without reshuffling the ClusterIP Service ports.
clusterPortsBefore := mustGetClusterIPSvc(t, fc, name).Spec.Ports
if err := fc.Delete(t.Context(), &discoveryv1.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{Name: v4Name, Namespace: "operator-ns"},
}); err != nil {
t.Fatalf("deleting EndpointSlice: %v", err)
}
expectMissing[discoveryv1.EndpointSlice](t, fc, "operator-ns", v4Name)
expectReconciled(t, esr, "default", "test")
clusterSvc := mustGetClusterIPSvc(t, fc, name)
expectEqual(t, fc, endpointSlice(name, svc, clusterSvc, discoveryv1.AddressTypeIPv4))
if diff := cmp.Diff(clusterSvc.Spec.Ports, clusterPortsBefore); diff != "" {
t.Errorf("ClusterIP Service ports changed after reprovision (-got +want):\n%s", diff)
}
}
func mustGetEndpointSlice(t *testing.T, cl client.Client, name string) *discoveryv1.EndpointSlice {
t.Helper()
eps := &discoveryv1.EndpointSlice{}
if err := cl.Get(context.Background(), client.ObjectKey{Namespace: "operator-ns", Name: name}, eps); err != nil {
t.Fatalf("getting EndpointSlice %s: %v", name, err)
}
return eps
}
// clusterIPInterceptor returns an interceptor.Funcs Create function that
// simulates the API server assigning ClusterIPs to ClusterIP Services.
// This is required because the reconciler iterates ClusterIPs to create
@@ -462,3 +580,46 @@ func clusterIPInterceptor(clusterIPs ...string) func(ctx context.Context, c clie
return c.Create(ctx, obj, opts...)
}
}
// TestMissingEndpointSliceFamilies verifies that the check reports an expected IP family's
// EndpointSlice as missing until a slice named for the current ClusterIP Service exists.
func TestMissingEndpointSliceFamilies(t *testing.T) {
zl, err := zap.NewDevelopment()
if err != nil {
t.Fatal(err)
}
extNSvc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
Annotations: map[string]string{
AnnotationTailnetTargetFQDN: "foo.bar.ts.net.",
AnnotationProxyGroup: "foo",
},
},
}
current := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "ts-foo-current", Namespace: "operator-ns"}}
fc := fake.NewClientBuilder().
WithScheme(tsapi.GlobalScheme).
Build()
esr := &egressSvcsReconciler{Client: fc, logger: zl.Sugar(), tsNamespace: "operator-ns"}
// No slice exists for the current ClusterIP Service yet.
missing, err := esr.missingEndpointSliceFamilies(t.Context(), current, []discoveryv1.AddressType{discoveryv1.AddressTypeIPv4})
if err != nil {
t.Fatalf("missingEndpointSliceFamilies: %v", err)
}
if !missing {
t.Error("expected missing=true when no EndpointSlice exists for the current ClusterIP Service")
}
// Should not be mising once the EndpointSlice exists.
mustCreate(t, fc, endpointSlice("ts-foo-current", extNSvc, current, discoveryv1.AddressTypeIPv4))
missing, err = esr.missingEndpointSliceFamilies(t.Context(), current, []discoveryv1.AddressType{discoveryv1.AddressTypeIPv4})
if err != nil {
t.Fatalf("missingEndpointSliceFamilies: %v", err)
}
if missing {
t.Error("expected missing=false once the current ClusterIP Service's IPv4 slice exists")
}
}
+2 -1
View File
@@ -572,12 +572,14 @@ func runReconcilers(opts reconcilerOpts) {
}
egressSvcFilter := handler.EnqueueRequestsFromMapFunc(egressSvcsHandler)
egressSvcFromEpsFilter := handler.EnqueueRequestsFromMapFunc(egressSvcFromEps)
egressProxyGroupFilter := handler.EnqueueRequestsFromMapFunc(egressSvcsFromEgressProxyGroup(mgr.GetClient(), opts.log))
err = builder.
ControllerManagedBy(mgr).
Named("egress-svcs-reconciler").
Watches(&corev1.Service{}, egressSvcFilter).
Watches(&tsapi.ProxyGroup{}, egressProxyGroupFilter).
Watches(&discoveryv1.EndpointSlice{}, egressSvcFromEpsFilter).
Complete(&egressSvcsReconciler{
Client: mgr.GetClient(),
tsNamespace: opts.tailscaleNamespace,
@@ -592,7 +594,6 @@ func runReconcilers(opts reconcilerOpts) {
startlog.Fatalf("failed setting up indexer for egress Services: %v", err)
}
egressSvcFromEpsFilter := handler.EnqueueRequestsFromMapFunc(egressSvcFromEps)
err = builder.
ControllerManagedBy(mgr).
Named("egress-svcs-readiness-reconciler").