From b0cbce7f5347ff49be2e35e5e8d4e9bfd5a1a2b9 Mon Sep 17 00:00:00 2001 From: William Saakyan Date: Tue, 28 Jan 2025 13:03:42 +0100 Subject: [PATCH] Add update flow tests --- controllers/sync.go | 10 +- controllers/update_flow_steps.go | 49 +-- controllers/update_flow_steps_test.go | 308 ++++++++++++++++++ docs/api.md | 22 +- .../crds/ytsaurus.cluster.ytsaurus.tech.yaml | 10 + 5 files changed, 371 insertions(+), 28 deletions(-) create mode 100644 controllers/update_flow_steps_test.go diff --git a/controllers/sync.go b/controllers/sync.go index 7f46bdb9..cf1d1660 100644 --- a/controllers/sync.go +++ b/controllers/sync.go @@ -171,10 +171,14 @@ func (r *YtsaurusReconciler) Sync(ctx context.Context, resource *ytv1.Ytsaurus) case ytv1.ClusterStateUpdating: updatingComponents := ytsaurus.GetUpdatingComponents() - result, err := buildAndExecuteFlow(ctx, ytsaurus, componentManager, updatingComponents) + progressed, err := buildAndExecuteFlow(ctx, ytsaurus, componentManager, updatingComponents) - if result != nil { - return *result, err + if err != nil { + return ctrl.Result{}, err + } + + if progressed { + return ctrl.Result{Requeue: true}, err } case ytv1.ClusterStateCancelUpdate: diff --git a/controllers/update_flow_steps.go b/controllers/update_flow_steps.go index 88afd177..21b46e26 100644 --- a/controllers/update_flow_steps.go +++ b/controllers/update_flow_steps.go @@ -4,8 +4,6 @@ import ( "context" "fmt" - ctrl "sigs.k8s.io/controller-runtime" - ytv1 "github.com/ytsaurus/ytsaurus-k8s-operator/api/v1" apiProxy "github.com/ytsaurus/ytsaurus-k8s-operator/pkg/apiproxy" "github.com/ytsaurus/ytsaurus-k8s-operator/pkg/components" @@ -22,11 +20,20 @@ const ( stepResultMarkUnhappy stepResultMark = "unhappy" ) -func buildAndExecuteFlow(ctx context.Context, ytsaurus *apiProxy.Ytsaurus, componentManager *ComponentManager, updatingComponents []ytv1.Component) (*ctrl.Result, error) { - tree := buildFlowTree(updatingComponents, componentManager.allComponents) - _, err := tree.execute(ctx, ytsaurus, componentManager) - return &ctrl.Result{Requeue: true}, err +func buildAndExecuteFlow(ctx context.Context, ytsaurus *apiProxy.Ytsaurus, componentManager *ComponentManager, updatingComponents []ytv1.Component) (bool, error) { + allComponents := convertToYtComponents(componentManager.allComponents) + tree := buildFlowTree(updatingComponents, allComponents) + return tree.execute(ctx, ytsaurus, componentManager) +} +func convertToYtComponents(components []components.Component) []ytv1.Component { + result := make([]ytv1.Component, len(components)) + for i, c := range components { + result[i] = ytv1.Component{ + ComponentType: c.GetType(), + } + } + return result } var terminateTransitions = map[ytv1.UpdateState]ytv1.ClusterState{ @@ -42,7 +49,6 @@ func flowCheckStatusCondition(conditionName string) flowCondition { } return stepResultMarkUnsatisfied } - } type flowStep struct { @@ -101,15 +107,15 @@ type flowTree struct { index map[ytv1.UpdateState]*flowStep head *flowStep tail *flowStep - - deferredChain ytv1.UpdateState } func newFlowTree(head *flowStep) *flowTree { return &flowTree{ - index: make(map[ytv1.UpdateState]*flowStep), - head: head, - tail: head, + index: map[ytv1.UpdateState]*flowStep{ + head.updateState: head, + }, + head: head, + tail: head, } } @@ -122,6 +128,7 @@ func (f *flowTree) execute(ctx context.Context, ytsaurus *apiProxy.Ytsaurus, com mark := currentStep.checkCondition(ctx, ytsaurus, componentManager) // condition is not met, wait for the next update if mark == stepResultMarkUnsatisfied { + ytsaurus.LogUpdate(ctx, fmt.Sprintf("Update flow: condition not met for %s", currentState)) return false, nil } @@ -153,12 +160,6 @@ func (f *flowTree) execute(ctx context.Context, ytsaurus *apiProxy.Ytsaurus, com } func (f *flowTree) chain(steps ...*flowStep) *flowTree { - if f.deferredChain != "" { - if len(steps) != 0 { - f.index[f.deferredChain].chain(steps[0]) - f.deferredChain = "" - } - } for _, step := range steps { f.index[step.updateState] = step f.tail.chain(step) @@ -206,6 +207,7 @@ var flowConditions = map[ytv1.UpdateState]flowCondition{ ytv1.UpdateStateWaitingForYqlaUpdatingPrepare: flowCheckStatusCondition(consts.ConditionYqlaPreparedForUpdating), ytv1.UpdateStateWaitingForYqlaUpdate: flowCheckStatusCondition(consts.ConditionYqlaUpdated), ytv1.UpdateStateWaitingForSafeModeDisabled: flowCheckStatusCondition(consts.ConditionSafeModeDisabled), + ytv1.UpdateStateWaitingForMasterExitReadOnly: flowCheckStatusCondition(consts.ConditionMasterExitedReadOnly), ytv1.UpdateStateWaitingForPodsRemoval: func(ctx context.Context, ytsaurus *apiProxy.Ytsaurus, componentManager *ComponentManager) stepResultMark { if componentManager.areNonMasterPodsRemoved() { return stepResultMarkHappy @@ -232,7 +234,7 @@ var flowConditions = map[ytv1.UpdateState]flowCondition{ }, } -func buildFlowTree(updatingComponents []ytv1.Component, allComponents []components.Component) *flowTree { +func buildFlowTree(updatingComponents []ytv1.Component, allComponents []ytv1.Component) *flowTree { st := newSimpleStep head := st(ytv1.UpdateStateNone) tree := newFlowTree(head) @@ -245,7 +247,8 @@ func buildFlowTree(updatingComponents []ytv1.Component, allComponents []componen updYqlAgent := hasComponent(updatingComponents, allComponents, consts.YqlAgentType) // TODO: if validation conditions can be not mentioned here or needed - tree.chain( + tree.chainIf( + updMasterOrTablet, newConditionalForkStep( ytv1.UpdateStatePossibilityCheck, // This is the unhappy path. @@ -297,10 +300,10 @@ func buildFlowTree(updatingComponents []ytv1.Component, allComponents []componen return tree } -func hasComponent(updatingComponents []ytv1.Component, allComponents []components.Component, componentType consts.ComponentType) bool { - if updatingComponents == nil { +func hasComponent(updatingComponents []ytv1.Component, allComponents []ytv1.Component, componentType consts.ComponentType) bool { + if len(updatingComponents) == 0 { for _, component := range allComponents { - if component.GetType() == componentType { + if component.ComponentType == componentType { return true } } diff --git a/controllers/update_flow_steps_test.go b/controllers/update_flow_steps_test.go new file mode 100644 index 00000000..76fa34e3 --- /dev/null +++ b/controllers/update_flow_steps_test.go @@ -0,0 +1,308 @@ +package controllers + +import ( + "testing" + + "github.com/stretchr/testify/require" + + ytv1 "github.com/ytsaurus/ytsaurus-k8s-operator/api/v1" + "github.com/ytsaurus/ytsaurus-k8s-operator/pkg/consts" +) + +func TestBuildFlowTree(t *testing.T) { + fullComponents := []ytv1.Component{ + {ComponentType: consts.MasterType}, + {ComponentType: consts.TabletNodeType}, + {ComponentType: consts.SchedulerType}, + {ComponentType: consts.QueryTrackerType}, + {ComponentType: consts.YqlAgentType}, + } + + masterTabletComponents := []ytv1.Component{ + {ComponentType: consts.MasterType}, + {ComponentType: consts.TabletNodeType}, + } + + onlyMasterComponents := []ytv1.Component{ + {ComponentType: consts.MasterType}, + } + + tests := []struct { + name string + updatingComponents []ytv1.Component + allComponents []ytv1.Component + expectedStates []ytv1.UpdateState + }{ + { + name: "empty updating components with full components", + updatingComponents: []ytv1.Component{}, + allComponents: fullComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStatePossibilityCheck, + ytv1.UpdateStateWaitingForSafeModeEnabled, + ytv1.UpdateStateWaitingForTabletCellsSaving, + ytv1.UpdateStateWaitingForTabletCellsRemovingStart, + ytv1.UpdateStateWaitingForTabletCellsRemoved, + ytv1.UpdateStateWaitingForEnableRealChunkLocations, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForSnapshots, + ytv1.UpdateStateWaitingForMasterPodsRemoval, + ytv1.UpdateStateWaitingForMasterPodsCreation, + ytv1.UpdateStateWaitingForMasterExitReadOnly, + ytv1.UpdateStateWaitingForTabletCellsRecovery, + ytv1.UpdateStateWaitingForOpArchiveUpdatingPrepare, + ytv1.UpdateStateWaitingForOpArchiveUpdate, + ytv1.UpdateStateWaitingForQTStateUpdatingPrepare, + ytv1.UpdateStateWaitingForQTStateUpdate, + ytv1.UpdateStateWaitingForYqlaUpdatingPrepare, + ytv1.UpdateStateWaitingForYqlaUpdate, + ytv1.UpdateStateWaitingForSafeModeDisabled, + }, + }, + { + name: "empty updating components with master-tablet only", + updatingComponents: []ytv1.Component{}, + allComponents: masterTabletComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStatePossibilityCheck, + ytv1.UpdateStateWaitingForSafeModeEnabled, + ytv1.UpdateStateWaitingForTabletCellsSaving, + ytv1.UpdateStateWaitingForTabletCellsRemovingStart, + ytv1.UpdateStateWaitingForTabletCellsRemoved, + ytv1.UpdateStateWaitingForEnableRealChunkLocations, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForSnapshots, + ytv1.UpdateStateWaitingForMasterPodsRemoval, + ytv1.UpdateStateWaitingForMasterPodsCreation, + ytv1.UpdateStateWaitingForMasterExitReadOnly, + ytv1.UpdateStateWaitingForTabletCellsRecovery, + ytv1.UpdateStateWaitingForSafeModeDisabled, + }, + }, + { + name: "empty updating components with master only", + updatingComponents: []ytv1.Component{}, + allComponents: onlyMasterComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStatePossibilityCheck, + ytv1.UpdateStateWaitingForSafeModeEnabled, + ytv1.UpdateStateWaitingForEnableRealChunkLocations, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForSnapshots, + ytv1.UpdateStateWaitingForMasterPodsRemoval, + ytv1.UpdateStateWaitingForMasterPodsCreation, + ytv1.UpdateStateWaitingForMasterExitReadOnly, + ytv1.UpdateStateWaitingForSafeModeDisabled, + }, + }, + { + name: "master update with full components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.MasterType}, + }, + allComponents: fullComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStatePossibilityCheck, + ytv1.UpdateStateWaitingForSafeModeEnabled, + ytv1.UpdateStateWaitingForEnableRealChunkLocations, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForSnapshots, + ytv1.UpdateStateWaitingForMasterPodsRemoval, + ytv1.UpdateStateWaitingForMasterPodsCreation, + ytv1.UpdateStateWaitingForMasterExitReadOnly, + ytv1.UpdateStateWaitingForSafeModeDisabled, + }, + }, + { + name: "tablet update with full components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.TabletNodeType}, + }, + allComponents: fullComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStatePossibilityCheck, + ytv1.UpdateStateWaitingForSafeModeEnabled, + ytv1.UpdateStateWaitingForTabletCellsSaving, + ytv1.UpdateStateWaitingForTabletCellsRemovingStart, + ytv1.UpdateStateWaitingForTabletCellsRemoved, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForTabletCellsRecovery, + }, + }, + { + name: "scheduler update with full components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.SchedulerType}, + }, + allComponents: fullComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForOpArchiveUpdatingPrepare, + ytv1.UpdateStateWaitingForOpArchiveUpdate, + }, + }, + { + name: "query tracker update with full components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.QueryTrackerType}, + }, + allComponents: fullComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForQTStateUpdatingPrepare, + ytv1.UpdateStateWaitingForQTStateUpdate, + }, + }, + { + name: "yql agent update with full components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.YqlAgentType}, + }, + allComponents: fullComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForYqlaUpdatingPrepare, + ytv1.UpdateStateWaitingForYqlaUpdate, + }, + }, + { + name: "random stateless component update with full components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.DiscoveryType}, + }, + allComponents: fullComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + }, + }, + { + name: "combined master and tablet update with full components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.MasterType}, + {ComponentType: consts.TabletNodeType}, + }, + allComponents: fullComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStatePossibilityCheck, + ytv1.UpdateStateWaitingForSafeModeEnabled, + ytv1.UpdateStateWaitingForTabletCellsSaving, + ytv1.UpdateStateWaitingForTabletCellsRemovingStart, + ytv1.UpdateStateWaitingForTabletCellsRemoved, + ytv1.UpdateStateWaitingForEnableRealChunkLocations, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForSnapshots, + ytv1.UpdateStateWaitingForMasterPodsRemoval, + ytv1.UpdateStateWaitingForMasterPodsCreation, + ytv1.UpdateStateWaitingForMasterExitReadOnly, + ytv1.UpdateStateWaitingForTabletCellsRecovery, + ytv1.UpdateStateWaitingForSafeModeDisabled, + }, + }, + { + name: "master update with master-tablet components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.MasterType}, + }, + allComponents: masterTabletComponents, + expectedStates: []ytv1.UpdateState{ + ytv1.UpdateStateNone, + ytv1.UpdateStatePossibilityCheck, + ytv1.UpdateStateWaitingForSafeModeEnabled, + ytv1.UpdateStateWaitingForEnableRealChunkLocations, + ytv1.UpdateStateWaitingForPodsRemoval, + ytv1.UpdateStateWaitingForPodsCreation, + ytv1.UpdateStateWaitingForSnapshots, + ytv1.UpdateStateWaitingForMasterPodsRemoval, + ytv1.UpdateStateWaitingForMasterPodsCreation, + ytv1.UpdateStateWaitingForMasterExitReadOnly, + ytv1.UpdateStateWaitingForSafeModeDisabled, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tree := buildFlowTree(tt.updatingComponents, tt.allComponents) + + // Collect all states from the tree + var states []ytv1.UpdateState + currentStep := tree.head + for currentStep != nil { + states = append(states, currentStep.updateState) + currentStep = currentStep.nextSteps[stepResultMarkHappy] + } + + require.Equal(t, tt.expectedStates, states, "Flow states mismatch") + }) + } +} + +func TestHasComponent(t *testing.T) { + allComponents := []ytv1.Component{ + {ComponentType: consts.MasterType}, + {ComponentType: consts.TabletNodeType}, + } + + tests := []struct { + name string + updatingComponents []ytv1.Component + componentType consts.ComponentType + expected bool + }{ + { + name: "nil updating components", + updatingComponents: nil, + componentType: consts.MasterType, + expected: true, + }, + { + name: "empty updating components", + updatingComponents: []ytv1.Component{}, + componentType: consts.MasterType, + expected: true, + }, + { + name: "component present in updating components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.MasterType}, + }, + componentType: consts.MasterType, + expected: true, + }, + { + name: "component not present in updating components", + updatingComponents: []ytv1.Component{ + {ComponentType: consts.TabletNodeType}, + }, + componentType: consts.MasterType, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasComponent(tt.updatingComponents, allComponents, tt.componentType) + require.Equal(t, tt.expected, result) + }) + } +} diff --git a/docs/api.md b/docs/api.md index 9ef2dff1..6436bf2f 100644 --- a/docs/api.md +++ b/docs/api.md @@ -327,6 +327,23 @@ _Appears in:_ | `imagePullSecrets` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#localobjectreference-v1-core) array_ | | | | +#### Component + + + + + + + +_Appears in:_ +- [UpdateStatus](#updatestatus) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `componentName` _string_ | | | | +| `componentType` _[ComponentType](#componenttype)_ | | | | + + #### ControllerAgentsSpec @@ -2200,8 +2217,9 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `state` _[UpdateState](#updatestate)_ | | None | | -| `components` _string array_ | | | | -| `flow` _[UpdateFlow](#updateflow)_ | Flow is an internal field that is needed to persist the chosen flow until the end of an update.
Flow can be on of ""(unspecified), Stateless, Master, TabletNodes, Full and update cluster stage
executes steps corresponding to that update flow. | | | +| `components` _string array_ | Deprecated: Use updatingComponents instead. | | | +| `updatingComponents` _[Component](#component) array_ | | | | +| `flow` _[UpdateFlow](#updateflow)_ | Flow is an internal field that is needed to persist the chosen flow until the end of an update.
Flow can be on of ""(unspecified), Stateless, Master, TabletNodes, Full and update cluster stage
executes steps corresponding to that update flow.
Deprecated: Use updatingComponents instead. | | | | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#condition-v1-meta) array_ | | | | | `tabletCellBundles` _[TabletCellBundleInfo](#tabletcellbundleinfo) array_ | | | | | `masterMonitoringPaths` _string array_ | | | | diff --git a/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml b/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml index 833f5507..d557e967 100644 --- a/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml +++ b/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml @@ -42233,6 +42233,7 @@ spec: updateStatus: properties: components: + description: 'Deprecated: Use updatingComponents instead.' items: type: string type: array @@ -42307,6 +42308,15 @@ spec: - tabletCellCount type: object type: array + updatingComponents: + items: + properties: + componentName: + type: string + componentType: + type: string + type: object + type: array type: object type: object type: object