fix(preview): propagate cleanup errors and defer container removal
cleanupOrphanDomainResources now returns an error. If resource cleanup fails, the container is not removed — it stays stopped and will be rediscovered on the next GC scan for retry.
- commit
- 8b31c1c17e38858a9e1d35417b97b4456432d20d
- author
- brice · · 2026-03-25 08:00 UTC
- parent
- b32e7e8
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
1 file · +19 -4
M internal/app/run.go +19 -4
| @@ -652,7 +652,11 @@ func gcOrphanedPreviews(ctx context.Context, svc *services, previewConfig domain | |||
| 652 | 652 | } | |
| 653 | 653 | ||
| 654 | 654 | if orphanDomain != "" { | |
| 655 | - | cleanupOrphanDomainResources(ctx, svc, orphanDomain) | |
| 655 | + | if err := cleanupOrphanDomainResources(ctx, svc, orphanDomain); err != nil { | |
| 656 | + | log.Warn().Err(err).Str("container", c.Name).Str("domain", orphanDomain). | |
| 657 | + | Msg("orphan resource cleanup failed, deferring container removal to next scan") | |
| 658 | + | continue | |
| 659 | + | } | |
| 656 | 660 | } | |
| 657 | 661 | ||
| 658 | 662 | if err := svc.runtime.RemoveContainer(ctx, c.Name, true); err != nil { | |
| @@ -663,20 +667,27 @@ func gcOrphanedPreviews(ctx context.Context, svc *services, previewConfig domain | |||
| 663 | 667 | } | |
| 664 | 668 | } | |
| 665 | 669 | ||
| 666 | - | // cleanupOrphanDomainResources removes volumes, network, and route for an orphaned preview domain. | |
| 667 | - | func cleanupOrphanDomainResources(ctx context.Context, svc *services, orphanDomain string) { | |
| 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. | |
| 673 | + | func cleanupOrphanDomainResources(ctx context.Context, svc *services, orphanDomain string) error { | |
| 668 | 674 | log := zerowrap.FromCtx(ctx) | |
| 669 | 675 | domainSanitized := strings.ReplaceAll(orphanDomain, ".", "-") | |
| 676 | + | var cleanupErr error | |
| 670 | 677 | ||
| 671 | 678 | // Remove volumes matching domain prefix (convention: {volPrefix}-{domain-sanitized}-). | |
| 672 | 679 | _, volPrefix, _ := svc.configSvc.GetVolumeConfig() | |
| 673 | 680 | fullVolumePrefix := volPrefix + "-" + domainSanitized + "-" | |
| 674 | 681 | volumes, err := svc.runtime.ListVolumes(ctx) | |
| 675 | - | if err == nil { | |
| 682 | + | if err != nil { | |
| 683 | + | log.Warn().Err(err).Msg("failed to list volumes for orphan cleanup") | |
| 684 | + | cleanupErr = err | |
| 685 | + | } else { | |
| 676 | 686 | for _, v := range volumes { | |
| 677 | 687 | if strings.HasPrefix(v.Name, fullVolumePrefix) { | |
| 678 | 688 | if err := svc.runtime.RemoveVolume(ctx, v.Name, true); err != nil { | |
| 679 | 689 | log.Warn().Err(err).Str("volume", v.Name).Msg("failed to remove orphan volume") | |
| 690 | + | cleanupErr = err | |
| 680 | 691 | } | |
| 681 | 692 | } | |
| 682 | 693 | } | |
| @@ -687,15 +698,19 @@ func cleanupOrphanDomainResources(ctx context.Context, svc *services, orphanDoma | |||
| 687 | 698 | networkName := networkPrefix + "-" + domainSanitized | |
| 688 | 699 | if err := svc.runtime.RemoveNetwork(ctx, networkName); err != nil { | |
| 689 | 700 | log.Warn().Err(err).Str("network", networkName).Msg("failed to remove orphan network") | |
| 701 | + | cleanupErr = err | |
| 690 | 702 | } | |
| 691 | 703 | ||
| 692 | 704 | // Remove route and invalidate proxy. | |
| 693 | 705 | if err := svc.configSvc.RemoveRoute(ctx, orphanDomain); err != nil { | |
| 694 | 706 | if !errors.Is(err, domain.ErrRouteNotFound) { | |
| 695 | 707 | log.Warn().Err(err).Str("domain", orphanDomain).Msg("failed to remove orphan route") | |
| 708 | + | cleanupErr = err | |
| 696 | 709 | } | |
| 697 | 710 | } | |
| 698 | 711 | svc.proxySvc.InvalidateTarget(ctx, orphanDomain) | |
| 712 | + | ||
| 713 | + | return cleanupErr | |
| 699 | 714 | } | |
| 700 | 715 | ||
| 701 | 716 | // injectTelemetryMetrics creates and injects OTel metrics into services when | |
| @@ -652,7 +652,11 @@ func gcOrphanedPreviews(ctx context.Context, svc *services, previewConfig domain | |||||
| 652 | } | 652 | } | ||
| 653 | 653 | ||||
| 654 | if orphanDomain != "" { | 654 | if orphanDomain != "" { | ||
| 655 | - | cleanupOrphanDomainResources(ctx, svc, orphanDomain) | 655 | + | if err := cleanupOrphanDomainResources(ctx, svc, orphanDomain); err != nil { |
| 656 | + | log.Warn().Err(err).Str("container", c.Name).Str("domain", orphanDomain). | |||
| 657 | + | Msg("orphan resource cleanup failed, deferring container removal to next scan") | |||
| 658 | + | continue | |||
| 659 | + | } | |||
| 656 | } | 660 | } | ||
| 657 | 661 | ||||
| 658 | if err := svc.runtime.RemoveContainer(ctx, c.Name, true); err != nil { | 662 | if err := svc.runtime.RemoveContainer(ctx, c.Name, true); err != nil { | ||
| @@ -663,20 +667,27 @@ func gcOrphanedPreviews(ctx context.Context, svc *services, previewConfig domain | |||||
| 663 | } | 667 | } | ||
| 664 | } | 668 | } | ||
| 665 | 669 | ||||
| 666 | - | // cleanupOrphanDomainResources removes volumes, network, and route for an orphaned preview domain. | 670 | + | // cleanupOrphanDomainResources removes volumes, network, and route for an |
| 667 | - | func cleanupOrphanDomainResources(ctx context.Context, svc *services, orphanDomain string) { | 671 | + | // orphaned preview domain. Returns an error if any step fails so the caller |
| 672 | + | // can defer container removal until the next scan. | |||
| 673 | + | func cleanupOrphanDomainResources(ctx context.Context, svc *services, orphanDomain string) error { | |||
| 668 | log := zerowrap.FromCtx(ctx) | 674 | log := zerowrap.FromCtx(ctx) | ||
| 669 | domainSanitized := strings.ReplaceAll(orphanDomain, ".", "-") | 675 | domainSanitized := strings.ReplaceAll(orphanDomain, ".", "-") | ||
| 676 | + | var cleanupErr error | |||
| 670 | 677 | ||||
| 671 | // Remove volumes matching domain prefix (convention: {volPrefix}-{domain-sanitized}-). | 678 | // Remove volumes matching domain prefix (convention: {volPrefix}-{domain-sanitized}-). | ||
| 672 | _, volPrefix, _ := svc.configSvc.GetVolumeConfig() | 679 | _, volPrefix, _ := svc.configSvc.GetVolumeConfig() | ||
| 673 | fullVolumePrefix := volPrefix + "-" + domainSanitized + "-" | 680 | fullVolumePrefix := volPrefix + "-" + domainSanitized + "-" | ||
| 674 | volumes, err := svc.runtime.ListVolumes(ctx) | 681 | volumes, err := svc.runtime.ListVolumes(ctx) | ||
| 675 | - | if err == nil { | 682 | + | if err != nil { |
| 683 | + | log.Warn().Err(err).Msg("failed to list volumes for orphan cleanup") | |||
| 684 | + | cleanupErr = err | |||
| 685 | + | } else { | |||
| 676 | for _, v := range volumes { | 686 | for _, v := range volumes { | ||
| 677 | if strings.HasPrefix(v.Name, fullVolumePrefix) { | 687 | if strings.HasPrefix(v.Name, fullVolumePrefix) { | ||
| 678 | if err := svc.runtime.RemoveVolume(ctx, v.Name, true); err != nil { | 688 | if err := svc.runtime.RemoveVolume(ctx, v.Name, true); err != nil { | ||
| 679 | log.Warn().Err(err).Str("volume", v.Name).Msg("failed to remove orphan volume") | 689 | log.Warn().Err(err).Str("volume", v.Name).Msg("failed to remove orphan volume") | ||
| 690 | + | cleanupErr = err | |||
| 680 | } | 691 | } | ||
| 681 | } | 692 | } | ||
| 682 | } | 693 | } | ||
| @@ -687,15 +698,19 @@ func cleanupOrphanDomainResources(ctx context.Context, svc *services, orphanDoma | |||||
| 687 | networkName := networkPrefix + "-" + domainSanitized | 698 | networkName := networkPrefix + "-" + domainSanitized | ||
| 688 | if err := svc.runtime.RemoveNetwork(ctx, networkName); err != nil { | 699 | if err := svc.runtime.RemoveNetwork(ctx, networkName); err != nil { | ||
| 689 | log.Warn().Err(err).Str("network", networkName).Msg("failed to remove orphan network") | 700 | log.Warn().Err(err).Str("network", networkName).Msg("failed to remove orphan network") | ||
| 701 | + | cleanupErr = err | |||
| 690 | } | 702 | } | ||
| 691 | 703 | ||||
| 692 | // Remove route and invalidate proxy. | 704 | // Remove route and invalidate proxy. | ||
| 693 | if err := svc.configSvc.RemoveRoute(ctx, orphanDomain); err != nil { | 705 | if err := svc.configSvc.RemoveRoute(ctx, orphanDomain); err != nil { | ||
| 694 | if !errors.Is(err, domain.ErrRouteNotFound) { | 706 | if !errors.Is(err, domain.ErrRouteNotFound) { | ||
| 695 | log.Warn().Err(err).Str("domain", orphanDomain).Msg("failed to remove orphan route") | 707 | log.Warn().Err(err).Str("domain", orphanDomain).Msg("failed to remove orphan route") | ||
| 708 | + | cleanupErr = err | |||
| 696 | } | 709 | } | ||
| 697 | } | 710 | } | ||
| 698 | svc.proxySvc.InvalidateTarget(ctx, orphanDomain) | 711 | svc.proxySvc.InvalidateTarget(ctx, orphanDomain) | ||
| 712 | + | ||||
| 713 | + | return cleanupErr | |||
| 699 | } | 714 | } | ||
| 700 | 715 | ||||
| 701 | // injectTelemetryMetrics creates and injects OTel metrics into services when | 716 | // injectTelemetryMetrics creates and injects OTel metrics into services when | ||