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

StatefulSet Support for Updating Volume Claim Template #1

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

huww98
Copy link
Owner

@huww98 huww98 commented May 18, 2024

  • One-line PR description:
  • Issue link:
  • Other comments:

@huww98 huww98 force-pushed the sts-update-claim branch from 565444d to df261df Compare May 19, 2024 09:44
@huww98 huww98 force-pushed the sts-update-claim branch from a40a3f1 to a5e98fa Compare May 19, 2024 11:21
e.g.:
- If `spec.updateStrategy.type` is `RollingUpdate`,
update the PVCs in the order from the largest ordinal to the smallest.
Only proceed to the next ordinal when all the PVCs of the previous ordinal

Choose a reason for hiding this comment

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

Does just being compatible mean we can move on to the next ordinal? Shouldn't the resizing be finished?

Copy link
Owner Author

Choose a reason for hiding this comment

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

By saying compatible, we are already waiting for the resize to finish. Because in "What PVC is compatible", status.capacity.storage should be updated before a PVC becomes compatible.

- totalCapacity: the sum of `status.capacity` of all the PVCs.

Some fields in the `status` are also updated to reflect the staus of the PVCs:
- readyReplicas: in addition to pods, also consider the PVCs status. A PVC is not ready if:

Choose a reason for hiding this comment

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

If strategy is ondelete, and old pvc works, the corresponding pod is ready or not?

Copy link
Owner Author

Choose a reason for hiding this comment

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

if volumeClaimUpdateStrategy is OnDelete, the PVC is always considered ready. This is required to not break the current behavior.

(`spec.resources.requests.storage`, `spec.volumeAttributesClassName`, `metadata.labels`, and `metadata.annotations` currently),
update the PVC in-place to the extent possible.
Do not perform the update that will be rejected by API server, such as
decreasing the storage size below its current status.

Choose a reason for hiding this comment

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

Change API server to allow any updates to volumeClaimTemplate of a StatefulSet.

I'm confused. Will decreasing the storage size be supported in this proposal?

Copy link
Owner Author

@huww98 huww98 May 22, 2024

Choose a reason for hiding this comment

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

Decreasing in template is supported, but only for the purpose of recovering from failed expansion, or as template for new PVCs. Shrink the PV/PVC is not supported.

We accept any update to the template, but decreasing the size will not affect the existing healthy PVC.

- `volumeClaimUpdateStrategy` is `InPlace` and the PVC is updating;
- availableReplicas: total number of replicas of which both Pod and PVCs are ready for at least `minReadySeconds`
- currentRevision, updateRevision, currentReplicas, updatedReplicas
are updated to reflect the status of PVCs.

Choose a reason for hiding this comment

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

When expanding the size of pvcs and then kubectl rollout undo, actually pvcs will not be rollbacked, they will stay the expanded size. Right?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It depends. If you enabled feature gate RecoverVolumeExpansionFailure, and the PVC pvc.Status.AllocatedResources is lower, then PVC is rolled back, and expansion is cancelled.

This is what I mean by writing "update the PVC in-place to the extent possible". Maybe I should make this more explicit.


#### Story 5: Asymmetric Replicas

The replicas of our StatefulSet are not identical, so we still want to update

Choose a reason for hiding this comment

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

Suggested change
The replicas of our StatefulSet are not identical, so we still want to update
The templates for different replicas of a StatefulSet are not identical, so we still want to update

Copy link
Owner Author

Choose a reason for hiding this comment

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

I would like the word "template" to specifically refer to the one written in StatefulSet. So I don't like "templates for different replicas". Maybe "The storage requirement of different replicas"?

Choose a reason for hiding this comment

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

that's fine.


We're running a CI/CD system and the end-to-end automation is desired.
To expand the volumes managed by a StatefulSet,
we can just use the same pipeline that we are already using to updating the Pod.

Choose a reason for hiding this comment

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

Suggested change
we can just use the same pipeline that we are already using to updating the Pod.
we can just use the same pipeline that we are already using to update the Pod.

2. Introduce a new field in StatefulSet `spec.updateStrategy.rollingUpdate`: `volumeClaimSyncStrategy`
to specify how to update PVCs and Pods. Possible values are:
- `Async`: the default value, preseve the current behavior.
- `LockStep`: update PVCs first, then update Pods. See below for details.

Choose a reason for hiding this comment

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

I don't understand why it's called LockStep. I came up with a few names for you to consider. Stepwise, Serial, Ordered, Sync.

Copy link
Owner Author

Choose a reason for hiding this comment

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

kubernetes#3412 (comment)

See here for the context of word lock-step.

Choose a reason for hiding this comment

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

okay

@huww98 huww98 force-pushed the sts-update-claim branch from 294de45 to 9ef734c Compare May 27, 2024 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants