diff --git a/designs/site-node-slices.md b/designs/site-node-slices.md new file mode 100644 index 0000000000..f0fd5897ed --- /dev/null +++ b/designs/site-node-slices.md @@ -0,0 +1,57 @@ +# SiteNodeSlice Reconciliation + +## Purpose + +The site controller publishes non-gateway node connectivity data in +`SiteNodeSlice` resources. Kubernetes remains the authoritative persisted +state. Informer caches are used to avoid API reads when cached desired and +observed state already match. + +## Reconciliation Scope + +Node and `SiteNodeSlice` events dirty only the affected Sites when their Site +membership is known. A reconciliation pass still evaluates Site ownership +against the complete cached Site set, but only rebuilds slices, status, and +protection finalizers for dirty Sites. + +Changes with broad matching effects use a global fallback: + +- Site additions, updates, and deletions +- GatewayPool selector changes +- Initial controller startup +- Events whose resource type cannot be decoded safely + +This preserves correctness when a change can move nodes between Sites or +change which nodes are excluded as gateways. + +## Cache and API Authority + +An unchanged cache-synchronized pass performs no live Site or SiteNodeSlice +reads. Cached objects are comparison inputs only. Before a create, update, or +delete, the controller reads the live object and applies these fences: + +- Site UID must still match, preventing writes for a deleted and recreated Site. +- Slice updates start from the live object, preserving metadata, finalizers, + and fields owned by other actors. +- Conflict, AlreadyExists, and NotFound responses cause a fresh live read. +- Slice deletes use live UID and resourceVersion preconditions. +- Site status and protection finalizers use live state whenever a cached + difference requires mutation. + +Informer events provide eventual repair when another actor changes managed +slice fields or Site metadata. + +## Orphan Repair + +Orphan detection requires a live cluster-wide Site LIST and live slice reads, +so it does not run for every topology event. The controller runs it at startup +and periodically afterward. + +Repair remains conservative: + +- A failed Site LIST does not authorize any deletion. +- An empty Site LIST does not mean every slice is orphaned. +- Legacy net-group Sites are checked during migration. +- Orphan deletion uses live UID and resourceVersion preconditions. + +Owner-reference garbage collection remains the normal path for deleted Sites. diff --git a/e2e/operator/slice_window_e2e_test.go b/e2e/operator/slice_window_e2e_test.go index 77decc4c2f..e6151b7fbd 100644 --- a/e2e/operator/slice_window_e2e_test.go +++ b/e2e/operator/slice_window_e2e_test.go @@ -45,8 +45,8 @@ const sliceWindowClusterName = "operator-slice-window-e2e" // re-owned and still present. // - Phase 3 (partial migration): with the machina set non-empty, a slice whose // Site is still only in the legacy group must survive (legacySiteExists). -// - Phase 4 (genuine orphan): a slice whose Site is absent from both groups is -// still cleaned up, proving the guard did not neuter cleanup. +// - Phase 4 (genuine orphan): after controller restart, a slice whose Site is +// absent from both groups is cleaned up by the production startup repair. func TestSiteControllerPreservesSlicesDuringMigrationWindow(t *testing.T) { requireBins(t, "kind", "kubectl", "docker") @@ -100,6 +100,19 @@ func TestSiteControllerPreservesSlicesDuringMigrationWindow(t *testing.T) { defer cancelController() controllerResult := startRestrictedSiteController(controllerCtx, t, restrictedConfig) + stopController := func() { + t.Helper() + cancelController() + + select { + case err := <-controllerResult: + if err != nil { + t.Fatalf("restricted SiteController stopped with error: %v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("restricted SiteController did not stop") + } + } // Phase 1: empty machina window. The slice must never disappear while the // controller reconciles with zero machina Sites. @@ -118,21 +131,19 @@ func TestSiteControllerPreservesSlicesDuringMigrationWindow(t *testing.T) { stageLegacyOwnedSlice(ctx, t, cli, "edge-site-0", "edge-site", edgeSite.GetUID(), nodeData) assertSliceContinuouslyPresent(controllerCtx, t, cli, "edge-site-0", 8*time.Second, controllerResult) - // Phase 4: genuine orphan. A slice whose Site exists in neither group must be - // deleted, proving the guard did not disable cleanup. + // Phase 4: restart with a genuine orphan so the startup repair removes it + // without waiting for the low-frequency periodic repair. + stopController() stageLegacyOwnedSlice(ctx, t, cli, "ghost-0", "ghost", "", nodeData) - waitForSliceDeleted(controllerCtx, t, cli, "ghost-0", controllerResult) - cancelController() + controllerCtx, cancelController = context.WithCancel(ctx) + defer cancelController() - select { - case err := <-controllerResult: - if err != nil { - t.Fatalf("restricted SiteController stopped with error: %v", err) - } - case <-time.After(10 * time.Second): - t.Fatal("restricted SiteController did not stop") - } + controllerResult = startRestrictedSiteController(controllerCtx, t, restrictedConfig) + waitForSliceDeleted(controllerCtx, t, cli, "ghost-0", controllerResult) + assertSliceOwnerGroup(ctx, t, cli, "win-site-0", machinaSiteGVK.Group) + assertSliceOwnerGroup(ctx, t, cli, "edge-site-0", legacySiteGVK.Group) + stopController() } // firstKindNode returns the single kind node and its InternalIPs. diff --git a/internal/net/controller/site_controller.go b/internal/net/controller/site_controller.go index dc8e5e8d68..9c4f3b092d 100644 --- a/internal/net/controller/site_controller.go +++ b/internal/net/controller/site_controller.go @@ -43,6 +43,8 @@ import ( ) const ( + orphanSliceRepairInterval = 10 * time.Minute + // canonicalSiteLabelKey is the canonical site-membership label the // controller applies to Nodes (shared with the Machine site label). It // supersedes deprecatedSiteLabelKey. @@ -159,6 +161,11 @@ type SiteController struct { // slicesDirty is set when node or slice state changes. The periodic loop // atomically consumes it to coalesce updates and restores it after failures. slicesDirty atomic.Bool + // sliceDirtySites limits ordinary slice reconciliation to affected Sites. + // allSlicesDirty is reserved for changes with cluster-wide matching effects. + sliceDirtyLock sync.Mutex + sliceDirtySites map[string]struct{} + allSlicesDirty bool // hasSynced indicates whether the informer caches have completed initial sync hasSynced atomic.Bool @@ -258,6 +265,7 @@ func NewSiteController( workqueue: workqueue.NewTypedRateLimitingQueueWithConfig(workqueue.DefaultTypedControllerRateLimiter[string](), workqueue.TypedRateLimitingQueueConfig[string]{Name: "Sites"}), assignmentAllocators: make(map[string]*assignmentAllocator), loggedNoSiteNodes: make(map[string]struct{}), + sliceDirtySites: make(map[string]struct{}), ready: make(chan struct{}), } @@ -266,46 +274,8 @@ func NewSiteController( AddFunc: func(obj interface{}) { sc.enqueueNode(obj) }, - UpdateFunc: func(old, new interface{}) { - oldNode := old.(*corev1.Node) //nolint:errcheck - newNode := new.(*corev1.Node) //nolint:errcheck - // Re-process if addresses changed, site label changed, WireGuard pubkey changed, or podCIDRs changed - if !nodeAddressesEqual(oldNode, newNode) || - getNodeSiteLabel(oldNode) != getNodeSiteLabel(newNode) || - getNodeAnnotation(oldNode, WireGuardPubKeyAnnotation) != getNodeAnnotation(newNode, WireGuardPubKeyAnnotation) || - !labels.Equals(labels.Set(oldNode.Labels), labels.Set(newNode.Labels)) || - oldNode.Spec.PodCIDR != newNode.Spec.PodCIDR || - !stringSlicesEqual(oldNode.Spec.PodCIDRs, newNode.Spec.PodCIDRs) { - sc.enqueueNode(new) - } - }, - DeleteFunc: func(obj interface{}) { - var node *corev1.Node - - switch t := obj.(type) { - case *corev1.Node: - node = t - case cache.DeletedFinalStateUnknown: - var ok bool - - node, ok = t.Obj.(*corev1.Node) - if !ok { - klog.Errorf("DeletedFinalStateUnknown contained non-Node object: %#v", t.Obj) - sc.enqueueSiteChange() - - return - } - default: - klog.Errorf("Delete event contained non-Node object: %#v", obj) - sc.enqueueSiteChange() - - return - } - - sc.releaseNodeCIDRs(node) - sc.markSlicesDirty() - sc.enqueueSiteChange() - }, + UpdateFunc: sc.handleNodeUpdate, + DeleteFunc: sc.handleNodeDelete, }); err != nil { return nil, fmt.Errorf("failed to add node event handler: %w", err) } @@ -330,13 +300,11 @@ func NewSiteController( // own writes may trigger one redundant pass, which is harmless. if _, err := sliceInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - sc.markSlicesDirty() - }, - UpdateFunc: func(old, new interface{}) { - sc.markSlicesDirty() + sc.markSliceObjectDirty(obj) }, + UpdateFunc: sc.handleSliceUpdate, DeleteFunc: func(obj interface{}) { - sc.markSlicesDirty() + sc.markSliceObjectDirty(obj) }, }); err != nil { return nil, fmt.Errorf("failed to add site node slice event handler: %w", err) @@ -927,6 +895,14 @@ func (sc *SiteController) Run(ctx context.Context, workers int) error { } }, 5*time.Second) + // Orphan detection requires authoritative cluster-wide reads. Keep it off + // the topology event path and run it as a low-frequency repair instead. + go wait.UntilWithContext(ctx, func(ctx context.Context) { + if err := sc.repairOrphanSiteNodeSlices(ctx); err != nil { + klog.Errorf("Failed to repair orphan SiteNodeSlices: %v", err) + } + }, orphanSliceRepairInterval) + sc.readyOnce.Do(func() { close(sc.ready) }) klog.Info("Site controller started") <-ctx.Done() @@ -1012,7 +988,7 @@ func (sc *SiteController) syncNode(ctx context.Context, key string) error { return err } - sc.markSlicesDirty() + sc.markSiteSlicesDirty(currentSite, siteName) return nil } @@ -1043,7 +1019,7 @@ func (sc *SiteController) syncNode(ctx context.Context, key string) error { klog.Infof("Removed site label from node %s", node.Name) } - sc.markSlicesDirty() + sc.markSiteSlicesDirty(currentSite, siteName) } if needsCIDRs { @@ -1061,7 +1037,7 @@ func (sc *SiteController) syncNode(ctx context.Context, key string) error { // includes the node's WireGuard public key and other fields that may // have changed without a label or CIDR change. if siteName != "" { - sc.markSlicesDirty() + sc.markSiteSlicesDirty(currentSite, siteName) } return nil @@ -1083,12 +1059,13 @@ func (sc *SiteController) reconcileAllNodes(_ context.Context) { // updateSiteSlicesIfDirty updates slices after atomically consuming the dirty // flag. A failed pass restores the flag so the periodic loop retries it. func (sc *SiteController) updateSiteSlicesIfDirty(ctx context.Context) error { - if !sc.slicesDirty.CompareAndSwap(true, false) { + work, ok := sc.takeSliceWork() + if !ok { return nil } - if err := sc.updateAllSiteSlices(ctx); err != nil { - sc.markSlicesDirty() + if err := sc.updateSiteSlicesForWork(ctx, work); err != nil { + sc.restoreSliceWork(work) return err } @@ -1098,6 +1075,15 @@ func (sc *SiteController) updateSiteSlicesIfDirty(ctx context.Context) error { // updateAllSiteSlices updates the SiteNodeSlice objects for all sites func (sc *SiteController) updateAllSiteSlices(ctx context.Context) error { + return sc.updateSiteSlicesForWork(ctx, sliceWork{all: true}) +} + +type sliceWork struct { + all bool + sites map[string]struct{} +} + +func (sc *SiteController) updateSiteSlicesForWork(ctx context.Context, work sliceWork) error { // Ensure only one slice update runs at a time to prevent conflicts sc.sliceUpdateLock.Lock() defer sc.sliceUpdateLock.Unlock() @@ -1106,41 +1092,29 @@ func (sc *SiteController) updateAllSiteSlices(ctx context.Context) error { cachedSites := append([]unboundedv1alpha3.Site(nil), sc.sitesCache...) sc.sitesCacheLock.RUnlock() - // Continue converging independent resources, but return every error so the - // periodic loop schedules another pass. - var updateErrors []error - - if err := sc.cleanupOrphanSiteNodeSlices(ctx); err != nil { - updateErrors = append(updateErrors, fmt.Errorf("clean up orphan site node slices: %w", err)) - } - - // Never use an informer-cached Site as a mutation input. Besides protecting - // against stale fields, the UID check prevents a delete/recreate with the - // same name from receiving writes intended for the deleted Site. - sites := make([]unboundedv1alpha3.Site, 0, len(cachedSites)) - - for _, cachedSite := range cachedSites { - liveSite, err := sc.getLiveSiteWithUID(ctx, cachedSite.Name, cachedSite.UID) - if err != nil { - updateErrors = append(updateErrors, fmt.Errorf("site %s: %w", cachedSite.Name, err)) - - continue + sites := cachedSites + if !work.all { + sites = make([]unboundedv1alpha3.Site, 0, len(work.sites)) + for _, site := range cachedSites { + if _, dirty := work.sites[site.Name]; dirty { + sites = append(sites, site) + } } - - sites = append(sites, liveSite) } if len(sites) == 0 { - return errors.Join(updateErrors...) + return nil } nodes, err := sc.nodeLister.List(labels.Everything()) if err != nil { - updateErrors = append(updateErrors, fmt.Errorf("failed to list nodes for slice update: %w", err)) - - return errors.Join(updateErrors...) + return fmt.Errorf("failed to list nodes for slice update: %w", err) } + // Site matching remains global even for targeted output because CIDR ranges + // from every Site participate in deciding which Site owns a Node. + allSites := cachedSites + // Track which nodes belong to each site // Gateway nodes are excluded - they are accessed via GatewayPool.Status.Nodes instead siteNodesInfo := make(map[string][]unboundednetv1alpha1.NodeInfo) @@ -1151,8 +1125,8 @@ func (sc *SiteController) updateAllSiteSlices(ctx context.Context) error { continue } - siteName := sc.findSiteForNode(node, sites) - if siteName != "" { + siteName := sc.findSiteForNode(node, allSites) + if _, dirty := work.sites[siteName]; siteName != "" && (work.all || dirty) { nodeInfo := sc.buildNodeInfo(node) siteNodesInfo[siteName] = append(siteNodesInfo[siteName], nodeInfo) } @@ -1164,12 +1138,16 @@ func (sc *SiteController) updateAllSiteSlices(ctx context.Context) error { allSiteNodeCounts := make(map[string]int) for _, node := range nodes { - siteName := sc.findSiteForNode(node, sites) - if siteName != "" { + siteName := sc.findSiteForNode(node, allSites) + if _, dirty := work.sites[siteName]; siteName != "" && (work.all || dirty) { allSiteNodeCounts[siteName]++ } } + // Continue converging independent resources, but return every error so the + // periodic loop schedules another pass. + var updateErrors []error + for _, site := range sites { nodesInfo := siteNodesInfo[site.Name] // Sort by node name for consistent ordering @@ -1187,10 +1165,12 @@ func (sc *SiteController) updateAllSiteSlices(ctx context.Context) error { for _, site := range sites { nodeCount := allSiteNodeCounts[site.Name] if nodeCount > 0 { - if err := ensureFinalizer(ctx, sc.dynamicClient, siteGVR, site.Name, site.Finalizers, site.UID); err != nil { - updateErrors = append(updateErrors, fmt.Errorf("site %s protection finalizer: %w", site.Name, err)) + if !containsFinalizer(site.Finalizers, ProtectionFinalizer) { + if err := ensureFinalizer(ctx, sc.dynamicClient, siteGVR, site.Name, site.Finalizers, site.UID); err != nil { + updateErrors = append(updateErrors, fmt.Errorf("site %s protection finalizer: %w", site.Name, err)) + } } - } else { + } else if containsFinalizer(site.Finalizers, ProtectionFinalizer) { if err := removeFinalizer(ctx, sc.dynamicClient, siteGVR, site.Name, site.Finalizers, site.UID); err != nil { updateErrors = append(updateErrors, fmt.Errorf("site %s protection finalizer: %w", site.Name, err)) } @@ -1207,13 +1187,6 @@ func (sc *SiteController) updateSiteSlices( nodesInfo []unboundednetv1alpha1.NodeInfo, allNodeCount int, ) error { - var err error - - site, err = sc.getLiveSiteWithUID(ctx, site.Name, site.UID) - if err != nil { - return err - } - // Calculate how many slices we need numSlices := (len(nodesInfo) + unboundednetv1alpha1.MaxNodesPerSlice - 1) / unboundednetv1alpha1.MaxNodesPerSlice if numSlices == 0 { @@ -1455,12 +1428,24 @@ func (sc *SiteController) cleanupOrphanSiteNodeSlices(ctx context.Context) error // source is confirmed healthy and non-empty, so this is a genuine orphan. if err := deleteLiveSiteNodeSlice(ctx, resource, liveSlice); err != nil && !apierrors.IsNotFound(err) { cleanupErrors = append(cleanupErrors, fmt.Errorf("delete orphan slice %s: %w", name, err)) + } else if !found || siteName == "" { + // The malformed object may occupy the deterministic name of a + // valid slice. Rebuild all sites after deleting it because there is + // no trustworthy siteName with which to scope the work. + sc.markSlicesDirty() } } return errors.Join(cleanupErrors...) } +func (sc *SiteController) repairOrphanSiteNodeSlices(ctx context.Context) error { + sc.sliceUpdateLock.Lock() + defer sc.sliceUpdateLock.Unlock() + + return sc.cleanupOrphanSiteNodeSlices(ctx) +} + // legacySiteExists reports whether a pre-migration net-group Site with the given // name still exists. It lets orphan cleanup avoid deleting SiteNodeSlices for // Sites that have not yet been translated into the machina group during the @@ -1517,6 +1502,12 @@ func (sc *SiteController) createOrUpdateSlice(ctx context.Context, site unbounde } resource := sc.dynamicClient.Resource(siteNodeSliceGVR) + + desired := sc.buildSliceObject(site, sliceName, sliceIndex, nodesData) + if cached, ok := sc.cachedSlice(sliceName); ok && sc.sliceMatchesDesired(cached, desired, site) { + return nil + } + backoff := wait.Backoff{ Duration: 100 * time.Millisecond, Factor: 2, @@ -1524,8 +1515,8 @@ func (sc *SiteController) createOrUpdateSlice(ctx context.Context, site unbounde } err := wait.ExponentialBackoffWithContext(ctx, backoff, func(ctx context.Context) (bool, error) { - // Always read directly from the API. The informer may remain stale after - // an AlreadyExists or Conflict response. + // Once cached state differs, use live state for every mutation attempt. + // The informer may remain stale after an AlreadyExists or Conflict. existing, err := resource.Get(ctx, sliceName, metav1.GetOptions{}) if apierrors.IsNotFound(err) { liveSite, siteErr := sc.getLiveSiteWithUID(ctx, site.Name, site.UID) @@ -1534,7 +1525,7 @@ func (sc *SiteController) createOrUpdateSlice(ctx context.Context, site unbounde } // Create new slice - desired := sc.buildSliceObject(liveSite, sliceName, sliceIndex, nodesData) + desired = sc.buildSliceObject(liveSite, sliceName, sliceIndex, nodesData) _, err = resource.Create(ctx, desired, metav1.CreateOptions{}) if err != nil { @@ -1556,26 +1547,16 @@ func (sc *SiteController) createOrUpdateSlice(ctx context.Context, site unbounde return false, fmt.Errorf("failed to get slice %s: %w", sliceName, err) } + if sc.sliceMatchesDesired(existing, desired, site) { + return true, nil + } + liveSite, err := sc.getLiveSiteWithUID(ctx, site.Name, site.UID) if err != nil { return false, err } - desired := sc.buildSliceObject(liveSite, sliceName, sliceIndex, nodesData) - - // Check if update is needed - existingNodes, _, _ := unstructured.NestedSlice(existing.Object, "nodes") //nolint:errcheck - existingNodeCount, foundNodeCount, _ := unstructured.NestedInt64(existing.Object, "nodeCount") //nolint:errcheck - existingSiteName, foundSiteName, _ := unstructured.NestedString(existing.Object, "siteName") //nolint:errcheck - existingSliceIndex, foundSliceIndex, _ := unstructured.NestedInt64(existing.Object, "sliceIndex") //nolint:errcheck - - if sc.sliceNodesEqual(existingNodes, nodesData) && - foundNodeCount && existingNodeCount == int64(len(nodesData)) && - foundSiteName && existingSiteName == liveSite.Name && - foundSliceIndex && existingSliceIndex == int64(sliceIndex) && - hasExactSiteOwnerReference(existing.GetOwnerReferences(), liveSite) { - return true, nil - } + desired = sc.buildSliceObject(liveSite, sliceName, sliceIndex, nodesData) // Mutate a copy of the live object so labels, annotations, finalizers, // and metadata owned by other actors survive this update. @@ -1612,6 +1593,64 @@ func (sc *SiteController) createOrUpdateSlice(ctx context.Context, site unbounde return nil } +func (sc *SiteController) cachedSlice(name string) (*unstructured.Unstructured, bool) { + if sc.sliceInformer == nil { + return nil, false + } + + item, exists, err := sc.sliceInformer.GetStore().GetByKey(name) + if err != nil || !exists { + return nil, false + } + + slice, ok := item.(*unstructured.Unstructured) + + return slice, ok +} + +func (sc *SiteController) sliceMatchesDesired(existing, desired *unstructured.Unstructured, site unboundedv1alpha3.Site) bool { + existingNodes, foundNodes, err := unstructured.NestedSlice(existing.Object, "nodes") + if err != nil || !foundNodes { + return false + } + + desiredNodes, foundDesiredNodes, err := unstructured.NestedSlice(desired.Object, "nodes") + if err != nil || !foundDesiredNodes { + return false + } + + existingNodeCount, foundNodeCount, err := unstructured.NestedInt64(existing.Object, "nodeCount") + if err != nil { + return false + } + + desiredNodeCount, foundDesiredNodeCount, err := unstructured.NestedInt64(desired.Object, "nodeCount") + if err != nil || !foundDesiredNodeCount { + return false + } + + existingSiteName, foundSiteName, err := unstructured.NestedString(existing.Object, "siteName") + if err != nil { + return false + } + + existingSliceIndex, foundSliceIndex, err := unstructured.NestedInt64(existing.Object, "sliceIndex") + if err != nil { + return false + } + + desiredSliceIndex, foundDesiredSliceIndex, err := unstructured.NestedInt64(desired.Object, "sliceIndex") + if err != nil || !foundDesiredSliceIndex { + return false + } + + return sc.sliceNodesEqual(existingNodes, desiredNodes) && + foundNodeCount && existingNodeCount == desiredNodeCount && + foundSiteName && existingSiteName == site.Name && + foundSliceIndex && existingSliceIndex == desiredSliceIndex && + hasExactSiteOwnerReference(existing.GetOwnerReferences(), site) +} + func stringSliceToInterfaceSlice(values []string) []interface{} { result := make([]interface{}, len(values)) for i, value := range values { @@ -1766,6 +1805,10 @@ func (sc *SiteController) updateSiteStatusIfChanged(ctx context.Context, site un SiteNodesGauge.WithLabelValues(site.Name).Set(float64(nodeCount)) SiteNodeSlicesGauge.WithLabelValues(site.Name).Set(float64(sliceCount)) + if site.Status.NodeCount == nodeCount && site.Status.SliceCount == sliceCount { + return nil + } + backoff := wait.Backoff{ Duration: 50 * time.Millisecond, Factor: 2, @@ -2590,6 +2633,150 @@ func (sc *SiteController) validateSiteSnapshot(sites []unboundedv1alpha3.Site) e // markSlicesDirty signals that SiteNodeSlice objects need rebuilding. func (sc *SiteController) markSlicesDirty() { + sc.sliceDirtyLock.Lock() + sc.allSlicesDirty = true + clear(sc.sliceDirtySites) + sc.sliceDirtyLock.Unlock() + sc.slicesDirty.Store(true) +} + +func (sc *SiteController) markSiteSlicesDirty(siteNames ...string) { + sc.sliceDirtyLock.Lock() + + if !sc.allSlicesDirty { + if sc.sliceDirtySites == nil { + sc.sliceDirtySites = make(map[string]struct{}) + } + + for _, siteName := range siteNames { + if siteName != "" { + sc.sliceDirtySites[siteName] = struct{}{} + } + } + } + + hasWork := sc.allSlicesDirty || len(sc.sliceDirtySites) > 0 + sc.sliceDirtyLock.Unlock() + + if hasWork { + sc.slicesDirty.Store(true) + } +} + +func (sc *SiteController) handleNodeUpdate(old, new interface{}) { + oldNode := old.(*corev1.Node) //nolint:errcheck + newNode := new.(*corev1.Node) //nolint:errcheck + // Re-process if addresses changed, site label changed, WireGuard pubkey changed, or podCIDRs changed + if !nodeAddressesEqual(oldNode, newNode) || + getNodeSiteLabel(oldNode) != getNodeSiteLabel(newNode) || + getNodeAnnotation(oldNode, WireGuardPubKeyAnnotation) != getNodeAnnotation(newNode, WireGuardPubKeyAnnotation) || + !labels.Equals(labels.Set(oldNode.Labels), labels.Set(newNode.Labels)) || + oldNode.Spec.PodCIDR != newNode.Spec.PodCIDR || + !stringSlicesEqual(oldNode.Spec.PodCIDRs, newNode.Spec.PodCIDRs) { + sc.markSiteSlicesDirty(append(sc.nodeSliceSites(oldNode), sc.nodeSliceSites(newNode)...)...) + sc.enqueueNode(new) + } +} + +func (sc *SiteController) handleNodeDelete(obj interface{}) { + var node *corev1.Node + + switch t := obj.(type) { + case *corev1.Node: + node = t + case cache.DeletedFinalStateUnknown: + var ok bool + + node, ok = t.Obj.(*corev1.Node) + if !ok { + klog.Errorf("DeletedFinalStateUnknown contained non-Node object: %#v", t.Obj) + sc.enqueueSiteChange() + + return + } + default: + klog.Errorf("Delete event contained non-Node object: %#v", obj) + sc.enqueueSiteChange() + + return + } + + sc.releaseNodeCIDRs(node) + sc.markSiteSlicesDirty(sc.nodeSliceSites(node)...) +} + +// handleSliceUpdate dirties both Sites when a slice's siteName changes, so the +// original Site can repair a slice that the new Site deletes as extra. +func (sc *SiteController) handleSliceUpdate(old, new interface{}) { + sc.markSliceObjectDirty(old) + sc.markSliceObjectDirty(new) +} + +// nodeSliceSites returns the Sites whose slices may include node. Membership +// is IP-derived, while the label may lag or disagree, so both are returned. +func (sc *SiteController) nodeSliceSites(node *corev1.Node) []string { + return []string{getNodeSiteLabel(node), sc.GetSiteForNode(node)} +} + +func (sc *SiteController) markSliceObjectDirty(obj interface{}) { + if tombstone, ok := obj.(cache.DeletedFinalStateUnknown); ok { + obj = tombstone.Obj + } + + slice, ok := obj.(*unstructured.Unstructured) + if !ok { + sc.markSlicesDirty() + + return + } + + siteName, found, err := unstructured.NestedString(slice.Object, "siteName") + if err != nil || !found || siteName == "" { + // A malformed object may occupy a valid slice name, and its deletion + // may be the only event that allows reconstruction. + sc.markSlicesDirty() + + return + } + + sc.markSiteSlicesDirty(siteName) +} + +func (sc *SiteController) takeSliceWork() (sliceWork, bool) { + if !sc.slicesDirty.CompareAndSwap(true, false) { + return sliceWork{}, false + } + + sc.sliceDirtyLock.Lock() + defer sc.sliceDirtyLock.Unlock() + + work := sliceWork{ + all: sc.allSlicesDirty, + sites: sc.sliceDirtySites, + } + sc.allSlicesDirty = false + sc.sliceDirtySites = make(map[string]struct{}) + + return work, work.all || len(work.sites) > 0 +} + +func (sc *SiteController) restoreSliceWork(work sliceWork) { + sc.sliceDirtyLock.Lock() + + if work.all { + sc.allSlicesDirty = true + clear(sc.sliceDirtySites) + } else if !sc.allSlicesDirty { + if sc.sliceDirtySites == nil { + sc.sliceDirtySites = make(map[string]struct{}) + } + + for siteName := range work.sites { + sc.sliceDirtySites[siteName] = struct{}{} + } + } + + sc.sliceDirtyLock.Unlock() sc.slicesDirty.Store(true) } diff --git a/internal/net/controller/site_controller_helpers_test.go b/internal/net/controller/site_controller_helpers_test.go index 7fd82df659..4f0571f342 100644 --- a/internal/net/controller/site_controller_helpers_test.go +++ b/internal/net/controller/site_controller_helpers_test.go @@ -9,12 +9,15 @@ import ( "encoding/json" "errors" "fmt" + "reflect" + "sort" "strings" "testing" "time" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -714,6 +717,193 @@ func TestCreateOrUpdateSliceRepairsOwnerReferenceWithoutNodeChanges(t *testing.T } } +func TestUnchangedCachedSiteSliceReconcileUsesNoLiveAPIReads(t *testing.T) { + site := unboundedv1alpha3.Site{ + ObjectMeta: metav1.ObjectMeta{ + Name: "site-a", + UID: "site-a-uid", + Finalizers: []string{ProtectionFinalizer}, + }, + Spec: unboundedv1alpha3.SiteSpec{NodeCidrs: []string{"10.0.0.0/24"}}, + Status: unboundedv1alpha3.SiteStatus{ + NodeCount: 1, + SliceCount: 1, + }, + } + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node-a", + Annotations: map[string]string{WireGuardPubKeyAnnotation: "key-a"}, + }, + Spec: corev1.NodeSpec{PodCIDRs: []string{"10.244.0.0/24"}}, + Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{ + Type: corev1.NodeInternalIP, + Address: "10.0.0.10", + }}}, + } + + sc := &SiteController{} + slice := sc.buildSliceObject(site, "site-a-0", 0, []interface{}{map[string]interface{}{ + "name": node.Name, + "wireGuardPublicKey": "key-a", + "internalIPs": []interface{}{"10.0.0.10"}, + "podCIDRs": []interface{}{"10.244.0.0/24"}, + }}) + slice.SetResourceVersion("slice-rv") + + sliceStore := cache.NewStore(cache.MetaNamespaceKeyFunc) + if err := sliceStore.Add(slice.DeepCopy()); err != nil { + t.Fatalf("add slice to cache: %v", err) + } + + nodeIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if err := nodeIndexer.Add(node); err != nil { + t.Fatalf("add node to cache: %v", err) + } + + dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds( + runtime.NewScheme(), + map[schema.GroupVersionResource]string{ + siteGVR: "SiteList", + siteNodeSliceGVR: "SiteNodeSliceList", + }, + siteUnstructured(t, site), + slice.DeepCopy(), + ) + sc.dynamicClient = dynamicClient + sc.nodeLister = corev1listers.NewNodeLister(nodeIndexer) + sc.sliceInformer = &fakeInformer{store: sliceStore} + sc.sitesCache = []unboundedv1alpha3.Site{site} + + sc.markSiteSlicesDirty(site.Name) + + if err := sc.updateSiteSlicesIfDirty(context.Background()); err != nil { + t.Fatalf("updateSiteSlicesIfDirty: %v", err) + } + + if actions := dynamicClient.Actions(); len(actions) != 0 { + t.Fatalf("unchanged cache-synchronized reconcile made live API requests: %#v", actions) + } +} + +func TestTargetedSiteSliceReconcileDoesNotReadOtherSite(t *testing.T) { + siteA := unboundedv1alpha3.Site{ + ObjectMeta: metav1.ObjectMeta{Name: "site-a", UID: "site-a-uid", Finalizers: []string{ProtectionFinalizer}}, + Spec: unboundedv1alpha3.SiteSpec{NodeCidrs: []string{"10.0.0.0/24"}}, + Status: unboundedv1alpha3.SiteStatus{NodeCount: 1, SliceCount: 1}, + } + siteB := unboundedv1alpha3.Site{ + ObjectMeta: metav1.ObjectMeta{Name: "site-b", UID: "site-b-uid", Finalizers: []string{ProtectionFinalizer}}, + Spec: unboundedv1alpha3.SiteSpec{NodeCidrs: []string{"10.1.0.0/24"}}, + Status: unboundedv1alpha3.SiteStatus{NodeCount: 1, SliceCount: 1}, + } + + nodeA := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "node-a", Annotations: map[string]string{WireGuardPubKeyAnnotation: "new-key"}}, + Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{ + Type: corev1.NodeInternalIP, Address: "10.0.0.10", + }}}, + } + nodeB := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "node-b"}, + Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{ + Type: corev1.NodeInternalIP, Address: "10.1.0.10", + }}}, + } + + builder := &SiteController{} + sliceA := builder.buildSliceObject(siteA, "site-a-0", 0, []interface{}{map[string]interface{}{ + "name": "node-a", + "wireGuardPublicKey": "old-key", + "internalIPs": []interface{}{"10.0.0.10"}, + }}) + sliceA.SetResourceVersion("a-rv") + + sliceB := builder.buildSliceObject(siteB, "site-b-0", 0, []interface{}{map[string]interface{}{ + "name": "node-b", + "internalIPs": []interface{}{"10.1.0.10"}, + }}) + sliceB.SetResourceVersion("b-rv") + + sliceStore := cache.NewStore(cache.MetaNamespaceKeyFunc) + for _, slice := range []*unstructured.Unstructured{sliceA, sliceB} { + if err := sliceStore.Add(slice.DeepCopy()); err != nil { + t.Fatalf("add slice %s to cache: %v", slice.GetName(), err) + } + } + + nodeIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + for _, node := range []*corev1.Node{nodeA, nodeB} { + if err := nodeIndexer.Add(node); err != nil { + t.Fatalf("add node %s to cache: %v", node.Name, err) + } + } + + dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds( + runtime.NewScheme(), + map[schema.GroupVersionResource]string{ + siteGVR: "SiteList", + siteNodeSliceGVR: "SiteNodeSliceList", + }, + siteUnstructured(t, siteA), + siteUnstructured(t, siteB), + sliceA.DeepCopy(), + sliceB.DeepCopy(), + ) + sc := &SiteController{ + dynamicClient: dynamicClient, + nodeLister: corev1listers.NewNodeLister(nodeIndexer), + sliceInformer: &fakeInformer{store: sliceStore}, + sitesCache: []unboundedv1alpha3.Site{siteA, siteB}, + } + + sc.markSiteSlicesDirty(siteA.Name) + + if err := sc.updateSiteSlicesIfDirty(context.Background()); err != nil { + t.Fatalf("updateSiteSlicesIfDirty: %v", err) + } + + for _, action := range dynamicClient.Actions() { + if action.GetVerb() == "list" { + t.Fatalf("targeted reconcile made cluster-wide list request: %#v", action) + } + + switch typed := action.(type) { + case clienttesting.UpdateAction: + accessor, err := meta.Accessor(typed.GetObject()) + if err != nil { + t.Fatalf("access updated object metadata: %v", err) + } + + if accessor.GetName() == siteB.Name || accessor.GetName() == sliceB.GetName() { + t.Fatalf("targeted reconcile updated site-b: %#v", action) + } + case clienttesting.PatchAction: + if typed.GetName() == siteB.Name { + t.Fatalf("targeted reconcile patched site-b: %#v", action) + } + case clienttesting.GetAction: + if typed.GetName() == siteB.Name || typed.GetName() == sliceB.GetName() { + t.Fatalf("targeted reconcile read site-b: %#v", action) + } + } + } + + gotB, err := dynamicClient.Tracker().Get(siteNodeSliceGVR, "", sliceB.GetName()) + if err != nil { + t.Fatalf("get site-b slice from tracker: %v", err) + } + + accessor, err := meta.Accessor(gotB) + if err != nil { + t.Fatalf("access site-b slice metadata: %v", err) + } + + if accessor.GetResourceVersion() != "b-rv" { + t.Fatalf("site-b slice changed during site-a reconcile") + } +} + func TestCreateOrUpdateSliceGetsLiveObjectAfterConflict(t *testing.T) { site := unboundedv1alpha3.Site{ObjectMeta: metav1.ObjectMeta{Name: "site-a", UID: "current-uid"}} desired := (&SiteController{}).buildSliceObject(site, "site-a-0", 0, []interface{}{map[string]interface{}{"name": "node-a"}}) @@ -1245,6 +1435,195 @@ func TestCleanupOrphanSiteNodeSlicesRevalidatesSitesAndDeletePreconditions(t *te } } +func TestMalformedSliceEventsDirtyAllSites(t *testing.T) { + t.Parallel() + + malformed := &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": unboundednetv1alpha1.GroupVersion.String(), + "kind": "SiteNodeSlice", + "metadata": map[string]interface{}{ + "name": "site-a-0", + }, + "siteName": "", + }} + + tests := []struct { + name string + obj interface{} + }{ + {name: "update", obj: malformed}, + {name: "delete", obj: cache.DeletedFinalStateUnknown{Key: malformed.GetName(), Obj: malformed}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + sc := &SiteController{} + sc.markSliceObjectDirty(tt.obj) + + work, ok := sc.takeSliceWork() + if !ok || !work.all { + t.Fatalf("slice work = %#v, available = %v, want all sites dirty", work, ok) + } + }) + } +} + +func TestCleanupMalformedSliceSchedulesReconstruction(t *testing.T) { + site := unboundedv1alpha3.Site{ + TypeMeta: metav1.TypeMeta{APIVersion: unboundedv1alpha3.GroupVersion.String(), Kind: "Site"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "site-a", + UID: "site-uid", + Finalizers: []string{ProtectionFinalizer}, + }, + Spec: unboundedv1alpha3.SiteSpec{NodeCidrs: []string{"10.0.0.0/24"}}, + } + malformed := (&SiteController{}).buildSliceObject(site, "site-a-0", 0, nil) + malformed.Object["siteName"] = "" + malformed.SetUID("slice-uid") + malformed.SetResourceVersion("slice-rv") + + sliceStore := cache.NewStore(cache.MetaNamespaceKeyFunc) + if err := sliceStore.Add(malformed.DeepCopy()); err != nil { + t.Fatalf("add malformed slice to cache: %v", err) + } + + dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds( + runtime.NewScheme(), + map[schema.GroupVersionResource]string{ + siteGVR: "SiteList", + legacySiteGVR: "SiteList", + siteNodeSliceGVR: "SiteNodeSliceList", + }, + siteUnstructured(t, site), + malformed.DeepCopy(), + ) + + nodeIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if err := nodeIndexer.Add(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "node-a"}, + Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{ + Type: corev1.NodeInternalIP, + Address: "10.0.0.10", + }}}, + }); err != nil { + t.Fatalf("add node: %v", err) + } + + sc := &SiteController{ + dynamicClient: dynamicClient, + nodeLister: corev1listers.NewNodeLister(nodeIndexer), + sliceInformer: &fakeInformer{store: sliceStore}, + sitesCache: []unboundedv1alpha3.Site{site}, + } + + if err := sc.cleanupOrphanSiteNodeSlices(context.Background()); err != nil { + t.Fatalf("cleanup malformed slice: %v", err) + } + + if !sc.slicesDirty.Load() { + t.Fatalf("cleanup did not schedule slice reconstruction") + } + + dynamicClient.ClearActions() + + if err := sc.updateSiteSlicesIfDirty(context.Background()); err != nil { + t.Fatalf("reconstruct slices: %v", err) + } + + reconstructed, err := dynamicClient.Resource(siteNodeSliceGVR).Get(context.Background(), malformed.GetName(), metav1.GetOptions{}) + if err != nil { + t.Fatalf("get reconstructed slice: %v", err) + } + + if got, found, err := unstructured.NestedString(reconstructed.Object, "siteName"); err != nil || !found || got != site.Name { + t.Fatalf("reconstructed siteName = %q, found = %v, err = %v", got, found, err) + } + + nodes, found, err := unstructured.NestedSlice(reconstructed.Object, "nodes") + if err != nil || !found || len(nodes) != 1 { + t.Fatalf("reconstructed nodes = %#v, found = %v, err = %v", nodes, found, err) + } +} + +func TestTopologyReconcileDefersOrphanRepair(t *testing.T) { + site := unboundedv1alpha3.Site{ + ObjectMeta: metav1.ObjectMeta{Name: "site-a", UID: "site-uid"}, + Status: unboundedv1alpha3.SiteStatus{}, + } + orphan := (&SiteController{}).buildSliceObject(site, "orphan-0", 0, nil) + orphan.Object["siteName"] = "" + orphan.SetUID("orphan-uid") + orphan.SetResourceVersion("orphan-rv") + + sliceStore := cache.NewStore(cache.MetaNamespaceKeyFunc) + if err := sliceStore.Add(orphan.DeepCopy()); err != nil { + t.Fatalf("add orphan to cache: %v", err) + } + + dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds( + runtime.NewScheme(), + map[schema.GroupVersionResource]string{ + siteGVR: "SiteList", + legacySiteGVR: "SiteList", + siteNodeSliceGVR: "SiteNodeSliceList", + }, + siteUnstructured(t, site), + orphan.DeepCopy(), + ) + sc := &SiteController{ + dynamicClient: dynamicClient, + nodeLister: corev1listers.NewNodeLister(cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})), + sliceInformer: &fakeInformer{store: sliceStore}, + sitesCache: []unboundedv1alpha3.Site{site}, + } + + sc.markSlicesDirty() + + if err := sc.updateSiteSlicesIfDirty(context.Background()); err != nil { + t.Fatalf("topology reconcile: %v", err) + } + + if _, err := dynamicClient.Tracker().Get(siteNodeSliceGVR, "", orphan.GetName()); err != nil { + t.Fatalf("topology reconcile deleted orphan before periodic repair: %v", err) + } + + for _, action := range dynamicClient.Actions() { + if action.GetVerb() == "list" { + t.Fatalf("topology reconcile performed cluster-wide orphan list: %#v", action) + } + } + + dynamicClient.ClearActions() + + if err := sc.repairOrphanSiteNodeSlices(context.Background()); err != nil { + t.Fatalf("periodic orphan repair: %v", err) + } + + if _, err := dynamicClient.Tracker().Get(siteNodeSliceGVR, "", orphan.GetName()); !apierrors.IsNotFound(err) { + t.Fatalf("periodic orphan repair left orphan present: %v", err) + } + + sawSiteList := false + sawDelete := false + + for _, action := range dynamicClient.Actions() { + if action.GetVerb() == "list" && action.GetResource() == siteGVR { + sawSiteList = true + } + + if action.GetVerb() == "delete" && action.GetResource() == siteNodeSliceGVR { + sawDelete = true + } + } + + if !sawSiteList || !sawDelete { + t.Fatalf("periodic orphan repair actions = %#v, want Site LIST and slice DELETE", dynamicClient.Actions()) + } +} + func TestCleanupOrphanSiteNodeSlicesPreservesSliceOnSiteLookupError(t *testing.T) { site := unboundedv1alpha3.Site{ObjectMeta: metav1.ObjectMeta{Name: "site-a", UID: "site-uid"}} slice := (&SiteController{}).buildSliceObject(site, "site-a-0", 0, nil) @@ -2003,3 +2382,114 @@ func waitForSlicesDirty(t *testing.T, ctx context.Context, sc *SiteController) { t.Fatalf("SiteNodeSlice event did not mark slices dirty: %v", err) } } + +func sliceWorkSites(t *testing.T, sc *SiteController) []string { + t.Helper() + + work, ok := sc.takeSliceWork() + if !ok || work.all { + t.Fatalf("slice work = %#v, available = %v, want targeted Sites", work, ok) + } + + sites := make([]string, 0, len(work.sites)) + for site := range work.sites { + sites = append(sites, site) + } + + sort.Strings(sites) + + return sites +} + +func newIPMatchedSliceTestController() *SiteController { + return &SiteController{ + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + sitesCache: []unboundedv1alpha3.Site{ + {ObjectMeta: metav1.ObjectMeta{Name: "site-a"}, Spec: unboundedv1alpha3.SiteSpec{NodeCidrs: []string{"10.1.0.0/16"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "site-b"}, Spec: unboundedv1alpha3.SiteSpec{NodeCidrs: []string{"10.2.0.0/16"}}}, + }, + pendingPodCIDRs: make(map[string]*pendingPodCIDRAssignment), + } +} + +func sliceTestNode(ip string, labels map[string]string) *corev1.Node { + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-1", UID: "node-uid", Labels: labels}} + if ip != "" { + node.Status.Addresses = []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: ip}} + } + + return node +} + +func TestNodeUpdateDirtiesIPMatchedSites(t *testing.T) { + t.Parallel() + + sc := newIPMatchedSliceTestController() + defer sc.workqueue.ShutDown() + + sc.handleNodeUpdate(sliceTestNode("10.1.0.5", nil), sliceTestNode("10.2.0.5", nil)) + + if got, want := sliceWorkSites(t, sc), []string{"site-a", "site-b"}; !reflect.DeepEqual(got, want) { + t.Fatalf("dirty Sites = %v, want %v", got, want) + } +} + +func TestNodeUpdateLosingAddressDirtiesPreviousSite(t *testing.T) { + t.Parallel() + + sc := newIPMatchedSliceTestController() + defer sc.workqueue.ShutDown() + + sc.handleNodeUpdate(sliceTestNode("10.1.0.5", nil), sliceTestNode("", nil)) + + if got, want := sliceWorkSites(t, sc), []string{"site-a"}; !reflect.DeepEqual(got, want) { + t.Fatalf("dirty Sites = %v, want %v", got, want) + } +} + +func TestNodeDeleteDirtiesIPMatchedAndLabeledSites(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + obj interface{} + }{ + {name: "delete", obj: sliceTestNode("10.1.0.5", map[string]string{canonicalSiteLabelKey: "site-b"})}, + {name: "tombstone", obj: cache.DeletedFinalStateUnknown{Key: "node-1", Obj: sliceTestNode("10.1.0.5", map[string]string{canonicalSiteLabelKey: "site-b"})}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + sc := newIPMatchedSliceTestController() + defer sc.workqueue.ShutDown() + + sc.handleNodeDelete(tt.obj) + + if got, want := sliceWorkSites(t, sc), []string{"site-a", "site-b"}; !reflect.DeepEqual(got, want) { + t.Fatalf("dirty Sites = %v, want %v", got, want) + } + }) + } +} + +func TestSliceSiteNameChangeDirtiesBothSites(t *testing.T) { + t.Parallel() + + slice := func(siteName string) *unstructured.Unstructured { + return &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": unboundednetv1alpha1.GroupVersion.String(), + "kind": "SiteNodeSlice", + "metadata": map[string]interface{}{"name": "site-a-0"}, + "siteName": siteName, + }} + } + + sc := &SiteController{} + sc.handleSliceUpdate(slice("site-a"), slice("site-b")) + + if got, want := sliceWorkSites(t, sc), []string{"site-a", "site-b"}; !reflect.DeepEqual(got, want) { + t.Fatalf("dirty Sites = %v, want %v", got, want) + } +}