Files
Eren Akbulut 94c91b21fb Revert and delete no longer hang when an actor's node is deleted (#2486)
After a node is deleted, revert can still hang for a CRASHED actor or a
paused actor whose latest upload is PENDING. Delete can hang for any
actor still bound to that node. #2883 already lets revert and resume
forget an unreachable node when the actor has no worker assignment or
crash and the latest local snapshot has a COMPLETED durable copy.

When no atelet is indexed on the assigned node, `ensureAteletTerminated`
uses `NodeGone` to check the Node through the live Kubernetes API. It
skips termination only on NotFound, after checking again that no atelet
appeared. Existing nodes and lookup failures stay retryable. The API
server gains `get` permission on nodes.

External volumes still go through `DetachVolume`: nil and NotFound
succeed, other errors propagate. Kubernetes Node deletion does not fence
a machine that is still running; resources on such a machine may remain.

Rebased onto main `872ab2077`. The whole-workflow test comment now
identifies its paused actor's PENDING upload. Validation:
- Targeted revert/delete/detach and `TestNodeGone` tests pass with
`-race -count=3 -shuffle=on` and Docker-backed database tests enabled.
- The full race suite passes all 15 ateapi packages with tests. Overall
it reports 10,601 passing test/subtest events, 8 skipped, and two
failures in `cmd/ateworker-microvm/internal/child`:
`TestReapedWhenItDiesOnItsOwn` and `TestStop`. Both report `unsupported
operation` on macOS and reproduce from unchanged main `872ab2077`; the
non-Linux pidfd implementation returns `errors.ErrUnsupported`.
- `hack/verify-all.sh` passes (using local Python 3.13 and clang-format
23.1.3).
- A fresh independent review found no issues in the rebased patch. `git
diff --check` passes.

Earlier kind runs exercised real worker deletion, paused/crashed revert
and delete, and the kept-node control. Those live runs were not repeated
for this rebase. Current volume tests use a mock plugin; no real
CSI-driver validation is claimed.

Part of #2470. Resume without a completed latest upload remains
separate.

AI assistance: Claude Code helped write the original code and tests.
Codex clarified the fixture comment, rebased the branch, reviewed the
integration, and ran validation.
2026-10-20 21:32:38 +00:00

269 lines
10 KiB
Go

// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package controlapi
import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"net"
"slices"
"strconv"
"sync"
"github.com/spiffe/go-spiffe/v2/bundle/x509bundle"
"github.com/spiffe/go-spiffe/v2/spiffeid"
"github.com/spiffe/go-spiffe/v2/svid/x509svid"
"go.ate.dev/substrate/internal/atelet"
"go.ate.dev/substrate/internal/credbundle"
"go.ate.dev/substrate/internal/installdefaults"
"go.ate.dev/substrate/internal/substratex509"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/tools/cache"
"k8s.io/utils/lru"
)
// ErrNoAteletOnNode reports that the informer cache holds no atelet pod for
// the requested node — e.g. the atelet is restarting, or the node is gone.
// Retryable.
var ErrNoAteletOnNode = errors.New("no atelet pod found on node")
// ateletConn is a cached connection and the pod IP it was dialed on.
type ateletConn struct {
ip string
conn *grpc.ClientConn
}
// AteletDialer handles gRPC connections to Atelet pods.
type AteletDialer struct {
ateletIndexer cache.Indexer
// getNode, when set, reads a node from the API server. NodeGone uses it to
// tell a deleted node from an atelet that is only missing for now.
getNode func(ctx context.Context, nodeName string) error
// mu makes the lookup, stale eviction and insert in DialForAteletOnNode
// atomic, so concurrent callers cannot evict each other's fresh conn.
mu sync.Mutex
// ateletConns holds one *ateletConn per atelet pod UID.
ateletConns *lru.Cache
// dialCredentials builds the transport credentials used to dial a given
// atelet, keyed on the atelet's expected pod UID. Production wires this to
// per-atelet mTLS; tests can override it with insecure credentials.
dialCredentials func(expectedPodUID string) (credentials.TransportCredentials, error)
}
// DialerOption customizes an AteletDialer built by NewAteletDialer.
type DialerOption func(*AteletDialer)
// WithDialCredentials overrides how transport credentials are built for a given
// atelet pod UID. Tests use it to reach a fake atelet over insecure transport
// while still exercising the real lookup, dial and connection-cache path.
func WithDialCredentials(build func(expectedPodUID string) (credentials.TransportCredentials, error)) DialerOption {
return func(d *AteletDialer) { d.dialCredentials = build }
}
// WithNodeGetter sets how NodeGone reads a node. getNode returns the API
// server's error, so a NotFound error means that the node is deleted.
func WithNodeGetter(getNode func(ctx context.Context, nodeName string) error) DialerOption {
return func(d *AteletDialer) { d.getNode = getNode }
}
// NodeGone reports whether nodeName is deleted from the cluster and no atelet
// pod is known on it. It reads the node live, so a stale cache cannot report a
// node as gone. Without a node getter it always reports false.
func (d *AteletDialer) NodeGone(ctx context.Context, nodeName string) (bool, error) {
if d.getNode == nil {
return false, nil
}
if err := d.getNode(ctx, nodeName); !apierrors.IsNotFound(err) {
return false, err // nil when the node exists
}
// An atelet that registered while the node was read means the node is back.
atelets, err := d.ateletIndexer.ByIndex(byNode, nodeName)
if err != nil {
return false, fmt.Errorf("while finding atelet on node %q: %w", nodeName, err)
}
return len(atelets) == 0, nil
}
// NewAteletDialer creates a new AteletDialer. clientBundlePath and serverCAPath
// are used to build the per-atelet mTLS credentials used for every atelet
// connection, and ateletSPIFFEID is the identity those credentials expect on
// the atelet serving cert.
func NewAteletDialer(ateletIndexer cache.Indexer, ateletSPIFFEID, clientBundlePath, serverCAPath string, opts ...DialerOption) *AteletDialer {
d := &AteletDialer{
ateletIndexer: ateletIndexer,
ateletConns: newAteletConnCache(1024),
dialCredentials: func(expectedPodUID string) (credentials.TransportCredentials, error) {
tlsConfig, err := buildTLSConfig(ateletSPIFFEID, clientBundlePath, serverCAPath, expectedPodUID)
if err != nil {
return nil, err
}
return credentials.NewTLS(tlsConfig), nil
},
}
for _, opt := range opts {
opt(d)
}
return d
}
// newAteletConnCache builds the atelet conn cache, which closes the
// connections it evicts. A conn pushed out of the LRU without Close is not
// reclaimed: grpc keeps most of its goroutines and buffers alive for the
// life of the process. Closing on eviction can fail an RPC still in flight
// on a conn that aged to the LRU tail, but that failure is visible and
// retryable, unlike the leak.
//
// TODO: Consider pool semantics instead of a cache: a conn evicted for
// capacity would drain — close only once its last in-flight RPC finishes
// (e.g. refcounted checkout/release) — rather than being closed out from
// under a caller. Worth revisiting if the retryable eviction failures show
// up in practice.
func newAteletConnCache(size int) *lru.Cache {
return lru.NewWithEvictionFunc(size, func(_ lru.Key, value interface{}) {
_ = value.(*ateletConn).conn.Close()
})
}
// DialForAteletOnNode resolves the single atelet pod on nodeName and dials it
// with per-atelet pod-UID-pinned credentials, caching the connection by the
// atelet's pod UID. A pod that keeps its UID but gets a new IP, such as after a
// node restart, is redialed.
func (d *AteletDialer) DialForAteletOnNode(nodeName string) (*grpc.ClientConn, error) {
matchingAtelets, err := d.ateletIndexer.ByIndex(byNode, nodeName)
if err != nil {
return nil, fmt.Errorf("while finding atelet on node %q: %w", nodeName, err)
}
if len(matchingAtelets) == 0 {
return nil, fmt.Errorf("%w: %q", ErrNoAteletOnNode, nodeName)
}
if len(matchingAtelets) > 1 {
return nil, fmt.Errorf("found %d atelet pods on node %q, expected 1", len(matchingAtelets), nodeName)
}
selectedAtelet := matchingAtelets[0].(*corev1.Pod)
if len(selectedAtelet.Status.PodIPs) == 0 {
return nil, fmt.Errorf("selected atelet %q has no assigned IPs", selectedAtelet.ObjectMeta.Namespace+"/"+selectedAtelet.ObjectMeta.Name)
}
ateletKey := string(selectedAtelet.ObjectMeta.UID)
ateletIP := selectedAtelet.Status.PodIPs[0].IP
d.mu.Lock()
defer d.mu.Unlock()
if cached, ok := d.ateletConns.Get(ateletKey); ok {
if c := cached.(*ateletConn); c.ip == ateletIP {
return c.conn, nil
}
// Remove closes the stale connection.
d.ateletConns.Remove(ateletKey)
}
creds, err := d.dialCredentials(string(selectedAtelet.ObjectMeta.UID))
if err != nil {
return nil, fmt.Errorf("while building atelet credentials: %w", err)
}
conn, err := grpc.NewClient(
net.JoinHostPort(ateletIP, strconv.Itoa(atelet.DefaultPort)),
grpc.WithTransportCredentials(creds),
grpc.WithStatsHandler(otelgrpc.NewClientHandler()),
)
if err != nil {
return nil, fmt.Errorf("while creating atelet gRPC client connection: %w", err)
}
d.ateletConns.Add(ateletKey, &ateletConn{ip: ateletIP, conn: conn})
return conn, nil
}
func buildTLSConfig(ateletSPIFFEID, clientBundlePath, serverCAPath, expectedPodUID string) (*tls.Config, error) {
trustDomain, err := spiffeid.TrustDomainFromString(installdefaults.AteletTrustDomain)
if err != nil {
return nil, fmt.Errorf("while parsing trust domain %q: %w", installdefaults.AteletTrustDomain, err)
}
bundle, err := x509bundle.Load(trustDomain, serverCAPath)
if err != nil {
return nil, fmt.Errorf("while loading CA bundle from %s: %w", serverCAPath, err)
}
expectedID, err := spiffeid.FromString(ateletSPIFFEID)
if err != nil {
return nil, fmt.Errorf("while parsing expected atelet SPIFFE ID %q: %w", ateletSPIFFEID, err)
}
verify, err := verifyAteletServerCert(bundle, expectedID, expectedPodUID)
if err != nil {
return nil, fmt.Errorf("while creating atelet server cert verifier: %w", err)
}
tlsConfig := tls.Config{
MinVersion: tls.VersionTLS13,
GetClientCertificate: credbundle.ClientLoader(clientBundlePath),
// Skip the default verification because the peer is dialed by IP and its
// certificate has no DNS/IP SAN.
InsecureSkipVerify: true,
VerifyConnection: verify,
}
return &tlsConfig, nil
}
func verifyAteletServerCert(bundle *x509bundle.Bundle, expectedID spiffeid.ID, expectedPodUID string) (func(tls.ConnectionState) error, error) {
if expectedPodUID == "" {
return nil, fmt.Errorf("expected pod UID must not be empty")
}
if expectedID.IsZero() {
return nil, fmt.Errorf("expected pod spiffe ID must not be empty")
}
return func(cs tls.ConnectionState) error {
if len(cs.PeerCertificates) == 0 {
return fmt.Errorf("server presented no certificate")
}
id, _, err := x509svid.Verify(cs.PeerCertificates, bundle)
if err != nil {
return fmt.Errorf("verifying server certificate chain: %w", err)
}
if id != expectedID {
return fmt.Errorf("server SPIFFE ID %q does not match expected %q", id, expectedID)
}
leaf := cs.PeerCertificates[0]
if !slices.Contains(leaf.ExtKeyUsage, x509.ExtKeyUsageServerAuth) {
return fmt.Errorf("server certificate lacks the serverAuth extended key usage")
}
identity, err := substratex509.PodIdentityFromCertificate(leaf)
if err != nil {
return fmt.Errorf("failed to parse PodIdentity extension: %w", err)
}
if identity == nil {
return fmt.Errorf("server certificate has no PodIdentity extension, expected pod UID %q", expectedPodUID)
}
if identity.PodUID != expectedPodUID {
return fmt.Errorf("pod UID %q does not match expected %q", identity.PodUID, expectedPodUID)
}
return nil
}, nil
}