From a80ff741398f665134a4f9addd7fb79026ff3762 Mon Sep 17 00:00:00 2001 From: Konstantin Khlebnikov Date: Wed, 23 Oct 2024 10:44:47 +0200 Subject: [PATCH] Remove DataNodes from StatelesOnly update Issue #325 --- api/v1/ytsaurus_types.go | 10 +- .../bases/cluster.ytsaurus.tech_ytsaurus.yaml | 3 +- controllers/sync.go | 135 ++++++++---------- docs/api.md | 2 +- test/e2e/ytsaurus_controller_test.go | 2 +- .../crds/ytsaurus.cluster.ytsaurus.tech.yaml | 3 +- 6 files changed, 73 insertions(+), 82 deletions(-) diff --git a/api/v1/ytsaurus_types.go b/api/v1/ytsaurus_types.go index 702e4777..b566582f 100644 --- a/api/v1/ytsaurus_types.go +++ b/api/v1/ytsaurus_types.go @@ -614,7 +614,7 @@ type YtsaurusSpec struct { //+optional EnableFullUpdate bool `json:"enableFullUpdate"` //+optional - //+kubebuilder:validation:Enum={"","Nothing","StatelessOnly","MasterOnly","TabletNodesOnly","ExecNodesOnly","Everything"} + //+kubebuilder:validation:Enum={"","Nothing","MasterOnly","DataNodesOnly","TabletNodesOnly","ExecNodesOnly","StatelessOnly","Everything"} // UpdateSelector is an experimental field. Behaviour may change. // If UpdateSelector is not empty EnableFullUpdate is ignored. UpdateSelector UpdateSelector `json:"updateSelector"` @@ -699,15 +699,17 @@ const ( UpdateSelectorUnspecified UpdateSelector = "" // UpdateSelectorNothing means that no component could be updated. UpdateSelectorNothing UpdateSelector = "Nothing" - // UpdateSelectorStatelessOnly means that only stateless components (everything but master and tablet nodes) - // could be updated. - UpdateSelectorStatelessOnly UpdateSelector = "StatelessOnly" // UpdateSelectorMasterOnly means that only master could be updated. UpdateSelectorMasterOnly UpdateSelector = "MasterOnly" + // UpdateSelectorTabletNodesOnly means that only data nodes could be updated + UpdateSelectorDataNodesOnly UpdateSelector = "DataNodesOnly" // UpdateSelectorTabletNodesOnly means that only tablet nodes could be updated UpdateSelectorTabletNodesOnly UpdateSelector = "TabletNodesOnly" // UpdateSelectorExecNodesOnly means that only tablet nodes could be updated UpdateSelectorExecNodesOnly UpdateSelector = "ExecNodesOnly" + // UpdateSelectorStatelessOnly means that only stateless components (everything but master, data nodes, and tablet nodes) + // could be updated. + UpdateSelectorStatelessOnly UpdateSelector = "StatelessOnly" // UpdateSelectorEverything means that all components could be updated. // With this setting and if master or tablet nodes need update all the components would be updated. UpdateSelectorEverything UpdateSelector = "Everything" diff --git a/config/crd/bases/cluster.ytsaurus.tech_ytsaurus.yaml b/config/crd/bases/cluster.ytsaurus.tech_ytsaurus.yaml index c8d18a16..16eb28f8 100644 --- a/config/crd/bases/cluster.ytsaurus.tech_ytsaurus.yaml +++ b/config/crd/bases/cluster.ytsaurus.tech_ytsaurus.yaml @@ -34637,10 +34637,11 @@ spec: enum: - "" - Nothing - - StatelessOnly - MasterOnly + - DataNodesOnly - TabletNodesOnly - ExecNodesOnly + - StatelessOnly - Everything type: string useIpv4: diff --git a/controllers/sync.go b/controllers/sync.go index 766185da..cbeec4d5 100644 --- a/controllers/sync.go +++ b/controllers/sync.go @@ -3,6 +3,7 @@ package controllers import ( "context" "fmt" + "strings" "time" "github.com/ytsaurus/ytsaurus-k8s-operator/pkg/components" @@ -386,77 +387,80 @@ func (r *YtsaurusReconciler) handleTabletNodesOnly( return nil, nil } -func getComponentNames(components []components.Component) []string { - if components == nil { - return nil - } - names := make([]string, 0) - for _, cmp := range components { - names = append(names, cmp.GetName()) - } - return names -} - type updateMeta struct { flow ytv1.UpdateFlow // componentNames is a list of component names that will be updated. It is built according to the update selector. componentNames []string } +func canUpdateComponent(selector ytv1.UpdateSelector, component consts.ComponentType) bool { + switch selector { + case ytv1.UpdateSelectorNothing: + return false + case ytv1.UpdateSelectorMasterOnly: + return component == consts.MasterType + case ytv1.UpdateSelectorDataNodesOnly: + return component == consts.DataNodeType + case ytv1.UpdateSelectorTabletNodesOnly: + return component == consts.TabletNodeType + case ytv1.UpdateSelectorExecNodesOnly: + return component == consts.ExecNodeType + case ytv1.UpdateSelectorStatelessOnly: + switch component { + case consts.MasterType: + return false + case consts.DataNodeType: + return false + case consts.TabletNodeType: + return false + } + return true + case ytv1.UpdateSelectorEverything: + return true + default: + return false + } +} + // chooseUpdateFlow considers spec and decides if operator should proceed with update or block. // Block case is indicated with non-empty blockMsg. // If update is not blocked, updateMeta containing a chosen flow and the component names to update returned. func chooseUpdateFlow(spec ytv1.YtsaurusSpec, needUpdate []components.Component) (meta updateMeta, blockMsg string) { - isFullUpdateEnabled := spec.EnableFullUpdate configuredSelector := spec.UpdateSelector + if configuredSelector == ytv1.UpdateSelectorUnspecified { + if spec.EnableFullUpdate { + configuredSelector = ytv1.UpdateSelectorEverything + } else { + configuredSelector = ytv1.UpdateSelectorStatelessOnly + } + } + + var canUpdate []string + var cannotUpdate []string + needFullUpdate := false - masterNeedsUpdate := false - tabletNodesNeedUpdate := false - execNodesNeedUpdate := false - statelessNeedUpdate := false - var masterNames []string - var tabletNodeNames []string - var execNodeNames []string - var statelessNames []string for _, comp := range needUpdate { - if comp.GetType() == consts.MasterType { - masterNeedsUpdate = true - masterNames = append(masterNames, comp.GetName()) - continue - } - if comp.GetType() == consts.TabletNodeType { - tabletNodesNeedUpdate = true - tabletNodeNames = append(tabletNodeNames, comp.GetName()) - continue - } - if comp.GetType() == consts.ExecNodeType { - execNodesNeedUpdate = true - execNodeNames = append(execNodeNames, comp.GetName()) - } - statelessNames = append(statelessNames, comp.GetName()) - statelessNeedUpdate = true + component := comp.GetType() + if canUpdateComponent(configuredSelector, component) { + canUpdate = append(canUpdate, string(component)) + } else { + cannotUpdate = append(cannotUpdate, string(component)) + } + if !canUpdateComponent(ytv1.UpdateSelectorStatelessOnly, component) && component != consts.DataNodeType { + needFullUpdate = true + } } - statefulNeedUpdate := masterNeedsUpdate || tabletNodesNeedUpdate - - allNamesNeedingUpdate := getComponentNames(needUpdate) - // Fallback to EnableFullUpdate field. - if configuredSelector == ytv1.UpdateSelectorUnspecified { - if statefulNeedUpdate { - if isFullUpdateEnabled { - return updateMeta{flow: ytv1.UpdateFlowFull, componentNames: nil}, "" - } else { - return updateMeta{flow: "", componentNames: nil}, "Full update is not allowed by enableFullUpdate field, ignoring it" - } + if len(canUpdate) == 0 { + if len(cannotUpdate) != 0 { + return updateMeta{}, fmt.Sprintf("All components allowed by updateSelector are uptodate, update of {%s} is not allowed", strings.Join(cannotUpdate, ", ")) } - return updateMeta{flow: ytv1.UpdateFlowStateless, componentNames: allNamesNeedingUpdate}, "" + return updateMeta{}, "All components are uptodate" } switch configuredSelector { - case ytv1.UpdateSelectorNothing: - return updateMeta{}, "All updates are blocked by updateSelector field." case ytv1.UpdateSelectorEverything: - if statefulNeedUpdate { + if needFullUpdate { return updateMeta{ flow: ytv1.UpdateFlowFull, componentNames: nil, @@ -464,40 +468,23 @@ func chooseUpdateFlow(spec ytv1.YtsaurusSpec, needUpdate []components.Component) } else { return updateMeta{ flow: ytv1.UpdateFlowStateless, - componentNames: allNamesNeedingUpdate, + componentNames: canUpdate, }, "" } case ytv1.UpdateSelectorMasterOnly: - if !masterNeedsUpdate { - return updateMeta{}, "Only Master update is allowed by updateSelector, but it doesn't need update" - } return updateMeta{ flow: ytv1.UpdateFlowMaster, - componentNames: masterNames, + componentNames: canUpdate, }, "" case ytv1.UpdateSelectorTabletNodesOnly: - if !tabletNodesNeedUpdate { - return updateMeta{}, "Only Tablet nodes update is allowed by updateSelector, but they don't need update" - } return updateMeta{ flow: ytv1.UpdateFlowTabletNodes, - componentNames: tabletNodeNames, - }, "" - case ytv1.UpdateSelectorExecNodesOnly: - if !execNodesNeedUpdate { - return updateMeta{}, "Only Exec nodes update is allowed by updateSelector, but they don't need update" - } - return updateMeta{ - flow: ytv1.UpdateFlowStateless, - componentNames: execNodeNames, + componentNames: canUpdate, }, "" - case ytv1.UpdateSelectorStatelessOnly: - if !statelessNeedUpdate { - return updateMeta{}, "Only stateless components update is allowed by updateSelector, but they don't need update" - } + case ytv1.UpdateSelectorDataNodesOnly, ytv1.UpdateSelectorExecNodesOnly, ytv1.UpdateSelectorStatelessOnly: return updateMeta{ flow: ytv1.UpdateFlowStateless, - componentNames: statelessNames, + componentNames: canUpdate, }, "" default: return updateMeta{}, fmt.Sprintf("Unexpected update selector %s", configuredSelector) @@ -548,7 +535,7 @@ func (r *YtsaurusReconciler) Sync(ctx context.Context, resource *ytv1.Ytsaurus) err := ytsaurus.SaveClusterState(ctx, ytv1.ClusterStateReconfiguration) return ctrl.Result{Requeue: true}, err - case needUpdate != nil: + case len(needUpdate) != 0: var needUpdateNames []string for _, c := range needUpdate { needUpdateNames = append(needUpdateNames, c.GetName()) diff --git a/docs/api.md b/docs/api.md index 4e7a9fc0..a39ec988 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1836,7 +1836,7 @@ _Appears in:_ | `oauthService` _[OauthServiceSpec](#oauthservicespec)_ | | | | | `isManaged` _boolean_ | | true | | | `enableFullUpdate` _boolean_ | | true | | -| `updateSelector` _[UpdateSelector](#updateselector)_ | UpdateSelector is an experimental field. Behaviour may change.
If UpdateSelector is not empty EnableFullUpdate is ignored. | | Enum: [ Nothing StatelessOnly MasterOnly TabletNodesOnly ExecNodesOnly Everything]
| +| `updateSelector` _[UpdateSelector](#updateselector)_ | UpdateSelector is an experimental field. Behaviour may change.
If UpdateSelector is not empty EnableFullUpdate is ignored. | | Enum: [ Nothing MasterOnly DataNodesOnly TabletNodesOnly ExecNodesOnly StatelessOnly Everything]
| | `nodeSelector` _object (keys:string, values:string)_ | | | | | `tolerations` _[Toleration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#toleration-v1-core) array_ | | | | | `bootstrap` _[BootstrapSpec](#bootstrapspec)_ | | | | diff --git a/test/e2e/ytsaurus_controller_test.go b/test/e2e/ytsaurus_controller_test.go index 6f9e0d3a..865863ab 100644 --- a/test/e2e/ytsaurus_controller_test.go +++ b/test/e2e/ytsaurus_controller_test.go @@ -432,7 +432,7 @@ var _ = Describe("Basic test for Ytsaurus controller", func() { ytsaurus.Spec.Discovery.InstanceCount += 1 Expect(k8sClient.Update(ctx, ytsaurus)).Should(Succeed()) EventuallyYtsaurus(ctx, name, reactionTimeout).Should( - HaveClusterUpdatingComponents("Discovery", "DataNode", "HttpProxy", "ExecNode", "Scheduler", "ControllerAgent"), + HaveClusterUpdatingComponents("Discovery", "HttpProxy", "ExecNode", "Scheduler", "ControllerAgent"), ) By("Wait cluster update with selector:StatelessOnly complete") EventuallyYtsaurus(ctx, name, upgradeTimeout).Should(HaveClusterState(ytv1.ClusterStateRunning)) diff --git a/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml b/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml index a2c37e16..d537e354 100644 --- a/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml +++ b/ytop-chart/templates/crds/ytsaurus.cluster.ytsaurus.tech.yaml @@ -34648,10 +34648,11 @@ spec: enum: - "" - Nothing - - StatelessOnly - MasterOnly + - DataNodesOnly - TabletNodesOnly - ExecNodesOnly + - StatelessOnly - Everything type: string useIpv4: