Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multiple update selectors #383

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 19 additions & 23 deletions api/v1/ytsaurus_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/ytsaurus/ytsaurus-k8s-operator/pkg/consts"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
Expand Down Expand Up @@ -614,11 +616,14 @@ type YtsaurusSpec struct {
//+optional
EnableFullUpdate bool `json:"enableFullUpdate"`
//+optional
//+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.
//+optional
// Deprecated: UpdateSelector is an experimental field. Behaviour may change.
UpdateSelector UpdateSelector `json:"updateSelector"`

//+optional
// Controls the components that should be updated during the update process.
UpdateSelectors []ComponentUpdateSelector `json:"updateSelectors,omitempty"`

NodeSelector map[string]string `json:"nodeSelector,omitempty"`
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Expand Down Expand Up @@ -694,26 +699,17 @@ type TabletCellBundleInfo struct {

type UpdateSelector string

const (
// UpdateSelectorUnspecified means that selector is disabled and would be ignored completely.
UpdateSelectorUnspecified UpdateSelector = ""
// UpdateSelectorNothing means that no component could be updated.
UpdateSelectorNothing UpdateSelector = "Nothing"
// 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"
)
type ComponentUpdateSelector struct {
//+optional
Component consts.ComponentType `json:"componentKind,omitempty"`
//+optional
ComponentGroup consts.ComponentGroup `json:"componentGroup,omitempty"`
//+kubebuilder:default:=false
//+optional
Update bool `json:"update"`

// TODO(#325): Add name field for specific sts and rolling options
}

type UpdateFlow string

Expand Down
20 changes: 20 additions & 0 deletions api/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 16 additions & 11 deletions config/crd/bases/cluster.ytsaurus.tech_ytsaurus.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34632,18 +34632,23 @@ spec:
uiImage:
type: string
updateSelector:
description: UpdateSelector is an experimental field. Behaviour may
change.
enum:
- ""
- Nothing
- MasterOnly
- DataNodesOnly
- TabletNodesOnly
- ExecNodesOnly
- StatelessOnly
- Everything
description: 'Deprecated: UpdateSelector is an experimental field.
Behaviour may change.'
type: string
updateSelectors:
description: Controls the components that should be updated during
the update process.
items:
properties:
componentGroup:
type: string
componentKind:
type: string
update:
default: false
type: boolean
type: object
type: array
useIpv4:
default: false
type: boolean
Expand Down
132 changes: 68 additions & 64 deletions controllers/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,60 +393,75 @@
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
func getFlowFromComponent(component consts.ComponentType) ytv1.UpdateFlow {
if component == consts.MasterType {
return ytv1.UpdateFlowMaster
}
if component == consts.TabletNodeType {
return ytv1.UpdateFlowTabletNodes
}
if component == consts.DataNodeType || component == consts.ExecNodeType {
return ytv1.UpdateFlowFull
}
return ytv1.UpdateFlowStateless
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this PR is raw yet, but just want to emphasize some things.
Obviously to add updateSelectors field is not challenging part of the issue, but to correctly update each component (or combination of components) selected by it, so all required pre and post steps were executed.

Currently we have some flows for component updates:
master

  • PossibilityCheck
  • WaitingForSafeModeEnabled
  • WaitingForSnapshots
  • WaitingForPodsRemoval
  • WaitingForPodsCreation
  • WaitingForMasterExitReadOnly
  • WaitingForSafeModeDisabled

tablet nodes

  • PossibilityCheck
  • WaitingForTabletCellsSaving
  • WaitingForTabletCellsRemovingStart
  • WaitingForTabletCellsRemoved
  • WaitingForPodsRemoval
  • WaitingForPodsCreation
  • WaitingForTabletCellsRecovery

stateless

  • WaitingForPodsRemoval
  • WaitingForPodsCreation
  • WaitingForOpArchiveUpdatingPrepare
  • WaitingForOpArchiveUpdate
  • WaitingForQTStateUpdatingPrepare
  • WaitingForQTStateUpdate

everything

  • PossibilityCheck
  • WaitingForSafeModeEnabled
  • WaitingForTabletCellsSaving
  • WaitingForTabletCellsRemovingStart
  • WaitingForTabletCellsRemoved
  • WaitingForSnapshots
  • WaitingForPodsRemoval
  • WaitingForPodsCreation
  • WaitingForMasterExitReadOnly
  • WaitingForTabletCellsRecovery
  • WaitingForOpArchiveUpdatingPrepare
  • WaitingForOpArchiveUpdate
  • WaitingForQTStateUpdatingPrepare
  • WaitingForQTStateUpdate
  • WaitingForSafeModeDisabled

As you see everything includes (basically copy-pastes) parts master,tnds,stateless flows.
And
if you want implement UpdateSelectors = {component=scheduler} you have to add flow for scheduler with WaitingForOpArchiveUpdatingPrepare/WaitingForOpArchiveUpdate/WaitingForQTStateUpdatingPrepare/WaitingForQTStateUpdate
if you want to implement UpdateSelectors = {component=qt} you have to add flow with WaitingForQTStateUpdatingPrepare/WaitingForQTStateUpdate
and so on.
So this predefined flows code is a dead end way if we want to achieve update granularity we want. So we should somehow concatenate workflows for particular components.

And i think the most simplest way here is to do this change with introducing Linear update (#115).
I assume the flow in the code should look something like that:

# update flow
if discoveryType in toUpdateList: // updatable by selector and require update
	remove/create discovery pods

if httpProxy in updateList:
       remove/create hp pods
// tcp proxy
// rpc proxy
// data nodes
// exec nodes
// master cache

if tabletNodes in updateList:
     - PossibilityCheck
     - WaitingForTabletCellsSaving
     - WaitingForTabletCellsRemovingStart
     - WaitingForTabletCellsRemoved
     - WaitingForPodsRemoval
     - WaitingForPodsCreation
     - WaitingForTabletCellsRecovery

// ui
// ca
// yqla (but soon also will have pre-update jobs)
// sch with its flow
// qt with its flow
// qa with its flow
// strawberry
// master with its flow (updating in the end, because it should never be newer than other components)

Initializing flow is the same, but master in the beginning.

So basically I think we need here to:

  1. resolve updateSelectors in a list of allowed components, match it with all component's statuses so we have a list of components we plan to update, sort it according to predefined order, choose first one we will update in this reconcillation loop, put it in UpdateStatus.Components
  2. check current UpdateStatus.State and UpdateStatus.Conditions and either call components.sync and requeue or set next State of this component flow and requeue.

I understand it is a bit vague overview, but maybe it helps.


func canUpdateComponent(selectors []ytv1.ComponentUpdateSelector, component consts.ComponentType) (bool, error) {
for _, selector := range selectors {
if selector.Component != "" {
if selector.Component == component {
return true, nil
}
} else {
switch selector.ComponentGroup {
case consts.ComponentGroupEverything:
return true, nil
case consts.ComponentGroupStateful:
if component == consts.DataNodeType || component == consts.ExecNodeType {
return true, nil
}
case consts.ComponentGroupStateless:
if component != consts.DataNodeType && component != consts.ExecNodeType && component != consts.MasterType {
return true, nil
}
default:
return false, fmt.Errorf("unexpected component group %s", selector.ComponentGroup)
}
}
}
return false, nil
}

// 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) {
configuredSelector := spec.UpdateSelector
if configuredSelector == ytv1.UpdateSelectorUnspecified {
if spec.EnableFullUpdate {
configuredSelector = ytv1.UpdateSelectorEverything
} else {
configuredSelector = ytv1.UpdateSelectorStatelessOnly
}
print("\n\n\n!!!!!AAAAAA!!!!!\n\n\n")
configuredSelectors := spec.UpdateSelectors
if len(configuredSelectors) == 0 && spec.EnableFullUpdate {
configuredSelectors = []ytv1.ComponentUpdateSelector{{ComponentGroup: consts.ComponentGroupEverything, Update: true}}
}
needFullUpdate := false

var canUpdate []string
var cannotUpdate []string
needFullUpdate := false
flows := make(map[ytv1.UpdateFlow]struct{})

for _, comp := range needUpdate {
component := comp.GetType()
if canUpdateComponent(configuredSelector, component) {
upd, err := canUpdateComponent(configuredSelectors, component)
if err != nil {
return updateMeta{}, err.Error()
}
if upd {
canUpdate = append(canUpdate, string(component))
flows[getFlowFromComponent(component)] = struct{}{}
} else {
cannotUpdate = append(cannotUpdate, string(component))
}
if !canUpdateComponent(ytv1.UpdateSelectorStatelessOnly, component) && component != consts.DataNodeType {

statelessCheck, err := canUpdateComponent([]ytv1.ComponentUpdateSelector{{ComponentGroup: consts.ComponentGroupStateless}}, component)

Check failure on line 463 in controllers/sync.go

View workflow job for this annotation

GitHub Actions / Run checks

SA4006: this value of `err` is never used (staticcheck)
if !statelessCheck && component != consts.DataNodeType {
needFullUpdate = true
}
}
Expand All @@ -458,37 +473,26 @@
return updateMeta{}, "All components are uptodate"
}

switch configuredSelector {
case ytv1.UpdateSelectorEverything:
if spec.EnableFullUpdate {
if needFullUpdate {
return updateMeta{
flow: ytv1.UpdateFlowFull,
componentNames: nil,
}, ""
return updateMeta{flow: ytv1.UpdateFlowFull, componentNames: canUpdate}, ""
} else {
return updateMeta{
flow: ytv1.UpdateFlowStateless,
componentNames: canUpdate,
}, ""
}
case ytv1.UpdateSelectorMasterOnly:
return updateMeta{
flow: ytv1.UpdateFlowMaster,
componentNames: canUpdate,
}, ""
case ytv1.UpdateSelectorTabletNodesOnly:
return updateMeta{
flow: ytv1.UpdateFlowTabletNodes,
componentNames: canUpdate,
}, ""
case ytv1.UpdateSelectorDataNodesOnly, ytv1.UpdateSelectorExecNodesOnly, ytv1.UpdateSelectorStatelessOnly:
return updateMeta{
flow: ytv1.UpdateFlowStateless,
componentNames: canUpdate,
}, ""
default:
return updateMeta{}, fmt.Sprintf("Unexpected update selector %s", configuredSelector)
return updateMeta{flow: ytv1.UpdateFlowStateless, componentNames: canUpdate}, ""
}

}

Check failure on line 483 in controllers/sync.go

View workflow job for this annotation

GitHub Actions / Run checks

unnecessary trailing newline (whitespace)

if len(flows) == 0 {
return updateMeta{}, fmt.Sprintf("Unexpected state: no flows for components {%s}", strings.Join(canUpdate, ", "))
}

if len(flows) == 1 {
for flow := range flows {
return updateMeta{flow: flow, componentNames: canUpdate}, ""
}
}
// If more than one flow is possible, we choose to follow full update flow.
return updateMeta{flow: ytv1.UpdateFlowFull, componentNames: canUpdate}, ""
}

func (r *YtsaurusReconciler) Sync(ctx context.Context, resource *ytv1.Ytsaurus) (ctrl.Result, error) {
Expand Down
2 changes: 2 additions & 0 deletions controllers/ytsaurus_local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

ytv1 "github.com/ytsaurus/ytsaurus-k8s-operator/api/v1"
"github.com/ytsaurus/ytsaurus-k8s-operator/controllers"
"github.com/ytsaurus/ytsaurus-k8s-operator/pkg/consts"
"github.com/ytsaurus/ytsaurus-k8s-operator/pkg/testutil"
)

Expand Down Expand Up @@ -117,6 +118,7 @@ func TestYtsaurusUpdateStatelessComponent(t *testing.T) {
ytsaurusResource.Spec.Discovery.Image = &imageUpdated
t.Log("[ Updating discovery with disabled full update ]")
ytsaurusResource.Spec.EnableFullUpdate = false
ytsaurusResource.Spec.UpdateSelectors = []ytv1.ComponentUpdateSelector{{ComponentGroup: consts.ComponentGroupStateless}}
testutil.UpdateObject(h, &ytv1.Ytsaurus{}, &ytsaurusResource)

waitClusterState(h, ytv1.ClusterStateRunning)
Expand Down
21 changes: 20 additions & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,24 @@ _Appears in:_
| `imagePullSecrets` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#localobjectreference-v1-core) array_ | | | |


#### ComponentUpdateSelector







_Appears in:_
- [YtsaurusSpec](#ytsaurusspec)

| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `componentKind` _[ComponentType](#componenttype)_ | | | |
| `componentGroup` _[ComponentGroup](#componentgroup)_ | | | |
| `update` _boolean_ | | false | |


#### ControllerAgentsSpec


Expand Down Expand Up @@ -2191,7 +2209,8 @@ _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 MasterOnly DataNodesOnly TabletNodesOnly ExecNodesOnly StatelessOnly Everything] <br /> |
| `updateSelector` _[UpdateSelector](#updateselector)_ | Deprecated: UpdateSelector is an experimental field. Behaviour may change. | | |
| `updateSelectors` _[ComponentUpdateSelector](#componentupdateselector) array_ | Controls the components that should be updated during the update process. | | |
| `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)_ | | | |
Expand Down
8 changes: 8 additions & 0 deletions pkg/consts/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,11 @@ const (
ChytType ComponentType = "CHYT"
SpytType ComponentType = "SPYT"
)

type ComponentGroup string

const (
ComponentGroupStateless ComponentGroup = "Stateless"
ComponentGroupStateful ComponentGroup = "Stateful"
ComponentGroupEverything ComponentGroup = "Everything"
)
Loading
Loading