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

[openshift-base] statefulset pvc resize #2050

Merged
merged 13 commits into from
Jan 6, 2022

Conversation

maorfr
Copy link
Contributor

@maorfr maorfr commented Dec 16, 2021

related to https://issues.redhat.com/browse/ASIC-145

during the incident, we noticed that pods were being picked up by the newly applied statefulset without being restarted, including picking up an updated storage size.

this needs to be validated through docs or an experiment.

@geoberle
Copy link
Contributor

StatefulSet PVC resize

The “delete statefulset —cascade=orphaned and reapply statefulset with increased storage request”-strategy does not have any effect on the PVCs. The recreated StatefulSet just adopts existing Pods and existing PVCs and does not change them. The statefulset controller has some sort of volumeClaimTemplate conformance check for the pods owned by a statefulset but it does not incorporate sizing and access modes and instead checks only basic things like existence.

It is strange, that the deletion of the owned_pods did not happen (are we 100% sure about this?). As far as i remember, we resized the PVCs manually and Pods gain immediate access to the increased storage once the PVC has been resized (see next section)

PVC resize without Pod restart

TL;DR; expansion of gp2 backed in-use PVCs works on Openshift 4.8 (I did not test on older versions) without Pod or Container restarts. Resizing of EBS volumes is only supported once every 6 hours though.

The ExpandInUsePersistentVolumes feature gate is enabled since Kubernetes 1.15 by default and enables us to resize PVCs without Pod restarts if supported by the storage class. Luckily gp2 supports this.

Experiment: Resize a PVC used by a Pod owned by a StatefulSet

oc describe pvc (right after resize request)
FileSystemResizePending   True    Mon, 01 Jan 0001 00:00:00 +0000   Tue, 28 Dec 2021 07:34:41 +0100           Waiting for user to (re-)start a pod to finish file system resize of volume on node.

And after 1 minute

Normal  FileSystemResizeSuccessful  7s (x2 over 7h52m)  kubelet  MountVolume.NodeExpandVolume succeeded for volume "pvc-d75ffd88-e145-44aa-bc75-76eb850921ae"

No Pod restart was required even though the initial event indicated that.

oc describe pod
…
Normal  FileSystemResizeSuccessful  2m40s (x2 over 7h55m)  kubelet  MountVolume.NodeExpandVolume succeeded for volume "pvc-d75ffd88-e145-44aa-bc75-76eb850921ae"

Pod was not recreated and no containers within the pod have been restarted

Not directly related to this but good to know: an EBS volume resize can only be done once every 6 hours (see https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_ModifyVolume.html). If a gp2 PVC resize is started before that, oc describe pvc shows something like this:

Warning  VolumeResizeFailed  0s  volume_expand  (combined from similar events): error expanding volume "gerd/www-web-0" of plugin "kubernetes.io/aws-ebs": AWS modifyVolume failed for vol-06164ef6eae80f9ec with VolumeModificationRateExceeded: You've reached the maximum modification rate per volume limit. Wait at least 6 hours between modifications per EBS volume.

But good news: after the required time has passed, the PVC resize operation reconciles automatically.

Summary

If we want to enable qontract-reconcile to take care of PVC resizes in StatefulSets, we need to take care of resizing the actual PVCs in our code - the “statefulset-dance” is not going to help. BUT: there is an ongoing KEP-0661 to enable StatefulSets to directly take care of PVC resizing.

@maorfr maorfr force-pushed the sts-no-delete-pods branch from 1a05bef to 2f0bd4d Compare December 29, 2021 08:44
@maorfr maorfr changed the title [openshift-base] do not delete pods on forbidden statefulset updates [openshift-base] statefulset dance Dec 29, 2021
@maorfr maorfr changed the title [openshift-base] statefulset dance [openshift-base] statefulset pvc resize Dec 29, 2021
@maorfr
Copy link
Contributor Author

maorfr commented Dec 29, 2021

great investigation! implemented exactly what you have proposed.

reconcile/test/test_utils_oc.py Show resolved Hide resolved
reconcile/utils/oc.py Outdated Show resolved Hide resolved
reconcile/utils/oc.py Show resolved Hide resolved
@maorfr maorfr force-pushed the sts-no-delete-pods branch from 9920dc0 to 43c1797 Compare January 6, 2022 08:38
Copy link
Contributor

@geoberle geoberle left a comment

Choose a reason for hiding this comment

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

lgtm

@maorfr
Copy link
Contributor Author

maorfr commented Jan 6, 2022

as type hints are optional (and i am fulfilling my right to not use them), and the open comment is a nit, i'm going to merge this to get the functionality promoted.

@maorfr maorfr force-pushed the sts-no-delete-pods branch from 0eaaa21 to 4b35708 Compare January 6, 2022 11:16
@maorfr maorfr merged commit 6f04594 into app-sre:master Jan 6, 2022
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.

3 participants