[CLD-5461] Add fix so that when force is used a release can run in pa…

…rallel with another one in progress forcing its way (#21) * CLD-5461 Add fix so that when force is used a release can run in parallel with another one in progress forcing its way * CLD-5461 Fix tests and add test case for force true --------- Co-authored-by: Stylianos Rigas <[email protected]>
mattermost · Jun 28, 2023 · 4e2c6e7 · 4e2c6e7
1 parent 63b4a70
commit 4e2c6e7
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 26 deletions.
diff --git a/internal/supervisor/ring.go b/internal/supervisor/ring.go
@@ -234,37 +234,47 @@ func (s *RingSupervisor) releaseRing(ring *model.Ring, logger log.FieldLogger) s
 }
 
 func (s *RingSupervisor) checkRingReleasePending(ring *model.Ring, logger log.FieldLogger) string {
-	logger.Debug("Checking if other Rings are locked...")
+	logger.Debugf("Checking if pending ring release should be forced...")
 
-	ringsLocked, err := s.store.GetRingsLocked()
+	release, err := s.store.GetRingRelease(ring.DesiredReleaseID)
 	if err != nil {
-		logger.WithError(err).Error("Failed to query for rings that are under lock")
+		logger.WithError(err).Error("Failed to get the ring release for the ring pending work")
 		return model.RingStateReleaseFailed
 	}
 
-	ringsReleaseInProgress, err := s.store.GetRingsReleaseInProgress()
-	if err != nil {
-		logger.WithError(err).Error("Failed to query for rings that are under release")
-		return model.RingStateReleaseFailed
-	}
+	if !release.Force {
+		logger.Debug("Checking if other Rings are locked...")
 
-	//The total rings locked at this time will be at least 1
-	if len(ringsLocked) > 1 || len(ringsReleaseInProgress) > 0 {
-		logger.Debug("Another ring is under lock and being updated...")
-		return model.InstallationGroupReleasePending
-	}
+		ringsLocked, err := s.store.GetRingsLocked()
+		if err != nil {
+			logger.WithError(err).Error("Failed to query for rings that are under lock")
+			return model.RingStateReleaseFailed
+		}
 
-	logger.Debugf("Checking ring %s prioritization", ring.ID)
-	rings, err := s.store.GetUnlockedRingsPendingWork()
-	if err != nil {
-		logger.WithError(err).Error("Failed to get rings pending work for prioritization check")
-		return model.RingStateReleaseFailed
-	}
+		ringsReleaseInProgress, err := s.store.GetRingsReleaseInProgress()
+		if err != nil {
+			logger.WithError(err).Error("Failed to query for rings that are under release")
+			return model.RingStateReleaseFailed
+		}
+
+		//The total rings locked at this time will be at least 1
+		if len(ringsLocked) > 1 || len(ringsReleaseInProgress) > 0 {
+			logger.Debug("Another ring is under lock and being updated...")
+			return model.InstallationGroupReleasePending
+		}
+
+		logger.Debugf("Checking ring %s prioritization", ring.ID)
+		rings, err := s.store.GetUnlockedRingsPendingWork()
+		if err != nil {
+			logger.WithError(err).Error("Failed to get rings pending work for prioritization check")
+			return model.RingStateReleaseFailed
+		}
 
-	for _, rg := range rings {
-		if rg.Priority < ring.Priority {
-			logger.Debugf("Ring %s is in priority", rg.ID)
-			return model.RingStateReleasePending
+		for _, rg := range rings {
+			if rg.Priority < ring.Priority {
+				logger.Debugf("Ring %s is in priority", rg.ID)
+				return model.RingStateReleasePending
+			}
 		}
 	}
 

diff --git a/internal/supervisor/ring_test.go b/internal/supervisor/ring_test.go
@@ -6,6 +6,7 @@ package supervisor_test
 
 import (
 	"testing"
+	"time"
 
 	"github.com/mattermost/elrond/internal/store"
 	"github.com/mattermost/elrond/internal/supervisor"
@@ -174,17 +175,62 @@ func TestRingSupervisorSupervise(t *testing.T) {
 			sqlStore := store.MakeTestSQLStore(t, logger)
 			supervisor := supervisor.NewRingSupervisor(sqlStore, &mockRingProvisioner{}, "instanceID", logger)
 
+			release, err := sqlStore.GetOrCreateRingRelease(&model.RingRelease{
+				Version:      "test-version",
+				Image:        "test-image",
+				Force:        false,
+				EnvVariables: nil,
+				CreateAt:     time.Now().UnixNano(),
+			})
+			require.NoError(t, err)
+
+			Ring := &model.Ring{
+				State:            tc.InitialState,
+				ActiveReleaseID:  release.ID,
+				DesiredReleaseID: release.ID,
+			}
+
+			installationGroup := model.InstallationGroup{
+				Name:  "group1",
+				State: model.InstallationGroupStable,
+			}
+
+			err = sqlStore.CreateRing(Ring, &installationGroup)
+			require.NoError(t, err)
+
+			supervisor.Supervise(Ring)
+
+			Ring, err = sqlStore.GetRing(Ring.ID)
+			require.NoError(t, err)
+			require.Equal(t, tc.ExpectedState, Ring.State)
+		})
+
+		t.Run(tc.Description, func(t *testing.T) {
+			logger := testlib.MakeLogger(t)
+			sqlStore := store.MakeTestSQLStore(t, logger)
+			supervisor := supervisor.NewRingSupervisor(sqlStore, &mockRingProvisioner{}, "instanceID", logger)
+
+			release, err := sqlStore.GetOrCreateRingRelease(&model.RingRelease{
+				Version:      "test-version",
+				Image:        "test-image",
+				Force:        true,
+				EnvVariables: nil,
+				CreateAt:     time.Now().UnixNano(),
+			})
+			require.NoError(t, err)
+
 			Ring := &model.Ring{
-				State:           tc.InitialState,
-				ActiveReleaseID: "active-release-id",
+				State:            tc.InitialState,
+				ActiveReleaseID:  release.ID,
+				DesiredReleaseID: release.ID,
 			}
 
 			installationGroup := model.InstallationGroup{
 				Name:  "group1",
 				State: model.InstallationGroupStable,
 			}
 
-			err := sqlStore.CreateRing(Ring, &installationGroup)
+			err = sqlStore.CreateRing(Ring, &installationGroup)
 			require.NoError(t, err)
 
 			supervisor.Supervise(Ring)