bhouse-nexthop opened a new pull request, #14609:
URL: https://github.com/apache/cloudstack/pull/14609
### 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.
- Start, stop and destroy keep going ahead without a state change from the
states where they did that on purpose:
- start: from Alert or Error, which is how such a cluster is brought
back;
- stop: from Created or Error;
- destroy: from Destroying, which is a retried destroy, also used by the
scanner and GC.
- **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.
- **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. While such a job is in
progress, the scanner recovers nothing, because the job could belong to any
cluster.
- **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)
- [x] 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
- [x] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [x] 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. While any is in progress, recovery waits.
- **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 destroys, or two concurrent starts of an Alert 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
