Remove DataNodes from StatelesOnly update

Issue ytsaurus#325
koct9i · Oct 23, 2024 · d9f082b · d9f082b
1 parent 638a5d8
commit d9f082b
Show file tree

Hide file tree

Showing 7 changed files with 115 additions and 99 deletions.
diff --git 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
@@ -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
@@ -3,6 +3,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/ytsaurus/ytsaurus-k8s-operator/pkg/components"
@@ -386,122 +387,110 @@ 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,
 			}, ""
 		} 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,
+			componentNames: canUpdate,
 		}, ""
+	case ytv1.UpdateSelectorDataNodesOnly:
 	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,
-		}, ""
 	case ytv1.UpdateSelectorStatelessOnly:
-		if !statelessNeedUpdate {
-			return updateMeta{}, "Only stateless components update is allowed by updateSelector, but they don't need update"
-		}
 		return updateMeta{
 			flow:           ytv1.UpdateFlowStateless,
-			componentNames: statelessNames,
+			componentNames: canUpdate,
 		}, ""
-	default:
-		return updateMeta{}, fmt.Sprintf("Unexpected update selector %s", configuredSelector)
 	}
+
+	return updateMeta{}, fmt.Sprintf("Unexpected update selector %s", configuredSelector)
 }
 
 func (r *YtsaurusReconciler) Sync(ctx context.Context, resource *ytv1.Ytsaurus) (ctrl.Result, error) {

diff --git 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.<br />If UpdateSelector is not empty EnableFullUpdate is ignored. |  | Enum: [ Nothing StatelessOnly MasterOnly TabletNodesOnly ExecNodesOnly Everything] <br /> |
+| `updateSelector` _[UpdateSelector](#updateselector)_ | UpdateSelector is an experimental field. Behaviour may change.<br />If UpdateSelector is not empty EnableFullUpdate is ignored. |  | Enum: [ Nothing MasterOnly DataNodesOnly TabletNodesOnly ExecNodesOnly StatelessOnly Everything] <br /> |
 | `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/suite_test.go b/test/e2e/suite_test.go
@@ -164,6 +164,12 @@ func NewYtsaurusStatusTracker() func(*ytv1.Ytsaurus) bool {
 	}
 }
 
+func ExpectYtsaurus(ctx context.Context, name types.NamespacedName) Assertion {
+	var ytsaurus ytv1.Ytsaurus
+	Expect(k8sClient.Get(ctx, name, &ytsaurus)).Should(Succeed())
+	return Expect(ytsaurus)
+}
+
 func EventuallyYtsaurus(ctx context.Context, name types.NamespacedName, timeout time.Duration) AsyncAssertion {
 	var ytsaurus ytv1.Ytsaurus
 	trackStatus := NewYtsaurusStatusTracker()
@@ -192,16 +198,24 @@ func HaveClusterState(state ytv1.ClusterState) otypes.GomegaMatcher {
 	return HaveField("Status.State", state)
 }
 
+func HaveClusterStateRunning() otypes.GomegaMatcher {
+	return HaveClusterState(ytv1.ClusterStateRunning)
+}
+
+func HaveClusterStateUpdating() otypes.GomegaMatcher {
+	return HaveClusterState(ytv1.ClusterStateUpdating)
+}
+
 func HaveClusterUpdateState(updateState ytv1.UpdateState) otypes.GomegaMatcher {
 	return And(
-		HaveClusterState(ytv1.ClusterStateUpdating),
+		HaveClusterStateUpdating(),
 		HaveField("Status.UpdateStatus.State", updateState),
 	)
 }
 
 func HaveClusterUpdatingComponents(components ...string) otypes.GomegaMatcher {
 	return And(
-		HaveClusterState(ytv1.ClusterStateUpdating),
+		HaveClusterStateRunning(),
 		HaveField("Status.UpdateStatus.Components", components),
 	)
 }