fix(container) correct readiness error message and reduce waitForReady complexity
- commit
- 198c8789ab5abbccd88ce7f7dd4308c5fc7fc479
- author
- bnema · · 2026-02-06 20:43 UTC
- parent
- 84879b9
- signature
- unsigned
1 file · +33 -23
M internal/usecase/container/service.go +33 -23
| @@ -1087,7 +1087,7 @@ func (s *Service) pullImageWithRetry(ctx context.Context, pullFn func(context.Co | |||
| 1087 | 1087 | return nil | |
| 1088 | 1088 | } | |
| 1089 | 1089 | if !isInternal || !isConnectionRefusedError(err) || attempt == internalPullMaxAttempts { | |
| 1090 | - | return err | |
| 1090 | + | return fmt.Errorf("internal image pull failed after %d attempts: %w", attempt, err) | |
| 1091 | 1091 | } | |
| 1092 | 1092 | ||
| 1093 | 1093 | backoff := time.Duration(attempt) * time.Second | |
| @@ -1764,30 +1764,14 @@ func (s *Service) findContainerByName(ctx context.Context, name string) *domain. | |||
| 1764 | 1764 | func (s *Service) waitForReady(ctx context.Context, containerID string) error { | |
| 1765 | 1765 | log := zerowrap.FromCtx(ctx) | |
| 1766 | 1766 | ||
| 1767 | - | // Poll for container to be running (max 30 seconds) | |
| 1768 | - | for i := 0; i < 30; i++ { | |
| 1769 | - | running, err := s.runtime.IsContainerRunning(ctx, containerID) | |
| 1770 | - | if err != nil { | |
| 1771 | - | return err | |
| 1772 | - | } | |
| 1773 | - | if running { | |
| 1774 | - | break | |
| 1775 | - | } | |
| 1776 | - | if i == 29 { | |
| 1777 | - | return fmt.Errorf("container did not start within 30 seconds") | |
| 1778 | - | } | |
| 1779 | - | select { | |
| 1780 | - | case <-time.After(time.Second): | |
| 1781 | - | case <-ctx.Done(): | |
| 1782 | - | return ctx.Err() | |
| 1783 | - | } | |
| 1767 | + | if err := s.pollContainerRunning(ctx, containerID); err != nil { | |
| 1768 | + | return err | |
| 1784 | 1769 | } | |
| 1785 | 1770 | ||
| 1786 | 1771 | s.mu.RLock() | |
| 1787 | 1772 | cfg := s.config | |
| 1788 | 1773 | s.mu.RUnlock() | |
| 1789 | 1774 | ||
| 1790 | - | // Additional readiness delay (configurable, default 5 seconds) | |
| 1791 | 1775 | delay := cfg.ReadinessDelay | |
| 1792 | 1776 | if delay == 0 { | |
| 1793 | 1777 | delay = 5 * time.Second | |
| @@ -1809,8 +1793,34 @@ func (s *Service) waitForReady(ctx context.Context, containerID string) error { | |||
| 1809 | 1793 | return nil | |
| 1810 | 1794 | } | |
| 1811 | 1795 | ||
| 1812 | - | // Be tolerant to short startup flaps: retry within a bounded recovery window | |
| 1813 | - | // before treating readiness as failed. | |
| 1796 | + | return s.waitForRecovery(ctx, containerID) | |
| 1797 | + | } | |
| 1798 | + | ||
| 1799 | + | // pollContainerRunning polls until the container is running (max 30 seconds). | |
| 1800 | + | func (s *Service) pollContainerRunning(ctx context.Context, containerID string) error { | |
| 1801 | + | for i := 0; i < 30; i++ { | |
| 1802 | + | running, err := s.runtime.IsContainerRunning(ctx, containerID) | |
| 1803 | + | if err != nil { | |
| 1804 | + | return err | |
| 1805 | + | } | |
| 1806 | + | if running { | |
| 1807 | + | return nil | |
| 1808 | + | } | |
| 1809 | + | if i == 29 { | |
| 1810 | + | return fmt.Errorf("container did not start within 30 seconds") | |
| 1811 | + | } | |
| 1812 | + | select { | |
| 1813 | + | case <-time.After(time.Second): | |
| 1814 | + | case <-ctx.Done(): | |
| 1815 | + | return ctx.Err() | |
| 1816 | + | } | |
| 1817 | + | } | |
| 1818 | + | return nil | |
| 1819 | + | } | |
| 1820 | + | ||
| 1821 | + | // waitForRecovery waits for a container to recover within the recovery window. | |
| 1822 | + | func (s *Service) waitForRecovery(ctx context.Context, containerID string) error { | |
| 1823 | + | log := zerowrap.FromCtx(ctx) | |
| 1814 | 1824 | log.Warn(). | |
| 1815 | 1825 | Dur("recovery_window", readinessRecoveryWindow). | |
| 1816 | 1826 | Msg("container not running after readiness delay, waiting for recovery") | |
| @@ -1823,7 +1833,7 @@ func (s *Service) waitForReady(ctx context.Context, containerID string) error { | |||
| 1823 | 1833 | return ctx.Err() | |
| 1824 | 1834 | } | |
| 1825 | 1835 | ||
| 1826 | - | running, err = s.runtime.IsContainerRunning(ctx, containerID) | |
| 1836 | + | running, err := s.runtime.IsContainerRunning(ctx, containerID) | |
| 1827 | 1837 | if err != nil { | |
| 1828 | 1838 | return err | |
| 1829 | 1839 | } | |
| @@ -1833,7 +1843,7 @@ func (s *Service) waitForReady(ctx context.Context, containerID string) error { | |||
| 1833 | 1843 | } | |
| 1834 | 1844 | } | |
| 1835 | 1845 | ||
| 1836 | - | return fmt.Errorf("container stopped during readiness delay") | |
| 1846 | + | return fmt.Errorf("container not running after readiness delay and recovery window") | |
| 1837 | 1847 | } | |
| 1838 | 1848 | ||
| 1839 | 1849 | // publishContainerDeployed publishes a container.deployed event. | |
| @@ -1087,7 +1087,7 @@ func (s *Service) pullImageWithRetry(ctx context.Context, pullFn func(context.Co | |||||
| 1087 | return nil | 1087 | return nil | ||
| 1088 | } | 1088 | } | ||
| 1089 | if !isInternal || !isConnectionRefusedError(err) || attempt == internalPullMaxAttempts { | 1089 | if !isInternal || !isConnectionRefusedError(err) || attempt == internalPullMaxAttempts { | ||
| 1090 | - | return err | 1090 | + | return fmt.Errorf("internal image pull failed after %d attempts: %w", attempt, err) |
| 1091 | } | 1091 | } | ||
| 1092 | 1092 | ||||
| 1093 | backoff := time.Duration(attempt) * time.Second | 1093 | backoff := time.Duration(attempt) * time.Second | ||
| @@ -1764,30 +1764,14 @@ func (s *Service) findContainerByName(ctx context.Context, name string) *domain. | |||||
| 1764 | func (s *Service) waitForReady(ctx context.Context, containerID string) error { | 1764 | func (s *Service) waitForReady(ctx context.Context, containerID string) error { | ||
| 1765 | log := zerowrap.FromCtx(ctx) | 1765 | log := zerowrap.FromCtx(ctx) | ||
| 1766 | 1766 | ||||
| 1767 | - | // Poll for container to be running (max 30 seconds) | 1767 | + | if err := s.pollContainerRunning(ctx, containerID); err != nil { |
| 1768 | - | for i := 0; i < 30; i++ { | 1768 | + | return err |
| 1769 | - | running, err := s.runtime.IsContainerRunning(ctx, containerID) | |||
| 1770 | - | if err != nil { | |||
| 1771 | - | return err | |||
| 1772 | - | } | |||
| 1773 | - | if running { | |||
| 1774 | - | break | |||
| 1775 | - | } | |||
| 1776 | - | if i == 29 { | |||
| 1777 | - | return fmt.Errorf("container did not start within 30 seconds") | |||
| 1778 | - | } | |||
| 1779 | - | select { | |||
| 1780 | - | case <-time.After(time.Second): | |||
| 1781 | - | case <-ctx.Done(): | |||
| 1782 | - | return ctx.Err() | |||
| 1783 | - | } | |||
| 1784 | } | 1769 | } | ||
| 1785 | 1770 | ||||
| 1786 | s.mu.RLock() | 1771 | s.mu.RLock() | ||
| 1787 | cfg := s.config | 1772 | cfg := s.config | ||
| 1788 | s.mu.RUnlock() | 1773 | s.mu.RUnlock() | ||
| 1789 | 1774 | ||||
| 1790 | - | // Additional readiness delay (configurable, default 5 seconds) | |||
| 1791 | delay := cfg.ReadinessDelay | 1775 | delay := cfg.ReadinessDelay | ||
| 1792 | if delay == 0 { | 1776 | if delay == 0 { | ||
| 1793 | delay = 5 * time.Second | 1777 | delay = 5 * time.Second | ||
| @@ -1809,8 +1793,34 @@ func (s *Service) waitForReady(ctx context.Context, containerID string) error { | |||||
| 1809 | return nil | 1793 | return nil | ||
| 1810 | } | 1794 | } | ||
| 1811 | 1795 | ||||
| 1812 | - | // Be tolerant to short startup flaps: retry within a bounded recovery window | 1796 | + | return s.waitForRecovery(ctx, containerID) |
| 1813 | - | // before treating readiness as failed. | 1797 | + | } |
| 1798 | + | ||||
| 1799 | + | // pollContainerRunning polls until the container is running (max 30 seconds). | |||
| 1800 | + | func (s *Service) pollContainerRunning(ctx context.Context, containerID string) error { | |||
| 1801 | + | for i := 0; i < 30; i++ { | |||
| 1802 | + | running, err := s.runtime.IsContainerRunning(ctx, containerID) | |||
| 1803 | + | if err != nil { | |||
| 1804 | + | return err | |||
| 1805 | + | } | |||
| 1806 | + | if running { | |||
| 1807 | + | return nil | |||
| 1808 | + | } | |||
| 1809 | + | if i == 29 { | |||
| 1810 | + | return fmt.Errorf("container did not start within 30 seconds") | |||
| 1811 | + | } | |||
| 1812 | + | select { | |||
| 1813 | + | case <-time.After(time.Second): | |||
| 1814 | + | case <-ctx.Done(): | |||
| 1815 | + | return ctx.Err() | |||
| 1816 | + | } | |||
| 1817 | + | } | |||
| 1818 | + | return nil | |||
| 1819 | + | } | |||
| 1820 | + | ||||
| 1821 | + | // waitForRecovery waits for a container to recover within the recovery window. | |||
| 1822 | + | func (s *Service) waitForRecovery(ctx context.Context, containerID string) error { | |||
| 1823 | + | log := zerowrap.FromCtx(ctx) | |||
| 1814 | log.Warn(). | 1824 | log.Warn(). | ||
| 1815 | Dur("recovery_window", readinessRecoveryWindow). | 1825 | Dur("recovery_window", readinessRecoveryWindow). | ||
| 1816 | Msg("container not running after readiness delay, waiting for recovery") | 1826 | Msg("container not running after readiness delay, waiting for recovery") | ||
| @@ -1823,7 +1833,7 @@ func (s *Service) waitForReady(ctx context.Context, containerID string) error { | |||||
| 1823 | return ctx.Err() | 1833 | return ctx.Err() | ||
| 1824 | } | 1834 | } | ||
| 1825 | 1835 | ||||
| 1826 | - | running, err = s.runtime.IsContainerRunning(ctx, containerID) | 1836 | + | running, err := s.runtime.IsContainerRunning(ctx, containerID) |
| 1827 | if err != nil { | 1837 | if err != nil { | ||
| 1828 | return err | 1838 | return err | ||
| 1829 | } | 1839 | } | ||
| @@ -1833,7 +1843,7 @@ func (s *Service) waitForReady(ctx context.Context, containerID string) error { | |||||
| 1833 | } | 1843 | } | ||
| 1834 | } | 1844 | } | ||
| 1835 | 1845 | ||||
| 1836 | - | return fmt.Errorf("container stopped during readiness delay") | 1846 | + | return fmt.Errorf("container not running after readiness delay and recovery window") |
| 1837 | } | 1847 | } | ||
| 1838 | 1848 | ||||
| 1839 | // publishContainerDeployed publishes a container.deployed event. | 1849 | // publishContainerDeployed publishes a container.deployed event. | ||