fix: ignore already deleted in postgres service delete#260
Conversation
Adds a condition to ignore NotFound errors in `PostgresService.Delete`, which happens when the service has already been deleted.
📝 WalkthroughWalkthroughThe PostgresService.Delete method in Docker Swarm orchestration now gracefully handles the case where a service is not found during scale-down operations, treating it as already deleted and returning nil instead of propagating the error. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/internal/orchestrator/swarm/postgres_service.go (1)
169-171: Handle ErrNotFound on ServiceRemove to avoid race failures.A concurrent delete between scale-down and remove can surface an error, which breaks the idempotent delete behavior. Docker's
ServiceRemove()returnsErrNotFoundwhen the service is already gone; this should be treated as success, mirroring the existing pattern forServiceScale(line 162–164).Suggested patch
- if err := client.ServiceRemove(ctx, s.ServiceName); err != nil { - return fmt.Errorf("failed to remove postgres service: %w", err) - } + if err := client.ServiceRemove(ctx, s.ServiceName); err != nil { + if errors.Is(err, docker.ErrNotFound) { + return nil + } + return fmt.Errorf("failed to remove postgres service: %w", err) + }
Summary
Adds a condition to ignore NotFound errors in
PostgresService.Delete, which happens when the service has already been deleted.Testing
host-2host-2control plane service and the postgres service for the database withdocker service rm <name or id>cp-req remove-host --force host-2Prior to this change, we'd get an error in the last step from trying to scale down a service that no longer exists.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.