From 4e2c6e756d7226ce151dd2e68ecce420e69b9528 Mon Sep 17 00:00:00 2001 From: Stylianos Rigas Date: Wed, 28 Jun 2023 19:26:50 +0300 Subject: [PATCH] [CLD-5461] Add fix so that when force is used a release can run in parallel 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 --- internal/supervisor/ring.go | 56 +++++++++++++++++++------------- internal/supervisor/ring_test.go | 52 +++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 26 deletions(-) diff --git a/internal/supervisor/ring.go b/internal/supervisor/ring.go index 5fff0aa..4bea7f0 100644 --- 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 index 85d5840..4dd3a66 100644 --- 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,9 +175,54 @@ 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{ @@ -184,7 +230,7 @@ func TestRingSupervisorSupervise(t *testing.T) { State: model.InstallationGroupStable, } - err := sqlStore.CreateRing(Ring, &installationGroup) + err = sqlStore.CreateRing(Ring, &installationGroup) require.NoError(t, err) supervisor.Supervise(Ring)