Merge pull request #149 from bnema/fix/preview-orphan-gc-network

fix(preview): move network removal after container removal

commit: 511705f7166173244d352d7a1816130e5aff71c9
author: bnema · 7mo ago · 2026-03-25 18:34 UTC
committer: GitHub · 7mo ago · 2026-03-25 18:34 UTC
merge of: 7568a1c + a7406bc · diff against the first parent
signature: GPG UnverifiedB5690EEEBB952194 The key is unknown
refs: tag: v2.30.0-pre6

1 file · +21 -6

M internal/app/run.go +21 -6


@@ -645,8 +645,9 @@ func gcOrphanedPreviews(ctx context.Context, svc *services, previewConfig domain
645	645		log.Warn().Str("container", c.Name).Str("image", c.Image).Str("domain", orphanDomain).
646	646		Time("created", c.Created).Msg("orphaned preview container detected, cleaning up")
647	647
648		-	// Stop container first, then clean up domain resources while container
649		-	// still exists (so future scans can rediscover it on failure), then remove.
	648	+	// Order: stop → remove volumes/route → remove container → remove network.
	649	+	// Network removal must happen after container removal because the runtime
	650	+	// refuses to delete a network that still has containers attached.
650	651		if err := svc.runtime.StopContainer(ctx, c.Name); err != nil {
651	652		log.Warn().Err(err).Str("container", c.Name).Msg("failed to stop orphan container")
652	653		}

@@ -663,20 +664,34 @@ func gcOrphanedPreviews(ctx context.Context, svc *services, previewConfig domain
663	664		log.Warn().Err(err).Str("container", c.Name).Msg("failed to remove orphan container")
664	665		}
665	666
	667	+	// Remove network after container is gone.
	668	+	if orphanDomain != "" {
	669	+	if err := removeOrphanNetwork(ctx, svc, strings.ReplaceAll(orphanDomain, ".", "-"), log); err != nil {
	670	+	log.Warn().Err(err).Str("container", c.Name).Msg("failed to remove orphan network after container removal")
	671	+	}
	672	+	}
	673	+
666	674		log.Info().Str("container", c.Name).Str("domain", orphanDomain).Msg("orphaned preview container cleaned up")
667	675		}
	676	+
	677	+	// Re-sync container state so stale routes disappear from routes list.
	678	+	if len(orphans) > 0 {
	679	+	if err := svc.containerSvc.SyncContainers(ctx); err != nil {
	680	+	log.Warn().Err(err).Msg("failed to sync containers after orphan GC")
	681	+	}
	682	+	}
668	683		}
669	684
670		-	// cleanupOrphanDomainResources removes volumes, network, and route for an
671		-	// orphaned preview domain. Returns an error if any step fails so the caller
672		-	// can defer container removal until the next scan.
	685	+	// cleanupOrphanDomainResources removes volumes and route for an orphaned
	686	+	// preview domain. Network removal is handled separately in gcOrphanedPreviews
	687	+	// after the container is removed. Returns an error if any step fails so the
	688	+	// caller can defer container removal until the next scan.
673	689		func cleanupOrphanDomainResources(ctx context.Context, svc *services, orphanDomain string) error {
674	690		log := zerowrap.FromCtx(ctx)
675	691		domainSanitized := strings.ReplaceAll(orphanDomain, ".", "-")
676	692		var errs []error
677	693
678	694		errs = append(errs, removeOrphanVolumes(ctx, svc, domainSanitized, log)...)
679		-	errs = append(errs, removeOrphanNetwork(ctx, svc, domainSanitized, log))
680	695		errs = append(errs, removeOrphanRoute(ctx, svc, orphanDomain, log))
681	696		svc.proxySvc.InvalidateTarget(ctx, orphanDomain)
682	697

 		log.Warn().Str("container", c.Name).Str("image", c.Image).Str("domain", orphanDomain).
 			Time("created", c.Created).Msg("orphaned preview container detected, cleaning up")
-		// Stop container first, then clean up domain resources while container
+		// Order: stop → remove volumes/route → remove container → remove network.
-		// still exists (so future scans can rediscover it on failure), then remove.
+		// Network removal must happen after container removal because the runtime
+		// refuses to delete a network that still has containers attached.
 		if err := svc.runtime.StopContainer(ctx, c.Name); err != nil {
 			log.Warn().Err(err).Str("container", c.Name).Msg("failed to stop orphan container")
 		}
 			log.Warn().Err(err).Str("container", c.Name).Msg("failed to remove orphan container")
 		}
+		// Remove network after container is gone.
+		if orphanDomain != "" {
+			if err := removeOrphanNetwork(ctx, svc, strings.ReplaceAll(orphanDomain, ".", "-"), log); err != nil {
+				log.Warn().Err(err).Str("container", c.Name).Msg("failed to remove orphan network after container removal")
+			}
+		}
 		log.Info().Str("container", c.Name).Str("domain", orphanDomain).Msg("orphaned preview container cleaned up")
 	}
+	// Re-sync container state so stale routes disappear from routes list.
+	if len(orphans) > 0 {
+		if err := svc.containerSvc.SyncContainers(ctx); err != nil {
+			log.Warn().Err(err).Msg("failed to sync containers after orphan GC")
+		}
+	}
 }
-// cleanupOrphanDomainResources removes volumes, network, and route for an
+// cleanupOrphanDomainResources removes volumes and route for an orphaned
-// orphaned preview domain. Returns an error if any step fails so the caller
+// preview domain. Network removal is handled separately in gcOrphanedPreviews
-// can defer container removal until the next scan.
+// after the container is removed. Returns an error if any step fails so the
+// caller can defer container removal until the next scan.
 func cleanupOrphanDomainResources(ctx context.Context, svc *services, orphanDomain string) error {
 	log := zerowrap.FromCtx(ctx)
 	domainSanitized := strings.ReplaceAll(orphanDomain, ".", "-")
 	var errs []error
 	errs = append(errs, removeOrphanVolumes(ctx, svc, domainSanitized, log)...)
-	errs = append(errs, removeOrphanNetwork(ctx, svc, domainSanitized, log))
 	errs = append(errs, removeOrphanRoute(ctx, svc, orphanDomain, log))
 	svc.proxySvc.InvalidateTarget(ctx, orphanDomain)