Addressing some more comments

Author: kk-src

keps/sig-storage/1847-autoremove-statefulset-pvcs/README.md

@@ -98,12 +98,9 @@ tags, and then generate with `hack/update-toc.sh`.
   - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
   - [Version Skew Strategy](#version-skew-strategy)
 - [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
-  - [Scalability](#scalability)
-  - [Troubleshooting](#troubleshooting)
 - [Implementation History](#implementation-history)
 - [Drawbacks](#drawbacks)
 - [Alternatives](#alternatives)
-- [Infrastructure Needed (optional)](#infrastructure-needed-optional)
 <!-- /toc -->
 
 ## Release Signoff Checklist
@@ -187,7 +184,8 @@ demonstrate the interest in a KEP within the wider Kubernetes community.
 
 ### Goals
 
-Provide a feature to auto delete the PVCs created by StatefulSet.
+Provide a feature to auto delete the PVCs created by StatefulSet. 
+Ensure that the pod restarts due to non scale down events such as rolling update or node drain does not delete the PVC.
 
 <!--
 List the specific goals of the KEP.  What is it trying to achieve?  How will we
@@ -223,13 +221,12 @@ The following changes are required:
    * Retain - this is the default policy and is considered in cases where no policy is specified. 
        This would be the existing behaviour - when a StatefulSet is deleted, no action is taken with
        respect to the PVCs created by the StatefulSet.
-   * RemoveOnScaledown - Whenever a pod is deleted, the corresponding PVC is deleted as well. 
-       When a scale down occurs for the StatefulSet, the corresponding PVC is deleted. A scale up 
-       following a scale down, will wait till old PVC for the removed Pod is deleted and ensure 
+   * RemoveOnScaledown - When a pod is deleted on scale down, the corresponding PVC is deleted as well. 
+       A scale up following a scale down, will wait till old PVC for the removed Pod is deleted and ensure 
        that the PVC used is a freshly created one.
    * RemoveOnStatefulSetDeletion - PVCs corresponding to the StatefulSet are deleted when StatefulSet
        themselves get deleted.
-3. Add `patch` to the statefulset controller cluster role.
+3. Add `patch` to the statefulset controller rbac cluster role for `persistentvolumeclaims`.
 
 <!--
 This is where we get down to the specifics of what the proposal actually is.
@@ -252,7 +249,7 @@ bogged down.
 User environment is such at the content of the PVCs which are created automatically during StatefulSet 
 creation need not be retained after the StatefulSet is deleted. User also requires that the scale 
 up/down occurs in a fast manner, and leverages any previously existing auto created PVCs within the 
-life time of the StatefulSet. A provision needs to be provided for the user to auto-delete the PVCs 
+life time of the StatefulSet. An option needs to be provided for the user to auto-delete the PVCs 
 once the StatefulSet is deleted. 
 
 User would set the VolumeReclaimPolicy as 'RemoveOnStatefulSetDelete' which would ensure that 
@@ -306,20 +303,20 @@ will be added to the StatefulSet. This field represents the user indication on w
 associated PVCs can be deleted or not. If this field is not set, the default behaviour is 
 to leave the PVCs as is during the StatefulSet deletion.
 
-If `VolumeReclaimPolicy` is set to `RemoveOnScaledown` Pod is set as the owner of the PVCs created
-from the `VolumeClaimTemplates`. When a Pod is deleted, the PVC owned by the Pod is 
-also deleted. 
+If `VolumeReclaimPolicy` is set to `RemoveOnScaledown`, Pod is set as the owner of the PVCs created
+from the `VolumeClaimTemplates` just before the scale down is performed by the statefulset controller. 
+When a Pod is deleted, the PVC owned by the Pod is also deleted. 
+
+Current scaleset controller implementation ensures that the manually deleted pods are restored 
+before the scale down logic is run. This combined with the fact that the owner references are set 
+only before the scale down will ensure that manual deletions do not automatically delete the PVCs 
+in question.
 
 During scale-up, if a PVC has an OwnerRef that does not match the Pod, it 
 potentially indicates that the PVC is referred by the deleted Pod and is in the process of 
 getting deleted. Controller will exit the current reconcile loop and attempt to reconcile in the 
 next iteration. This avoids a race with PVC deletion.
 
-Current scaleset controller implementation ensures that the manually deleted pods are restored
-before the scale down logic is run. The Pod owner reference is only added to the PVC just 
-before the scaling down by the controller. This ensures that the manual deletions do not 
-automatically delete the PVCs in question.
-
 When `VolumeReclaimPolicy` is set to `RemoveOnStatefulSetDeletion` the owner reference in 
 PVC points to the StatefulSet. When a scale up or down occurs, the PVC would remain the same. 
 PVCs previously in use before scale down will be used again when the scale up occurs. The PVC deletion 

keps/sig-storage/1847-autoremove-statefulset-pvcs/kep.yaml

@@ -4,9 +4,9 @@ authors:
   - "@kk-src"
   - "@dsu-igeek"
   - "@mattcary"
-owning-sig: sig-storage
+owning-sig: sig-apps
 participating-sigs:
-  - sig-apps
+  - sig-storage
 status: provisional
 creation-date: 2020-06-04
 reviewers:
@@ -39,5 +39,5 @@ milestone:
 #    the feature come into action, hence not adding additional feature gate.
 
 # The following PRR answers are required at beta release
-metrics:
-  - existing metrics would be enough
+# metrics:
+# Currently no metrics is planned for alpha release.