[bhouse-nexthop]

Description

Two related fixes to how a CKS cluster's state is kept.

1. Operations on the same cluster can no longer run at the same time and clobber each other's state.
2. A cluster left in an operation's state by a management server that stopped is recovered, instead of staying stuck until the database is edited.

Fixes #14608

Concurrent operations

KubernetesClusterDaoImpl.updateState wrote the state by id alone, so two operations on one cluster both got past their first transition. The workers also ignored a failed entry transition, and carried on.

• Compare-and-set (KubernetesClusterDaoImpl.updateState): the update now has WHERE id = ? AND state = ? and reports whether a row changed, as VolumeDaoImpl and VMInstanceDaoImpl already do.
• Entering an operation (KubernetesClusterActionWorker.transitToOperationState): each worker moves the cluster into its operation's state before it changes anything. If another operation got there first, the worker fails with a "please retry" message. It does not move the cluster, and does not undo anything the other operation may need, such as the template launch permission.
  • From a state with no transition for the event, the operation is refused. Before, it went ahead without a state change.
  • Starting a cluster in Alert now takes it to Starting (new Alert -StartRequested-> Starting transition), like any other start. Before, the start ran without a state, so adding or removing nodes, a stop, or the scanner's Alert reconciliation could take the cluster partway through.
  • A few operations keep going ahead without a state change, where they did that on purpose:
    • start from Error, so that a cluster whose create failed stays in Error if the start fails too, rather than going to Alert, where the reconciliation could mark it Running without the provider and CSI driver;
    • stop from Created or Error;
    • destroy from Destroying, which is a retried destroy, also used by the scanner and GC.
• Holding the state: once a worker has started its operation, it only moves the cluster while the cluster is in a state the operation moved it to (KubernetesClusterActionWorker.stateTransitTo). So a failure or completion never ends another operation that has taken the cluster since. An operation that went ahead without a state change leaves the state alone when it ends, except a retried destroy, which completes Destroying.
• Scaling (KubernetesClusterResourceModifierActionWorker.enterScalingState): an operation enters Scaling or ScalingStoppedCluster once, at its first scaling step. A scale or autoscale of a Created cluster still only changes the stored settings, decided from the state the worker started from.
  • A scale of a Created cluster fails if a start has taken the cluster since the request was accepted. Created has no scaling transition to compare and set, so a narrow window remains there.
  • A scaling only fails or completes the state when it was the one that moved the cluster. Before, a validation failure in one scale request could move a concurrent scale's Scaling to Alert.
  • A failure in ScalingStoppedCluster now goes to Alert, as the state machine defines. Before, it was left in ScalingStoppedCluster.
  • A failed autoscaler deployment now fails the Scaling state. Before, the cluster was left in Scaling.
• Adding nodes: the cluster moves to Importing before the binaries ISO is attached, not after, so an add that is refused leaves nothing attached. On failure, the cluster is only failed if this add moved it to Importing and it is still there. Before, a failure ended whatever operation held the cluster.
• Status scanner:
  • The Alert-to-Stopped reconciliation now moves the cluster as it was listed. It no longer stops a cluster a user has started since.
  • reconcileAlertCluster only completes a recovery that it started itself.

Stuck states

Before this change, if a management server stopped during an operation, the async job was failed when the server came back or a peer saw it leave. The cluster stayed in Scaling, Upgrading, Starting and so on, and every later operation was refused.

• Recovery (KubernetesClusterManagerImpl.recoverOrphanedOperationStates, run by the status scanner on every pass): it covers a cluster in Starting, Stopping, Scaling, ScalingStoppedCluster, Upgrading, Importing, RemovingNodes or Recovering that has no IN_PROGRESS job on any management server. The scanner fails its operation; Upgrading, for example, goes to Alert. The existing Alert reconciliation then returns a healthy cluster to Running.
  • A Starting cluster goes to Alert. A create that fails while waiting for kubeconfig or the dashboard already does the same.
  • The cluster is read just before its job is checked, and the update is a compare-and-set on the state that was read. An operation that ends in between moves the cluster on, so the update misses.
• Finding the jobs: the async jobs of start, stop, scale and upgrade now record the cluster they act on (getApiResourceId). Create, delete and add/remove nodes already did. This also fills in the resource on those commands' events.
• Mixed versions during an upgrade: a management server of an older version does not record the cluster on these jobs, but the job's parameters name the cluster by its UUID. The scanner leaves alone any cluster named by such a job that is in progress. It leaves every cluster alone while such a job has no parameters.
• Replaced code: the scanner's first-run handling of Starting clusters. For running VMs it tried FaultsDetected, which has no transition from Starting.

No schema or API change. The binaries ISO that an interrupted operation attached is left on the VMs. The next operation's attach replaces it.

Relation to #14604

#14604 lets a cluster in Alert be scaled. It adds Alert -> Scaling transitions, which enterScalingState uses as they are. Its own check of the state when the scale worker starts still applies. This PR closes the narrow window between that check and the transition, which #14604 mentions. The two branches merge with one trivial conflict in KubernetesClusterManagerImplTest, where both add tests at the end.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• Build/CI
• Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

How Has This Been Tested?

• mvn -pl plugins/integrations/kubernetes-service verify passes: all of the plugin's unit tests, plus checkstyle.
• New tests:
  • KubernetesClusterDaoImplTest: the update is restricted to the current state, and returns false when no row changed.
  • KubernetesClusterActionWorkerTest:
    • transitToOperationState moves the cluster;
    • it fails when another operation moved the cluster first;
    • it fails in a state with no transition, unless that state is one where the operation goes ahead without one;
    • failOperationIfStillIn leaves a cluster that has moved on.
  • KubernetesClusterResourceModifierActionWorkerTest:
    • enterScalingState transitions once for all steps of an operation;
    • it fails when another operation got there first;
    • it leaves a never-started cluster in its state;
    • it fails for one that a start has taken since.
  • KubernetesClusterManagerImplTest:
    • recovery fails only operations with no job;
    • it waits while a job for an unknown cluster is in progress;
    • it skips the job lookup when no cluster is in an operation's state;
    • it leaves a cluster whose operation ended, or that gained a job since the scan;
    • it sends a Starting cluster to Alert.
• Not tested on a live cluster. I have not run concurrent operations, or restarted a management server mid-operation, against a real CKS cluster. The behaviour is covered by reading the code and by the unit tests above.

How did you try to break this feature and the system with this change?

• Two operations at once: only one gets past its first transition. The other fails without touching the state. A scale that fails validation before entering Scaling no longer fails another scale's Scaling.
• An operation that ends while the scanner looks at it: the scanner reads the cluster, then its job, then makes a compare-and-set on the state it read. An operation that ends in between moves the cluster on, so the update misses. To hit a live operation, one operation would have to end, and another start and reach the same state, between those reads and the update right after them.
• Job cleanup: recovery trusts that a running operation has an IN_PROGRESS job. The async job GC expunges unfinished jobs older than job.expire.minutes, which is 1440 by default. If that setting is lowered below the length of a long operation, recovery can fail the operation while it is still running.
• Rolling upgrade of management servers: jobs started by older servers carry no cluster id. Recovery leaves alone the clusters they name.
• Behaviour that changes on purpose: add or remove nodes, upgrade, scale and autoscale used to go ahead, without a state change, from states that do not allow them. They are now refused. Examples are adding nodes to a Stopped cluster, and stopping or destroying a cluster that is in the middle of a scale or upgrade. These operations used to run alongside the operation in progress.
• Not covered here, unchanged by this PR:
  • A stop can still take over a Starting cluster: the state machine allows Starting -> Stopping.
  • Two concurrent destroy retries, or two concurrent starts of an Error cluster, still both go ahead, as before.
  • A create interrupted in its last steps, after the dashboard check, is recovered to Alert, and the Alert reconciliation can then mark it Running without the provider and CSI driver. This is the same window in which a create that fails already ends in Alert.
  • An upgrade interrupted by a management server stop is recovered to Alert, and then Running, with some nodes upgraded. The cluster's version is unchanged, so the upgrade can be run again.

[codecov]

Codecov Report

❌ Patch coverage is 51.46199% with 83 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.34%. Comparing base (d02adcd) to head (fee9834).

Files with missing lines	Patch %	Lines
...r/actionworkers/KubernetesClusterActionWorker.java	67.39%	9 Missing and 6 partials ⚠️
...bernetes/cluster/KubernetesClusterManagerImpl.java	71.05%	9 Missing and 2 partials ⚠️
...er/actionworkers/KubernetesClusterScaleWorker.java	0.00%	11 Missing ⚠️
...bernetes/cluster/dao/KubernetesClusterDaoImpl.java	62.06%	10 Missing and 1 partial ⚠️
...KubernetesClusterResourceModifierActionWorker.java	55.00%	7 Missing and 2 partials ⚠️
...ster/actionworkers/KubernetesClusterAddWorker.java	0.00%	5 Missing ⚠️
...er/actionworkers/KubernetesClusterStartWorker.java	0.00%	4 Missing ⚠️
.../kubernetes/cluster/ScaleKubernetesClusterCmd.java	0.00%	3 Missing ⚠️
.../kubernetes/cluster/StartKubernetesClusterCmd.java	0.00%	3 Missing ⚠️
...r/kubernetes/cluster/StopKubernetesClusterCmd.java	0.00%	3 Missing ⚠️
... and 5 more

Additional details and impacted files

@@             Coverage Diff              @@
##               4.22   #14609      +/-   ##
============================================
+ Coverage     18.32%   18.34%   +0.01%     
- Complexity    16644    16665      +21     
============================================
  Files          5939     5939              
  Lines        537768   537892     +124     
  Branches      65929    65950      +21     
============================================
+ Hits          98551    98676     +125     
+ Misses       428008   427995      -13     
- Partials      11209    11221      +12     

Flag	Coverage Δ
uitests	4.08% <ø> (ø)
unittests	19.43% <51.46%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bhouse-nexthop]

Updated the commit (now fee9834af4) after another review pass:

• Workers hold their state. Once a worker has started its operation, it only moves the cluster while the cluster is in a state that operation moved it to. A failure or completion therefore no longer ends another operation that has taken the cluster since. An operation that went ahead without a state change (stop from Created or Error, start from Error) leaves the state alone when it ends. Before, a stop of an Error cluster racing a delete could move Destroying to Destroyed. A retried destroy still completes Destroying.
• Start from Alert takes the cluster. New Alert -StartRequested-> Starting transition. So adding or removing nodes, a stop, or the scanner's Alert reconciliation can no longer take the cluster partway through a start. A start from Error still goes ahead without a state, so that a cluster whose create failed stays in Error if the start fails too.
• A failed autoscaler deployment now fails the Scaling state. Before, the cluster was left in Scaling.
• Restoring the cluster object after a missed update. KubernetesClusterDaoImpl.updateState now puts back the state the caller read when the conditional update matches no row. Before, the object reported the next state, which the database never had.
• Older management servers only hold back the clusters they name. A job in progress on a management server of an older version only stops recovery of the cluster that its parameters name by UUID, not of every cluster. Recovery also lists the clusters with one query instead of one per state.

KubernetesClusterResourceModifierActionWorker keeps its own copy of the start state, rather than reusing KubernetesClusterScaleWorker.originalState, to avoid conflicting with #14604, which edits those lines.

mvn -pl api,plugins/integrations/kubernetes-service install passes (100 plugin unit tests, checkstyle). Still not tested on a live cluster.