From cf9267b8cce92de43ce261264634ffc9b7737f7a Mon Sep 17 00:00:00 2001 From: Kirill Sibirev Date: Tue, 16 Apr 2024 14:32:44 +0200 Subject: [PATCH] Refactor NeedsFullUpdate status (#238) * Refactor full update * test for update blocked --- controllers/component_manager.go | 23 ++++----------- controllers/sync.go | 46 +++++++++++++++++++++++------- controllers/ytsaurus_local_test.go | 30 +++++++++++++++++++ pkg/components/component.go | 3 +- pkg/components/master.go | 2 +- pkg/components/tablet_node.go | 2 +- 6 files changed, 74 insertions(+), 32 deletions(-) diff --git a/controllers/component_manager.go b/controllers/component_manager.go index eafb2c79..3debfc57 100644 --- a/controllers/component_manager.go +++ b/controllers/component_manager.go @@ -25,8 +25,7 @@ type ComponentManager struct { type ComponentManagerStatus struct { needSync bool needInit bool - needFullUpdate bool - needLocalUpdate []components.Component + needUpdate []components.Component allReadyOrUpdating bool } @@ -143,8 +142,7 @@ func NewComponentManager( status := ComponentManagerStatus{ needInit: false, needSync: false, - needFullUpdate: false, - needLocalUpdate: nil, + needUpdate: nil, allReadyOrUpdating: true, } for _, c := range allComponents { @@ -162,15 +160,8 @@ func NewComponentManager( c.SetReadyCondition(componentStatus) syncStatus := componentStatus.SyncStatus - if syncStatus == components.SyncStatusNeedFullUpdate { - status.needFullUpdate = true - } - if syncStatus == components.SyncStatusNeedLocalUpdate { - if status.needLocalUpdate == nil { - status.needLocalUpdate = make([]components.Component, 0) - } - status.needLocalUpdate = append(status.needLocalUpdate, c) + status.needUpdate = append(status.needUpdate, c) } if !components.IsRunningStatus(syncStatus) { @@ -247,12 +238,8 @@ func (cm *ComponentManager) needInit() bool { return cm.status.needInit } -func (cm *ComponentManager) needFullUpdate() bool { - return cm.status.needFullUpdate -} - -func (cm *ComponentManager) needLocalUpdate() []components.Component { - return cm.status.needLocalUpdate +func (cm *ComponentManager) needUpdate() []components.Component { + return cm.status.needUpdate } func (cm *ComponentManager) allReadyOrUpdating() bool { diff --git a/controllers/sync.go b/controllers/sync.go index 7717303b..00129eed 100644 --- a/controllers/sync.go +++ b/controllers/sync.go @@ -243,6 +243,37 @@ func getComponentNames(components []components.Component) []string { return names } +// chooseUpdateStrategy considers spec decides if operator should proceed with update or block. +// Block is indicated with non-empty blockMsg. +// Component names which are chosen for update are return in names slice. +// Nil names slice means "full update". +func chooseUpdateStrategy(spec ytv1.YtsaurusSpec, needUpdate []components.Component) (names []string, blockMsg string) { + isFullUpdateEnabled := spec.EnableFullUpdate + + masterNeedsUpdate := false + tabletNodesNeedUpdate := false + for _, comp := range needUpdate { + if comp.GetType() == consts.MasterType { + masterNeedsUpdate = true + continue + } + if comp.GetType() == consts.TabletNodeType { + tabletNodesNeedUpdate = true + continue + } + } + statefulNeedUpdate := masterNeedsUpdate || tabletNodesNeedUpdate + + if statefulNeedUpdate { + if isFullUpdateEnabled { + return nil, "" + } else { + return nil, "Full update is not allowed by enableFullUpdate field, ignoring it" + } + } + return getComponentNames(needUpdate), "" +} + func (r *YtsaurusReconciler) Sync(ctx context.Context, resource *ytv1.Ytsaurus) (ctrl.Result, error) { logger := log.FromContext(ctx) @@ -282,18 +313,13 @@ func (r *YtsaurusReconciler) Sync(ctx context.Context, resource *ytv1.Ytsaurus) err := ytsaurus.SaveClusterState(ctx, ytv1.ClusterStateReconfiguration) return ctrl.Result{Requeue: true}, err - case componentManager.needFullUpdate(): - logger.Info("Ytsaurus needs full update") - if !ytsaurus.GetResource().Spec.EnableFullUpdate { - logger.Info("Full update isn't allowed, ignore it") + case componentManager.needUpdate() != nil: + componentNames, blockMsg := chooseUpdateStrategy(ytsaurus.GetResource().Spec, componentManager.needUpdate()) + if blockMsg != "" { + logger.Info(blockMsg) return ctrl.Result{}, nil } - err := ytsaurus.SaveUpdatingClusterState(ctx, nil) - return ctrl.Result{Requeue: true}, err - - case componentManager.needLocalUpdate() != nil: - componentNames := getComponentNames(componentManager.needLocalUpdate()) - logger.Info("Ytsaurus needs local components update", "components", componentNames) + logger.Info("Ytsaurus needs components update", "components", componentNames) err := ytsaurus.SaveUpdatingClusterState(ctx, componentNames) return ctrl.Result{Requeue: true}, err } diff --git a/controllers/ytsaurus_local_test.go b/controllers/ytsaurus_local_test.go index dd3ba501..80d48652 100644 --- a/controllers/ytsaurus_local_test.go +++ b/controllers/ytsaurus_local_test.go @@ -4,6 +4,7 @@ import ( "fmt" "path/filepath" "testing" + "time" "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" @@ -124,3 +125,32 @@ func TestYtsaurusUpdateStatelessComponent(t *testing.T) { testutil.GetObject(h, "ds", &sts) require.Equal(t, imageUpdated, sts.Spec.Template.Spec.Containers[0].Image) } + +func TestYtsaurusUpdateMasterBlocked(t *testing.T) { + namespace := "upd-master" + h := prepareTest(t, namespace) + defer h.Stop() + + ytsaurusResource := testutil.BuildMinimalYtsaurus(namespace, ytsaurusName) + testutil.DeployObject(h, &ytsaurusResource) + + waitClusterState(h, ytv1.ClusterStateRunning) + + imageUpdated := testYtsaurusImage + "-updated" + ytsaurusResource.Spec.PrimaryMasters.Image = &imageUpdated + t.Log("[ Updating master with disabled full update (should block) ]") + ytsaurusResource.Spec.EnableFullUpdate = false + testutil.UpdateObject(h, &ytv1.Ytsaurus{}, &ytsaurusResource) + + // It is a bit hard to test if something NOT happened, so we just wait a little and check if update happened. + t.Log("[ Sleep for 5 seconds]") + time.Sleep(5 * time.Second) + + t.Log("[ Check that cluster is running but not updated ]") + ytsaurus := ytv1.Ytsaurus{} + testutil.GetObject(h, ytsaurusName, &ytsaurus) + require.Equal(t, ytv1.ClusterStateRunning, ytsaurus.Status.State) + sts := appsv1.StatefulSet{} + testutil.GetObject(h, "ms", &sts) + require.NotEqual(t, imageUpdated, sts.Spec.Template.Spec.Containers[0].Image) +} diff --git a/pkg/components/component.go b/pkg/components/component.go index 6c4298ef..267cc707 100644 --- a/pkg/components/component.go +++ b/pkg/components/component.go @@ -15,7 +15,6 @@ type SyncStatus string const ( SyncStatusBlocked SyncStatus = "Blocked" - SyncStatusNeedFullUpdate SyncStatus = "NeedFullUpdate" SyncStatusNeedLocalUpdate SyncStatus = "NeedLocalUpdate" SyncStatusPending SyncStatus = "Pending" SyncStatusReady SyncStatus = "Ready" @@ -23,7 +22,7 @@ const ( ) func IsRunningStatus(status SyncStatus) bool { - return status == SyncStatusReady || status == SyncStatusNeedLocalUpdate || status == SyncStatusNeedFullUpdate + return status == SyncStatusReady || status == SyncStatusNeedLocalUpdate } type ComponentStatus struct { diff --git a/pkg/components/master.go b/pkg/components/master.go index 28fa3f1c..47ba8cda 100644 --- a/pkg/components/master.go +++ b/pkg/components/master.go @@ -285,7 +285,7 @@ func (m *Master) doSync(ctx context.Context, dry bool) (ComponentStatus, error) var err error if ytv1.IsReadyToUpdateClusterState(m.ytsaurus.GetClusterState()) && m.server.needUpdate() { - return SimpleStatus(SyncStatusNeedFullUpdate), err + return SimpleStatus(SyncStatusNeedLocalUpdate), err } if m.ytsaurus.GetClusterState() == ytv1.ClusterStateUpdating { diff --git a/pkg/components/tablet_node.go b/pkg/components/tablet_node.go index fd90a9e4..763a7cfc 100644 --- a/pkg/components/tablet_node.go +++ b/pkg/components/tablet_node.go @@ -84,7 +84,7 @@ func (tn *TabletNode) doSync(ctx context.Context, dry bool) (ComponentStatus, er var err error if ytv1.IsReadyToUpdateClusterState(tn.ytsaurus.GetClusterState()) && tn.server.needUpdate() { - return SimpleStatus(SyncStatusNeedFullUpdate), err + return SimpleStatus(SyncStatusNeedLocalUpdate), err } if tn.ytsaurus.GetClusterState() == ytv1.ClusterStateUpdating {