Merge pull request #21 from bnema/fix/zero-downtime-deployment
fix(deploy): preserve tracked container during orphan cleanup + add install script
3 files · +242 -3
A install.sh +74 -0
| @@ -0,0 +1,74 @@ | |||
| 1 | + | #!/bin/sh | |
| 2 | + | set -e | |
| 3 | + | ||
| 4 | + | # Gordon installer script | |
| 5 | + | # Usage: curl -fsSL https://gordon.bnema.dev/install | bash | |
| 6 | + | ||
| 7 | + | REPO="bnema/gordon" | |
| 8 | + | INSTALL_DIR="/usr/local/bin" | |
| 9 | + | ||
| 10 | + | echo "Installing Gordon..." | |
| 11 | + | ||
| 12 | + | # Detect OS | |
| 13 | + | OS=$(uname -s | tr '[:upper:]' '[:lower:]') | |
| 14 | + | case "$OS" in | |
| 15 | + | linux) | |
| 16 | + | OS="linux" | |
| 17 | + | ;; | |
| 18 | + | darwin) | |
| 19 | + | OS="darwin" | |
| 20 | + | ;; | |
| 21 | + | *) | |
| 22 | + | echo "Error: Unsupported operating system: $OS" | |
| 23 | + | exit 1 | |
| 24 | + | ;; | |
| 25 | + | esac | |
| 26 | + | ||
| 27 | + | # Detect architecture | |
| 28 | + | ARCH=$(uname -m) | |
| 29 | + | case "$ARCH" in | |
| 30 | + | x86_64|amd64) | |
| 31 | + | ARCH="amd64" | |
| 32 | + | ;; | |
| 33 | + | aarch64|arm64) | |
| 34 | + | ARCH="arm64" | |
| 35 | + | ;; | |
| 36 | + | *) | |
| 37 | + | echo "Error: Unsupported architecture: $ARCH" | |
| 38 | + | exit 1 | |
| 39 | + | ;; | |
| 40 | + | esac | |
| 41 | + | ||
| 42 | + | echo "Detected: ${OS}/${ARCH}" | |
| 43 | + | ||
| 44 | + | # Construct download URL | |
| 45 | + | TARBALL="gordon_${OS}_${ARCH}.tar.gz" | |
| 46 | + | DOWNLOAD_URL="https://github.com/${REPO}/releases/latest/download/${TARBALL}" | |
| 47 | + | ||
| 48 | + | # Create temporary directory | |
| 49 | + | TMP_DIR=$(mktemp -d) | |
| 50 | + | trap 'rm -rf "$TMP_DIR"' EXIT | |
| 51 | + | ||
| 52 | + | echo "Downloading ${TARBALL}..." | |
| 53 | + | curl -fsSL "$DOWNLOAD_URL" -o "$TMP_DIR/$TARBALL" | |
| 54 | + | ||
| 55 | + | echo "Extracting..." | |
| 56 | + | tar -xzf "$TMP_DIR/$TARBALL" -C "$TMP_DIR" | |
| 57 | + | ||
| 58 | + | echo "Installing to ${INSTALL_DIR}..." | |
| 59 | + | if [ -w "$INSTALL_DIR" ]; then | |
| 60 | + | mv "$TMP_DIR/gordon" "$INSTALL_DIR/gordon" | |
| 61 | + | else | |
| 62 | + | echo "sudo required to install to ${INSTALL_DIR}" | |
| 63 | + | sudo mv "$TMP_DIR/gordon" "$INSTALL_DIR/gordon" | |
| 64 | + | fi | |
| 65 | + | ||
| 66 | + | # Verify installation | |
| 67 | + | if command -v gordon >/dev/null 2>&1; then | |
| 68 | + | echo "" | |
| 69 | + | echo "Gordon installed successfully!" | |
| 70 | + | gordon version | |
| 71 | + | else | |
| 72 | + | echo "" | |
| 73 | + | echo "Installation complete. You may need to add ${INSTALL_DIR} to your PATH." | |
| 74 | + | fi | |
| @@ -0,0 +1,74 @@ | |||||
| 1 | + | #!/bin/sh | |||
| 2 | + | set -e | |||
| 3 | + | ||||
| 4 | + | # Gordon installer script | |||
| 5 | + | # Usage: curl -fsSL https://gordon.bnema.dev/install | bash | |||
| 6 | + | ||||
| 7 | + | REPO="bnema/gordon" | |||
| 8 | + | INSTALL_DIR="/usr/local/bin" | |||
| 9 | + | ||||
| 10 | + | echo "Installing Gordon..." | |||
| 11 | + | ||||
| 12 | + | # Detect OS | |||
| 13 | + | OS=$(uname -s | tr '[:upper:]' '[:lower:]') | |||
| 14 | + | case "$OS" in | |||
| 15 | + | linux) | |||
| 16 | + | OS="linux" | |||
| 17 | + | ;; | |||
| 18 | + | darwin) | |||
| 19 | + | OS="darwin" | |||
| 20 | + | ;; | |||
| 21 | + | *) | |||
| 22 | + | echo "Error: Unsupported operating system: $OS" | |||
| 23 | + | exit 1 | |||
| 24 | + | ;; | |||
| 25 | + | esac | |||
| 26 | + | ||||
| 27 | + | # Detect architecture | |||
| 28 | + | ARCH=$(uname -m) | |||
| 29 | + | case "$ARCH" in | |||
| 30 | + | x86_64|amd64) | |||
| 31 | + | ARCH="amd64" | |||
| 32 | + | ;; | |||
| 33 | + | aarch64|arm64) | |||
| 34 | + | ARCH="arm64" | |||
| 35 | + | ;; | |||
| 36 | + | *) | |||
| 37 | + | echo "Error: Unsupported architecture: $ARCH" | |||
| 38 | + | exit 1 | |||
| 39 | + | ;; | |||
| 40 | + | esac | |||
| 41 | + | ||||
| 42 | + | echo "Detected: ${OS}/${ARCH}" | |||
| 43 | + | ||||
| 44 | + | # Construct download URL | |||
| 45 | + | TARBALL="gordon_${OS}_${ARCH}.tar.gz" | |||
| 46 | + | DOWNLOAD_URL="https://github.com/${REPO}/releases/latest/download/${TARBALL}" | |||
| 47 | + | ||||
| 48 | + | # Create temporary directory | |||
| 49 | + | TMP_DIR=$(mktemp -d) | |||
| 50 | + | trap 'rm -rf "$TMP_DIR"' EXIT | |||
| 51 | + | ||||
| 52 | + | echo "Downloading ${TARBALL}..." | |||
| 53 | + | curl -fsSL "$DOWNLOAD_URL" -o "$TMP_DIR/$TARBALL" | |||
| 54 | + | ||||
| 55 | + | echo "Extracting..." | |||
| 56 | + | tar -xzf "$TMP_DIR/$TARBALL" -C "$TMP_DIR" | |||
| 57 | + | ||||
| 58 | + | echo "Installing to ${INSTALL_DIR}..." | |||
| 59 | + | if [ -w "$INSTALL_DIR" ]; then | |||
| 60 | + | mv "$TMP_DIR/gordon" "$INSTALL_DIR/gordon" | |||
| 61 | + | else | |||
| 62 | + | echo "sudo required to install to ${INSTALL_DIR}" | |||
| 63 | + | sudo mv "$TMP_DIR/gordon" "$INSTALL_DIR/gordon" | |||
| 64 | + | fi | |||
| 65 | + | ||||
| 66 | + | # Verify installation | |||
| 67 | + | if command -v gordon >/dev/null 2>&1; then | |||
| 68 | + | echo "" | |||
| 69 | + | echo "Gordon installed successfully!" | |||
| 70 | + | gordon version | |||
| 71 | + | else | |||
| 72 | + | echo "" | |||
| 73 | + | echo "Installation complete. You may need to add ${INSTALL_DIR} to your PATH." | |||
| 74 | + | fi | |||
M internal/usecase/container/service.go +8 -3
| @@ -84,7 +84,12 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||
| 84 | 84 | s.mu.RUnlock() | |
| 85 | 85 | ||
| 86 | 86 | // Clean up orphaned containers (containers with same name but not tracked) | |
| 87 | - | if err := s.cleanupOrphanedContainers(ctx, route.Domain); err != nil { | |
| 87 | + | // Skip the currently tracked container to preserve zero-downtime deployment | |
| 88 | + | existingID := "" | |
| 89 | + | if hasExisting { | |
| 90 | + | existingID = existing.ID | |
| 91 | + | } | |
| 92 | + | if err := s.cleanupOrphanedContainers(ctx, route.Domain, existingID); err != nil { | |
| 88 | 93 | log.WrapErr(err, "failed to cleanup orphaned containers") | |
| 89 | 94 | } | |
| 90 | 95 | ||
| @@ -738,7 +743,7 @@ func (s *Service) createNetworkIfNeeded(ctx context.Context, networkName string) | |||
| 738 | 743 | return nil | |
| 739 | 744 | } | |
| 740 | 745 | ||
| 741 | - | func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName string) error { | |
| 746 | + | func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName string, skipContainerID string) error { | |
| 742 | 747 | log := zerowrap.FromCtx(ctx) | |
| 743 | 748 | expectedName := fmt.Sprintf("gordon-%s", domainName) | |
| 744 | 749 | ||
| @@ -748,7 +753,7 @@ func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName stri | |||
| 748 | 753 | } | |
| 749 | 754 | ||
| 750 | 755 | for _, c := range allContainers { | |
| 751 | - | if c.Name == expectedName { | |
| 756 | + | if c.Name == expectedName && c.ID != skipContainerID { | |
| 752 | 757 | log.Info().Str(zerowrap.FieldEntityID, c.ID).Str(zerowrap.FieldStatus, c.Status).Msg("found orphaned container, removing") | |
| 753 | 758 | ||
| 754 | 759 | if err := s.runtime.StopContainer(ctx, c.ID); err != nil { | |
| @@ -84,7 +84,12 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||||
| 84 | s.mu.RUnlock() | 84 | s.mu.RUnlock() | ||
| 85 | 85 | ||||
| 86 | // Clean up orphaned containers (containers with same name but not tracked) | 86 | // Clean up orphaned containers (containers with same name but not tracked) | ||
| 87 | - | if err := s.cleanupOrphanedContainers(ctx, route.Domain); err != nil { | 87 | + | // Skip the currently tracked container to preserve zero-downtime deployment |
| 88 | + | existingID := "" | |||
| 89 | + | if hasExisting { | |||
| 90 | + | existingID = existing.ID | |||
| 91 | + | } | |||
| 92 | + | if err := s.cleanupOrphanedContainers(ctx, route.Domain, existingID); err != nil { | |||
| 88 | log.WrapErr(err, "failed to cleanup orphaned containers") | 93 | log.WrapErr(err, "failed to cleanup orphaned containers") | ||
| 89 | } | 94 | } | ||
| 90 | 95 | ||||
| @@ -738,7 +743,7 @@ func (s *Service) createNetworkIfNeeded(ctx context.Context, networkName string) | |||||
| 738 | return nil | 743 | return nil | ||
| 739 | } | 744 | } | ||
| 740 | 745 | ||||
| 741 | - | func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName string) error { | 746 | + | func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName string, skipContainerID string) error { |
| 742 | log := zerowrap.FromCtx(ctx) | 747 | log := zerowrap.FromCtx(ctx) | ||
| 743 | expectedName := fmt.Sprintf("gordon-%s", domainName) | 748 | expectedName := fmt.Sprintf("gordon-%s", domainName) | ||
| 744 | 749 | ||||
| @@ -748,7 +753,7 @@ func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName stri | |||||
| 748 | } | 753 | } | ||
| 749 | 754 | ||||
| 750 | for _, c := range allContainers { | 755 | for _, c := range allContainers { | ||
| 751 | - | if c.Name == expectedName { | 756 | + | if c.Name == expectedName && c.ID != skipContainerID { |
| 752 | log.Info().Str(zerowrap.FieldEntityID, c.ID).Str(zerowrap.FieldStatus, c.Status).Msg("found orphaned container, removing") | 757 | log.Info().Str(zerowrap.FieldEntityID, c.ID).Str(zerowrap.FieldStatus, c.Status).Msg("found orphaned container, removing") | ||
| 753 | 758 | ||||
| 754 | if err := s.runtime.StopContainer(ctx, c.ID); err != nil { | 759 | if err := s.runtime.StopContainer(ctx, c.ID); err != nil { | ||
M internal/usecase/container/service_test.go +160 -0
| @@ -895,3 +895,163 @@ func TestService_AutoStart_HandlesDeployErrors(t *testing.T) { | |||
| 895 | 895 | assert.Error(t, err) | |
| 896 | 896 | assert.Contains(t, err.Error(), "auto-start completed with 1 errors") | |
| 897 | 897 | } | |
| 898 | + | ||
| 899 | + | // TestService_Deploy_OrphanCleanupSkipsTrackedContainer verifies that the orphan cleanup | |
| 900 | + | // does not remove the currently tracked container during zero-downtime deployment. | |
| 901 | + | // This is critical for preventing downtime - the old container must stay running | |
| 902 | + | // until the new container is ready and traffic is switched. | |
| 903 | + | func TestService_Deploy_OrphanCleanupSkipsTrackedContainer(t *testing.T) { | |
| 904 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 905 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 906 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 907 | + | ||
| 908 | + | config := Config{ | |
| 909 | + | ReadinessDelay: 0, // No delay for tests | |
| 910 | + | } | |
| 911 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 912 | + | ctx := testContext() | |
| 913 | + | ||
| 914 | + | // Pre-populate with existing tracked container | |
| 915 | + | existingContainer := &domain.Container{ | |
| 916 | + | ID: "tracked-container-123", | |
| 917 | + | Name: "gordon-test.example.com", | |
| 918 | + | Status: "running", | |
| 919 | + | } | |
| 920 | + | svc.containers["test.example.com"] = existingContainer | |
| 921 | + | ||
| 922 | + | route := domain.Route{ | |
| 923 | + | Domain: "test.example.com", | |
| 924 | + | Image: "myapp:v2", | |
| 925 | + | } | |
| 926 | + | ||
| 927 | + | // ListContainers returns the tracked container - this simulates the orphan check | |
| 928 | + | // finding a container with the same name. The bug was that it would remove this | |
| 929 | + | // container BEFORE the new one was ready, causing downtime. | |
| 930 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 931 | + | { | |
| 932 | + | ID: "tracked-container-123", | |
| 933 | + | Name: "gordon-test.example.com", | |
| 934 | + | Status: "running", | |
| 935 | + | }, | |
| 936 | + | }, nil) | |
| 937 | + | ||
| 938 | + | // Image operations | |
| 939 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil) | |
| 940 | + | runtime.EXPECT().PullImage(mock.Anything, "myapp:v2").Return(nil) | |
| 941 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v2").Return([]int{8080}, nil) | |
| 942 | + | ||
| 943 | + | // Environment | |
| 944 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |
| 945 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v2").Return([]string{}, nil) | |
| 946 | + | ||
| 947 | + | // Create new container with -new suffix for zero-downtime | |
| 948 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |
| 949 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool { | |
| 950 | + | return cfg.Name == "gordon-test.example.com-new" | |
| 951 | + | })).Return(newContainer, nil) | |
| 952 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |
| 953 | + | ||
| 954 | + | // Wait for ready | |
| 955 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |
| 956 | + | ||
| 957 | + | // Inspect after ready | |
| 958 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |
| 959 | + | ID: "new-container", | |
| 960 | + | Status: "running", | |
| 961 | + | }, nil) | |
| 962 | + | ||
| 963 | + | // Publish event | |
| 964 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |
| 965 | + | ||
| 966 | + | // NOW (after new container is ready) the old container should be stopped and removed | |
| 967 | + | // This is the correct zero-downtime sequence - not during orphan cleanup | |
| 968 | + | runtime.EXPECT().StopContainer(mock.Anything, "tracked-container-123").Return(nil) | |
| 969 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "tracked-container-123", true).Return(nil) | |
| 970 | + | ||
| 971 | + | // Rename new container to canonical name | |
| 972 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |
| 973 | + | ||
| 974 | + | result, err := svc.Deploy(ctx, route) | |
| 975 | + | ||
| 976 | + | assert.NoError(t, err) | |
| 977 | + | assert.Equal(t, "new-container", result.ID) | |
| 978 | + | ||
| 979 | + | // Verify new container is now tracked | |
| 980 | + | tracked, exists := svc.Get(ctx, "test.example.com") | |
| 981 | + | assert.True(t, exists) | |
| 982 | + | assert.Equal(t, "new-container", tracked.ID) | |
| 983 | + | } | |
| 984 | + | ||
| 985 | + | // TestService_Deploy_OrphanCleanupRemovesTrueOrphans verifies that containers | |
| 986 | + | // with the same name but NOT tracked are properly removed as orphans. | |
| 987 | + | func TestService_Deploy_OrphanCleanupRemovesTrueOrphans(t *testing.T) { | |
| 988 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 989 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 990 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 991 | + | ||
| 992 | + | config := Config{ | |
| 993 | + | ReadinessDelay: 0, // No delay for tests | |
| 994 | + | } | |
| 995 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 996 | + | ctx := testContext() | |
| 997 | + | ||
| 998 | + | // NO tracked container - service is empty for this domain | |
| 999 | + | ||
| 1000 | + | route := domain.Route{ | |
| 1001 | + | Domain: "test.example.com", | |
| 1002 | + | Image: "myapp:v1", | |
| 1003 | + | } | |
| 1004 | + | ||
| 1005 | + | // ListContainers returns an orphaned container (same name, but not tracked) | |
| 1006 | + | // This could happen if Gordon crashed and restarted, or container was created manually | |
| 1007 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 1008 | + | { | |
| 1009 | + | ID: "orphan-container", | |
| 1010 | + | Name: "gordon-test.example.com", | |
| 1011 | + | Status: "running", | |
| 1012 | + | }, | |
| 1013 | + | }, nil) | |
| 1014 | + | ||
| 1015 | + | // Orphan should be stopped and removed BEFORE we proceed | |
| 1016 | + | runtime.EXPECT().StopContainer(mock.Anything, "orphan-container").Return(nil) | |
| 1017 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "orphan-container", true).Return(nil) | |
| 1018 | + | ||
| 1019 | + | // Image operations | |
| 1020 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil) | |
| 1021 | + | runtime.EXPECT().PullImage(mock.Anything, "myapp:v1").Return(nil) | |
| 1022 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v1").Return([]int{8080}, nil) | |
| 1023 | + | ||
| 1024 | + | // Environment | |
| 1025 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |
| 1026 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v1").Return([]string{}, nil) | |
| 1027 | + | ||
| 1028 | + | // Create container (no -new suffix since no existing tracked container) | |
| 1029 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com", Status: "created"} | |
| 1030 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool { | |
| 1031 | + | return cfg.Name == "gordon-test.example.com" | |
| 1032 | + | })).Return(newContainer, nil) | |
| 1033 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |
| 1034 | + | ||
| 1035 | + | // Wait for ready | |
| 1036 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |
| 1037 | + | ||
| 1038 | + | // Inspect after ready | |
| 1039 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |
| 1040 | + | ID: "new-container", | |
| 1041 | + | Status: "running", | |
| 1042 | + | Ports: []int{8080}, | |
| 1043 | + | }, nil) | |
| 1044 | + | ||
| 1045 | + | // Publish event | |
| 1046 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |
| 1047 | + | ||
| 1048 | + | result, err := svc.Deploy(ctx, route) | |
| 1049 | + | ||
| 1050 | + | assert.NoError(t, err) | |
| 1051 | + | assert.Equal(t, "new-container", result.ID) | |
| 1052 | + | ||
| 1053 | + | // Verify new container is tracked | |
| 1054 | + | tracked, exists := svc.Get(ctx, "test.example.com") | |
| 1055 | + | assert.True(t, exists) | |
| 1056 | + | assert.Equal(t, "new-container", tracked.ID) | |
| 1057 | + | } | |
| @@ -895,3 +895,163 @@ func TestService_AutoStart_HandlesDeployErrors(t *testing.T) { | |||||
| 895 | assert.Error(t, err) | 895 | assert.Error(t, err) | ||
| 896 | assert.Contains(t, err.Error(), "auto-start completed with 1 errors") | 896 | assert.Contains(t, err.Error(), "auto-start completed with 1 errors") | ||
| 897 | } | 897 | } | ||
| 898 | + | ||||
| 899 | + | // TestService_Deploy_OrphanCleanupSkipsTrackedContainer verifies that the orphan cleanup | |||
| 900 | + | // does not remove the currently tracked container during zero-downtime deployment. | |||
| 901 | + | // This is critical for preventing downtime - the old container must stay running | |||
| 902 | + | // until the new container is ready and traffic is switched. | |||
| 903 | + | func TestService_Deploy_OrphanCleanupSkipsTrackedContainer(t *testing.T) { | |||
| 904 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 905 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 906 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 907 | + | ||||
| 908 | + | config := Config{ | |||
| 909 | + | ReadinessDelay: 0, // No delay for tests | |||
| 910 | + | } | |||
| 911 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 912 | + | ctx := testContext() | |||
| 913 | + | ||||
| 914 | + | // Pre-populate with existing tracked container | |||
| 915 | + | existingContainer := &domain.Container{ | |||
| 916 | + | ID: "tracked-container-123", | |||
| 917 | + | Name: "gordon-test.example.com", | |||
| 918 | + | Status: "running", | |||
| 919 | + | } | |||
| 920 | + | svc.containers["test.example.com"] = existingContainer | |||
| 921 | + | ||||
| 922 | + | route := domain.Route{ | |||
| 923 | + | Domain: "test.example.com", | |||
| 924 | + | Image: "myapp:v2", | |||
| 925 | + | } | |||
| 926 | + | ||||
| 927 | + | // ListContainers returns the tracked container - this simulates the orphan check | |||
| 928 | + | // finding a container with the same name. The bug was that it would remove this | |||
| 929 | + | // container BEFORE the new one was ready, causing downtime. | |||
| 930 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |||
| 931 | + | { | |||
| 932 | + | ID: "tracked-container-123", | |||
| 933 | + | Name: "gordon-test.example.com", | |||
| 934 | + | Status: "running", | |||
| 935 | + | }, | |||
| 936 | + | }, nil) | |||
| 937 | + | ||||
| 938 | + | // Image operations | |||
| 939 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil) | |||
| 940 | + | runtime.EXPECT().PullImage(mock.Anything, "myapp:v2").Return(nil) | |||
| 941 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v2").Return([]int{8080}, nil) | |||
| 942 | + | ||||
| 943 | + | // Environment | |||
| 944 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |||
| 945 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v2").Return([]string{}, nil) | |||
| 946 | + | ||||
| 947 | + | // Create new container with -new suffix for zero-downtime | |||
| 948 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |||
| 949 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool { | |||
| 950 | + | return cfg.Name == "gordon-test.example.com-new" | |||
| 951 | + | })).Return(newContainer, nil) | |||
| 952 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |||
| 953 | + | ||||
| 954 | + | // Wait for ready | |||
| 955 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |||
| 956 | + | ||||
| 957 | + | // Inspect after ready | |||
| 958 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |||
| 959 | + | ID: "new-container", | |||
| 960 | + | Status: "running", | |||
| 961 | + | }, nil) | |||
| 962 | + | ||||
| 963 | + | // Publish event | |||
| 964 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |||
| 965 | + | ||||
| 966 | + | // NOW (after new container is ready) the old container should be stopped and removed | |||
| 967 | + | // This is the correct zero-downtime sequence - not during orphan cleanup | |||
| 968 | + | runtime.EXPECT().StopContainer(mock.Anything, "tracked-container-123").Return(nil) | |||
| 969 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "tracked-container-123", true).Return(nil) | |||
| 970 | + | ||||
| 971 | + | // Rename new container to canonical name | |||
| 972 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |||
| 973 | + | ||||
| 974 | + | result, err := svc.Deploy(ctx, route) | |||
| 975 | + | ||||
| 976 | + | assert.NoError(t, err) | |||
| 977 | + | assert.Equal(t, "new-container", result.ID) | |||
| 978 | + | ||||
| 979 | + | // Verify new container is now tracked | |||
| 980 | + | tracked, exists := svc.Get(ctx, "test.example.com") | |||
| 981 | + | assert.True(t, exists) | |||
| 982 | + | assert.Equal(t, "new-container", tracked.ID) | |||
| 983 | + | } | |||
| 984 | + | ||||
| 985 | + | // TestService_Deploy_OrphanCleanupRemovesTrueOrphans verifies that containers | |||
| 986 | + | // with the same name but NOT tracked are properly removed as orphans. | |||
| 987 | + | func TestService_Deploy_OrphanCleanupRemovesTrueOrphans(t *testing.T) { | |||
| 988 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 989 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 990 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 991 | + | ||||
| 992 | + | config := Config{ | |||
| 993 | + | ReadinessDelay: 0, // No delay for tests | |||
| 994 | + | } | |||
| 995 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 996 | + | ctx := testContext() | |||
| 997 | + | ||||
| 998 | + | // NO tracked container - service is empty for this domain | |||
| 999 | + | ||||
| 1000 | + | route := domain.Route{ | |||
| 1001 | + | Domain: "test.example.com", | |||
| 1002 | + | Image: "myapp:v1", | |||
| 1003 | + | } | |||
| 1004 | + | ||||
| 1005 | + | // ListContainers returns an orphaned container (same name, but not tracked) | |||
| 1006 | + | // This could happen if Gordon crashed and restarted, or container was created manually | |||
| 1007 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |||
| 1008 | + | { | |||
| 1009 | + | ID: "orphan-container", | |||
| 1010 | + | Name: "gordon-test.example.com", | |||
| 1011 | + | Status: "running", | |||
| 1012 | + | }, | |||
| 1013 | + | }, nil) | |||
| 1014 | + | ||||
| 1015 | + | // Orphan should be stopped and removed BEFORE we proceed | |||
| 1016 | + | runtime.EXPECT().StopContainer(mock.Anything, "orphan-container").Return(nil) | |||
| 1017 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "orphan-container", true).Return(nil) | |||
| 1018 | + | ||||
| 1019 | + | // Image operations | |||
| 1020 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil) | |||
| 1021 | + | runtime.EXPECT().PullImage(mock.Anything, "myapp:v1").Return(nil) | |||
| 1022 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v1").Return([]int{8080}, nil) | |||
| 1023 | + | ||||
| 1024 | + | // Environment | |||
| 1025 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |||
| 1026 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v1").Return([]string{}, nil) | |||
| 1027 | + | ||||
| 1028 | + | // Create container (no -new suffix since no existing tracked container) | |||
| 1029 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com", Status: "created"} | |||
| 1030 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool { | |||
| 1031 | + | return cfg.Name == "gordon-test.example.com" | |||
| 1032 | + | })).Return(newContainer, nil) | |||
| 1033 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |||
| 1034 | + | ||||
| 1035 | + | // Wait for ready | |||
| 1036 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |||
| 1037 | + | ||||
| 1038 | + | // Inspect after ready | |||
| 1039 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |||
| 1040 | + | ID: "new-container", | |||
| 1041 | + | Status: "running", | |||
| 1042 | + | Ports: []int{8080}, | |||
| 1043 | + | }, nil) | |||
| 1044 | + | ||||
| 1045 | + | // Publish event | |||
| 1046 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |||
| 1047 | + | ||||
| 1048 | + | result, err := svc.Deploy(ctx, route) | |||
| 1049 | + | ||||
| 1050 | + | assert.NoError(t, err) | |||
| 1051 | + | assert.Equal(t, "new-container", result.ID) | |||
| 1052 | + | ||||
| 1053 | + | // Verify new container is tracked | |||
| 1054 | + | tracked, exists := svc.Get(ctx, "test.example.com") | |||
| 1055 | + | assert.True(t, exists) | |||
| 1056 | + | assert.Equal(t, "new-container", tracked.ID) | |||
| 1057 | + | } | |||