diff --git a/api/v1alpha1/sandbox_conversion.go b/api/v1alpha1/sandbox_conversion.go index 55e6518..18eb225 100644 --- a/api/v1alpha1/sandbox_conversion.go +++ b/api/v1alpha1/sandbox_conversion.go @@ -25,9 +25,7 @@ import ( const v1alpha1SandboxStateAnnotation = "api.agents.x-k8s.io/v1alpha1-sandbox-state" -// v1alpha1State is the round-trip payload: the fields v1beta1 cannot represent -// (replica counts collapse into OperatingMode). Legacy annotations carrying a -// full v1alpha1 Sandbox JSON decode into this shape too. +// v1alpha1State is the round-trip payload for the fields v1beta1 cannot represent. type v1alpha1State struct { Spec struct { Replicas *int32 `json:"replicas,omitempty"` diff --git a/controllers/sandbox_controller.go b/controllers/sandbox_controller.go index f52b549..abfb23a 100644 --- a/controllers/sandbox_controller.go +++ b/controllers/sandbox_controller.go @@ -1002,11 +1002,7 @@ func (r *SandboxReconciler) deleteExpiredChild(ctx context.Context, sandbox *san return nil } -// CacheByObject scopes a manager cache to the child objects this controller -// labels, so the Pod/Service/PVC informers never watch the whole cluster. Every -// cached read this controller makes targets a labeled child, and the PVC entry -// also stops the first volumeClaimTemplates Sandbox from spinning up a -// cluster-wide informer mid-reconcile. +// CacheByObject scopes the manager cache to this controller's labeled children, so no child informer watches the whole cluster. func CacheByObject() (map[client.Object]cache.ByObject, error) { sel, err := labels.Parse(sandboxLabel) if err != nil { @@ -1039,10 +1035,7 @@ func MergeVolumeClaimVolumes(existing, pvcVolumes []corev1.Volume) []corev1.Volu return append(filtered, pvcVolumes...) } -// checkOwnership determines whether a Kubernetes resource is owned by the given Sandbox, -// has no controller, or is owned by a different controller. -// It returns both the ownership classification and the controller reference (if any), -// so callers can log owner details without redundant GetControllerOf calls. +// checkOwnership classifies obj's ownership relative to sandbox and returns the controller reference it read. func checkOwnership(obj client.Object, sandbox *sandboxv1beta1.Sandbox) (resourceOwnership, *metav1.OwnerReference) { controllerRef := metav1.GetControllerOf(obj) if controllerRef == nil { @@ -1141,6 +1134,11 @@ func podIPsFromStatus(podIPs []corev1.PodIP) []string { return ips } +type ( + keyPredicate func(string) bool + keyCallback func(string) +) + // hasSystemReservedPrefix reports whether a key uses a label/annotation prefix // reserved for the sandbox system or its extensions. func hasSystemReservedPrefix(key string) bool { @@ -1178,7 +1176,7 @@ func isControllerManagedPodAnnotation(key string) bool { // filterSystemKeys copies src minus system-reserved keys, returning the copy and // the sorted-by-caller list of keys it carries. -func filterSystemKeys(src map[string]string, isSystem func(string) bool, onSkip func(string)) (map[string]string, []string) { +func filterSystemKeys(src map[string]string, isSystem keyPredicate, onSkip keyCallback) (map[string]string, []string) { out := make(map[string]string, len(src)) var kept []string for k, v := range src { @@ -1236,7 +1234,7 @@ func setEntry(m map[string]string, key, want string) bool { // propagateKeys copies template into live, skipping system-reserved keys, and // returns the keys it now manages plus whether live changed. -func propagateKeys(live, template map[string]string, isSystem func(string) bool, onSkip func(string)) ([]string, bool) { +func propagateKeys(live, template map[string]string, isSystem keyPredicate, onSkip keyCallback) ([]string, bool) { var managed []string updated := false for k, v := range template { @@ -1255,7 +1253,7 @@ func propagateKeys(live, template map[string]string, isSystem func(string) bool, // prunePropagated drops previously propagated keys the template no longer carries. // A system key recorded by an older controller is scrubbed unless keep claims it. -func prunePropagated(live map[string]string, recorded string, template map[string]string, isSystem, keep func(string) bool, onScrub func(string)) bool { +func prunePropagated(live map[string]string, recorded string, template map[string]string, isSystem, keep keyPredicate, onScrub keyCallback) bool { updated := false for k := range strings.SplitSeq(recorded, ",") { if k == "" { diff --git a/pkg/scale/apiserver/lifecycle_storage.go b/pkg/scale/apiserver/lifecycle_storage.go index 1390c87..330cdd1 100644 --- a/pkg/scale/apiserver/lifecycle_storage.go +++ b/pkg/scale/apiserver/lifecycle_storage.go @@ -42,7 +42,6 @@ func (r *lifecycleREST) Destroy() {} func (r *lifecycleREST) NamespaceScoped() bool { return true } -// GroupVersionKind pins the kind the request pipeline decodes the body as. func (r *lifecycleREST) GroupVersionKind(schema.GroupVersion) schema.GroupVersionKind { gvks, _, err := Scheme.ObjectKinds(r.newOptions()) if err != nil || len(gvks) == 0 { @@ -51,8 +50,6 @@ func (r *lifecycleREST) GroupVersionKind(schema.GroupVersion) schema.GroupVersio return gvks[0] } -// Create runs the verb. The subresource's parent name arrives separately from -// the body, which is why this is a NamedCreater. func (r *lifecycleREST) Create( ctx context.Context, name string, diff --git a/pkg/scale/apiserver/storage.go b/pkg/scale/apiserver/storage.go index 84b99d8..76580c5 100644 --- a/pkg/scale/apiserver/storage.go +++ b/pkg/scale/apiserver/storage.go @@ -100,11 +100,6 @@ func (r *sandboxREST) Watch(ctx context.Context, options *metainternalversion.Li return r.store.Watch(ctx, toScaleListOptions(ctx, options)) } -// Create is the L3 write path: it derives the warm pool from the submitted -// Sandbox and asks the store to hand over an already-running microVM from a node -// with warm capacity for that pool (a synchronous node-local claim). It writes NO -// per-sandbox object to etcd; it synthesizes and returns a Ready Sandbox carrying -// the claim id + connection address. With no warm node it returns a retryable 503. func (r *sandboxREST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, _ *metav1.CreateOptions) (runtime.Object, error) { sb, ok := obj.(*sandboxv1beta1.Sandbox) if !ok { @@ -149,12 +144,8 @@ func (r *sandboxREST) Create(ctx context.Context, obj runtime.Object, createVali return synthesizeClaimedSandbox(namespace, name, sb, assignment), nil } -// Delete is the L3 release path. It resolves the sandbox from live node inventory -// (so it knows the owning node), reads the sandboxd claim id the node published -// on that entry, and releases the node-local claim via the store. Per the delete- -// authorization contract this is owner-authorized teardown of the Sandbox resource -// itself; it never destroys a VM on pod state alone. Returns (object, true). func (r *sandboxREST) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, _ *metav1.DeleteOptions) (runtime.Object, bool, error) { + // owner-authorized teardown of the Sandbox resource only; pod state never reaches here namespace := genericapirequest.NamespaceValue(ctx) sb, err := r.store.Get(ctx, namespace, name) if err != nil { diff --git a/pkg/scale/claimgateway_impl.go b/pkg/scale/claimgateway_impl.go index bf794b6..0b4fe10 100644 --- a/pkg/scale/claimgateway_impl.go +++ b/pkg/scale/claimgateway_impl.go @@ -195,16 +195,8 @@ func (g *nodeClaimGateway) Claim(ctx context.Context, req ClaimRequest) (Assignm } // Release destroys the node-local microVM backing the Assignment. -// -// DELETE-AUTHORIZATION CONTRACT (G-0131 hard constraint). Release is invoked ONLY -// on owner-authorized teardown — when the SandboxClaim CR itself is being released -// or deleted by its owner. It is NEVER wired to Pod deletion or any pod-level -// state: a Pod disappearing must not destroy the VM. There is deliberately no code -// path in this package from Pod state to Release. As a structural guard the -// gateway only releases sandboxes it actually delivered (it holds the sandbox's -// own token, the Release credential); an Assignment the gateway never handed out -// — e.g. one synthesized from stale pod state — is refused without any sandboxd -// call, so no such state can drive a destroy. +// It runs only on owner-authorized teardown, never off pod state — the same +// delete-authorization contract the aggregated apiserver's Delete honors. func (g *nodeClaimGateway) Release(ctx context.Context, a Assignment) error { g.mu.Lock() d, ok := g.holdings[a.SandboxName] @@ -382,7 +374,6 @@ type ReviewAuthorizer struct { Verb string } -// Authorize denies missing identities and runs SubjectAccessReview. func (a *ReviewAuthorizer) Authorize(ctx context.Context, req ClaimRequest) error { user := req.Selector[RequestUserSelectorKey] if user == "" { diff --git a/pkg/scale/sandboxstore_impl.go b/pkg/scale/sandboxstore_impl.go index 53b4752..ba29e7d 100644 --- a/pkg/scale/sandboxstore_impl.go +++ b/pkg/scale/sandboxstore_impl.go @@ -187,6 +187,8 @@ func NewSandboxdClientFactory() SandboxdClientFactory { } } +type inventoryMatch func(inv *NodeInventory, i int) bool + var _ SandboxStore = (*scatterGatherStore)(nil) // scatterGatherStore is the concrete SandboxStore: List/Get/Watch synthesize @@ -381,7 +383,7 @@ func (s *scatterGatherStore) Watch(ctx context.Context, opts ListOptions) (watch // It reads the node the index last saw holding key and only sweeps the whole // fleet on a miss, canceling the rest of the sweep on the hit. Nil with a nil // error means no entry matched. -func (s *scatterGatherStore) findEntry(ctx context.Context, op, key string, match func(inv *NodeInventory, i int) bool) (*sandboxv1beta1.Sandbox, error) { +func (s *scatterGatherStore) findEntry(ctx context.Context, op, key string, match inventoryMatch) (*sandboxv1beta1.Sandbox, error) { if node, ok := s.index.lookup(key); ok { if sb := s.matchOnNode(ctx, node, match); sb != nil { return sb, nil @@ -439,7 +441,7 @@ func (s *scatterGatherStore) findEntry(ctx context.Context, op, key string, matc // matchOnNode resolves match against one node's inventory, returning nil when // that node is unreadable or no longer holds the entry. -func (s *scatterGatherStore) matchOnNode(ctx context.Context, node string, match func(inv *NodeInventory, i int) bool) *sandboxv1beta1.Sandbox { +func (s *scatterGatherStore) matchOnNode(ctx context.Context, node string, match inventoryMatch) *sandboxv1beta1.Sandbox { inv, err := s.src.NodeInventory(ctx, node) if err != nil { return nil @@ -706,8 +708,6 @@ func (s *StaticInventorySource) Put(inv *NodeInventory) { s.inv[inv.Node] = inv.DeepCopy() } -// Apply implements InventoryApplier: it stores the inventory and counts the -// apply, modeling the single O(nodes) server-side-apply write per node. func (s *StaticInventorySource) Apply(_ context.Context, inv *NodeInventory) error { s.mu.Lock() defer s.mu.Unlock() @@ -732,15 +732,12 @@ func (s *StaticInventorySource) Remove(node string) { delete(s.partition, node) } -// ListNodes returns the known node names in stable order. func (s *StaticInventorySource) ListNodes(_ context.Context) ([]string, error) { s.mu.RLock() defer s.mu.RUnlock() return slices.Sorted(maps.Keys(s.inv)), nil } -// NodeInventory returns a copy of one node's inventory, or an error if the node -// is partitioned or unknown. func (s *StaticInventorySource) NodeInventory(_ context.Context, node string) (*NodeInventory, error) { s.mu.RLock() defer s.mu.RUnlock() @@ -754,7 +751,6 @@ func (s *StaticInventorySource) NodeInventory(_ context.Context, node string) (* return inv.DeepCopy(), nil } -// NodeCapacity returns one node's address and pools without copying its entries. func (s *StaticInventorySource) NodeCapacity(_ context.Context, node string) (string, []PoolCapacity, error) { s.mu.RLock() defer s.mu.RUnlock() @@ -800,7 +796,6 @@ func NewClientInventorySource(reader client.Reader) *ClientInventorySource { return &ClientInventorySource{reader: reader} } -// ListNodes lists NodeInventory objects (O(nodes)) and returns their names. func (s *ClientInventorySource) ListNodes(ctx context.Context) ([]string, error) { ul := &unstructured.UnstructuredList{} ul.SetGroupVersionKind(NodeInventoryGVK.GroupVersion().WithKind(NodeInventoryGVK.Kind + "List")) @@ -815,7 +810,6 @@ func (s *ClientInventorySource) ListNodes(ctx context.Context) ([]string, error) return nodes, nil } -// NodeInventory fetches and decodes one node's NodeInventory object. func (s *ClientInventorySource) NodeInventory(ctx context.Context, node string) (*NodeInventory, error) { u := &unstructured.Unstructured{} u.SetGroupVersionKind(NodeInventoryGVK) @@ -829,8 +823,6 @@ func (s *ClientInventorySource) NodeInventory(ctx context.Context, node string) return inv, nil } -// NodeCapacity decodes only the address and pools fields, so the claim and -// routing paths never pay for the node's whole entry list. func (s *ClientInventorySource) NodeCapacity(ctx context.Context, node string) (string, []PoolCapacity, error) { u := &unstructured.Unstructured{} u.SetGroupVersionKind(NodeInventoryGVK) diff --git a/pkg/scale/sandboxstore_lifecycle.go b/pkg/scale/sandboxstore_lifecycle.go index d545ed0..ea6bf7d 100644 --- a/pkg/scale/sandboxstore_lifecycle.go +++ b/pkg/scale/sandboxstore_lifecycle.go @@ -7,7 +7,6 @@ import ( "github.com/cocoonstack/sandbox-operator/pkg/sandboxd" ) -// Pause hibernates a delivered sandbox on its owning node. func (s *scatterGatherStore) Pause(ctx context.Context, node, id string) error { cl, err := s.nodeClient(ctx, node, "pause", id) if err != nil { @@ -19,7 +18,6 @@ func (s *scatterGatherStore) Pause(ctx context.Context, node, id string) error { return nil } -// Resume restores a paused sandbox through cocoon's mmap fast path. func (s *scatterGatherStore) Resume(ctx context.Context, node, id string) error { cl, err := s.nodeClient(ctx, node, "resume", id) if err != nil { @@ -31,8 +29,6 @@ func (s *scatterGatherStore) Resume(ctx context.Context, node, id string) error return nil } -// Fork branches a sandbox into count children on its owning node. Children are -// fresh claims with their own ids; the parent keeps running. func (s *scatterGatherStore) Fork(ctx context.Context, node, id string, count, ttlSeconds int) ([]Assignment, error) { if count < 1 { return nil, fmt.Errorf("scale: fork count must be >= 1, got %d", count) @@ -58,7 +54,6 @@ func (s *scatterGatherStore) Fork(ctx context.Context, node, id string, count, t return out, nil } -// Snapshot captures a sandbox's state as a checkpoint on its owning node. func (s *scatterGatherStore) Snapshot(ctx context.Context, node, id, name string) (Snapshot, error) { cl, err := s.nodeClient(ctx, node, "snapshot", id) if err != nil { @@ -71,7 +66,6 @@ func (s *scatterGatherStore) Snapshot(ctx context.Context, node, id, name string return snapshotFrom(ck, node), nil } -// Snapshots lists a node's checkpoints. func (s *scatterGatherStore) Snapshots(ctx context.Context, node string) ([]Snapshot, error) { cl, err := s.nodeClient(ctx, node, "list snapshots", "") if err != nil { @@ -88,7 +82,6 @@ func (s *scatterGatherStore) Snapshots(ctx context.Context, node string) ([]Snap return out, nil } -// DeleteSnapshot removes a checkpoint from its node. func (s *scatterGatherStore) DeleteSnapshot(ctx context.Context, node, snapshotID string) error { cl, err := s.nodeClient(ctx, node, "delete snapshot", snapshotID) if err != nil { @@ -100,7 +93,6 @@ func (s *scatterGatherStore) DeleteSnapshot(ctx context.Context, node, snapshotI return nil } -// Stats reports a sandbox's resource usage from its owning node. func (s *scatterGatherStore) Stats(ctx context.Context, node, id string) (SandboxStats, error) { cl, err := s.nodeClient(ctx, node, "stats", id) if err != nil { diff --git a/pkg/scale/warmpool/driver.go b/pkg/scale/warmpool/driver.go index eb1a9b3..7b1f666 100644 --- a/pkg/scale/warmpool/driver.go +++ b/pkg/scale/warmpool/driver.go @@ -116,13 +116,6 @@ func New(kube client.Client, inv scale.InventorySource, token string, factory Cl return &Driver{kube: kube, inv: inv, token: token, factory: factory, interval: opts.Interval, log: opts.Log} } -// Reconcile runs a full pool reconcile on ANY SandboxWarmPool or NodeInventory -// event — so a `kubectl apply/patch/delete` reacts in milliseconds, not after a -// poll tick. It ignores the request key (the loop is global, O(pools+nodes)) and -// requeues after d.interval, which doubles as the sampling period of the warm -// count written back to status. Target changes therefore land in milliseconds -// (event-driven), while the fill they kick off — the node side provisions at -// O(100)/s per node — becomes visible one interval at a time. func (d *Driver) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) { if err := d.reconcileOnce(ctx); err != nil { return ctrl.Result{}, err