Update ControllerModifyVolume Status API

Author: sunnylovestiramisu

keps/sig-storage/3751-volume-attributes-class/README.md

@@ -172,27 +172,46 @@ type PersistentVolumeClaimSpec struct {
   ...
 }
 
+// Add the PersistentVolumeClaimModifyVolume condition to existing PersistentVolumeClaimConditionType
+// Condition is used to document the last error controller sees
+const (
+  ...
+  // PersistentVolumeClaimModifyVolume - a user trigger modify volume of pvc has been started
+  PersistentVolumeClaimModifyVolume PersistentVolumeClaimConditionType = "ModifyVolume"
+  ...
+)
+
+
 // PersistentVolumeClaimStatus represents the status of PV claim
 type PersistentVolumeClaimStatus struct {
   ...
-  // The VolumeAttributesClassName string cannot be empty
-  VolumeAttributesClassName string
-  ModifyVolumeStatus *PersistentVolumeClaimModifyVolumeStatus
+  ModifyVolumeStatus ModifyVolumeStatus
   ...
 }
 
+// ModifyVolumeStatus represents the status of ControllerModifyVolume operation
+type ModifyVolumeStatus struct {
+  ActualVolumeAttributesClassName string
+  TargetVolumeAttributesClassName string
+  Status *PersistentVolumeClaimModifyVolumeStatus
+}
+
 // +enum
+// When a controller receives persistentvolume claim update with PersistentVolumeClaimModifyVolumeStatus for a resource
+// that it does not recognizes, then it should ignore that update and let other controllers
+// handle it.
 type PersistentVolumeClaimModifyVolumeStatus string
 
 const (
-  // When modify volume is complete, the empty string is set by modify volume controller.
-  PersistentVolumeClaimNoModifyVolumeInProgress PersistentVolumeClaimModifyVolumeStatus = ""
+  // State set when modify volume controller wants to modify the volume in control-plane
+  // but the VolumeAttributesClass does not exist.
+  PersistentVolumeClaimControllerModifyVolumePending PersistentVolumeClaimModifyVolumeStatus = "ControllerModifyVolumePending"
   // State set when modify volume controller starts modifying the volume in control-plane
+  // When the request has been rejected as invalid by the CSI driver. To resolve this error,
+  // the PersistentVolumeClaim needs to specify a valid VolumeAttributesClass.
   PersistentVolumeClaimControllerModifyVolumeInProgress PersistentVolumeClaimModifyVolumeStatus = "ControllerModifyVolumeInProgress"
   // State set when modify volume has failed in modify volume controller with a terminal error.
-  // Transient errors such as timeout should not set this status and should leave ModifyVolumeStatus
-  // unmodified, so as modify volume controller can resume the volume modification.
-  PersistentVolumeClaimControllerModifyVolumeFailed PersistentVolumeClaimModifyVolumeStatus = "ControllerModifyVolumeFailed"
+  PersistentVolumeClaimControllerModifyVolumeInfeasible PersistentVolumeClaimModifyVolumeStatus = "ControllerModifyVolumeInfeasible"
 )
 
 ```
@@ -208,30 +227,44 @@ The CSI create request will be extended to add mutable parameters. A new Control
 // ControllerServer is the server API for Controller service.
 type ControllerServer interface {
     ...
-    rpc ControllerModifyVolume (ModifyVolumeRequest)
-        returns (ModifyVolumeResponse) {
+    rpc ControllerModifyVolume (ControllerModifyVolumeRequest)
+        returns (ControllerModifyVolumeResponse) {
             option (alpha_method) = true;
         }
     ...
 }
 
-message ModifyVolumeRequest {
+message ControllerModifyVolumeRequest {
   // Contains identity information for the existing volume.
   // This field is REQUIRED.
-  string volume_id = 1
-  // This field is OPTIONAL.This allows the CO to specify the
-  // mutable parameters to apply. 
-  map<string, string> mutable_parameters = 2;
+  string volume_id = 1;
+
+  // Secrets required by plugin to complete modify volume request.
+  // This field is OPTIONAL. Refer to the `Secrets Requirements`
+  // section on how to use this field.
+  map<string, string> secrets = 2 [(csi_secret) = true];
+
+  // Plugin specific volume attributes to mutate, passed in as 
+  // opaque key-value pairs. 
+  // This field is REQUIRED. The Plugin is responsible for
+  // parsing and validating these parameters. COs will treat these
+  // as opaque. The CO SHOULD specify the intended values of all mutable
+  // parameters it intends to modify. SPs MUST NOT modify volumes based
+  // on the absence of keys, only keys that are specified should result
+  // in modifications to the volume.
+  map<string, string> mutable_parameters = 3;
 }
-message ModifyVolumeResponse {}
+message ControllerModifyVolumeResponse {}
 
 message CreateVolumeRequest {
   ...
   // See CreateVolumeRequest.parameters.
   // This field is OPTIONAL.
   map<string, string> parameters = 4;
-  // This field is OPTIONAL. This allows the CO to specify the
-  // volume attributes class parameters to apply. 
+  // Plugins MUST treat these 
+  // as if they take precedence over the parameters field.
+  // This field SHALL NOT be specified unless the SP has the
+  // MODIFY_VOLUME plugin capability.
   map<string, string> mutable_parameters = 8;
 }
 ```
@@ -275,6 +308,7 @@ apiVersion: storage.k8s.io/v1alpha1
 kind: VolumeAttributesClass
 metadata:
   name: silver
+driverName: pd.csi.storage.gke.io
 parameters:
   iops: "500"
   throughput: "50MiB/s"
@@ -318,6 +352,7 @@ apiVersion: storage.k8s.io/v1alpha1
 kind: VolumeAttributesClass
 metadata:
   name: silver
+driverName: pd.csi.storage.gke.io
 parameters:
   iops: "500"
   throughput: "50MiB/s"
@@ -345,6 +380,7 @@ apiVersion: storage.k8s.io/v1alpha1
 kind: VolumeAttributesClass
 metadata:
   name: gold
+driverName: pd.csi.storage.gke.io
 parameters:
   iops: "1000"
   throughput: "100MiB/s"
@@ -409,12 +445,25 @@ The resource quota controller is the only component capable of monitoring and re
 ### 3. Add new statuses in PVC API to indicate changes of VolumeAttributesClass and the status of the ModifyVolume operation.
 
 ```
-type VolumeAttributesClassStatus string
+type ModifyVolumeStatus struct {
+  ActualClassName string
+  TargetClassName string
+  Status *PersistentVolumeClaimModifyVolumeStatus
+}
+
+// +enum
+type PersistentVolumeClaimModifyVolumeStatus string
 
 const (
-    PersistentVolumeClaimControllerModifyVolumeProgress VolumeAttributesClassStatus = "ControllerModifyVolumeInProgress"
-    PersistentVolumeClaimControllerModifyVolumePending VolumeAttributesClassStatus = "ControllerModifyVolumePending"
-    PersistentVolumeClaimControllerModifyVolumeFailed VolumeAttributesClassStatus = "ControllerModifyVolumeFailed"
+  // State set when modify volume controller wants to modify the volume in control-plane
+  // but the VolumeAttributesClass does not exist.
+  PersistentVolumeClaimControllerModifyVolumePending PersistentVolumeClaimModifyVolumeStatus = "ControllerModifyVolumePending"
+  // State set when modify volume controller starts modifying the volume in control-plane
+  // When the request has been rejected as invalid by the CSI driver. To resolve this error,
+  // the PersistentVolumeClaim needs to specify a valid VolumeAttributesClass.
+  PersistentVolumeClaimControllerModifyVolumeInProgress PersistentVolumeClaimModifyVolumeStatus = "ControllerModifyVolumeInProgress"
+  // State set when modify volume has failed in modify volume controller with a terminal error.
+  PersistentVolumeClaimControllerModifyVolumeInfeasible PersistentVolumeClaimModifyVolumeStatus = "ControllerModifyVolumeInfeasible"
 )
 ```
 ### 4. Add new CSI API ControllerModifyVolume, when there is a change of VolumeAttributesClass in PVC, external-resizer triggers a ControllerModifyVolume operation against a CSI endpoint. A Controller Plugin MUST implement this RPC call if it has MODIFY_VOLUME capability. 
@@ -454,7 +503,7 @@ There are a few conditions that will trigger add/remove pvc finalizers in the Vo
 1. PVC created with a VolumeAttributesClass
 
     The **VACObjectInUseProtection admission controller**:
-    * Check if the VolumeAttributesClass exists. If not, the PVC will enter the PENDING state because we do not want to impose ordering on object creation
+    * Check if the VolumeAttributesClass exists. If not, the PVC will enter the INPROGRESS state because we do not want to impose ordering on object creation
     * Check if this VolumeAttributesClass already has a protection finalizer
     * Add the finalizer to the VolumeAttributesClass if there is none 
 2. PVC created with a VolumeAttributesClass being deleted
@@ -480,7 +529,7 @@ For unbound PVs referencing a VAC:
 
 1. Unbound PV created with a VolumeAttributesClass
     The **VACObjectInUseProtection admission controller**:
-    * Check if the VolumeAttributesClass exists. If not, the PV will enter the PENDING state because we do not want to impose ordering on object creation
+    * Check if the VolumeAttributesClass exists. If not, the PV will enter the INPROGRESS state because we do not want to impose ordering on object creation
     * Check if this VolumeAttributesClass already has a protection finalizer
     * Add the finalizer to the VolumeAttributesClass if there is none
 2. PV has a VolumeAttributesClass and this PV is deleted
@@ -496,6 +545,7 @@ apiVersion: storage.k8s.io/v1alpha1
 kind: VolumeAttributesClass
 metadata:
   name: silver
+driverName: pd.csi.storage.gke.io
 parameters:
   iops: "500"
   throughput: "50MiB/s"
@@ -569,6 +619,7 @@ apiVersion: storage.k8s.io/v1alpha1
 kind: VolumeAttributesClass
 metadata:
   name: gold
+driverName: pd.csi.storage.gke.io
 parameters:
   iops: "1000"
   throughput: "100MiB/s"
@@ -593,7 +644,7 @@ spec:
 
 ModifyVolume is only allowed on bound PVCs. Under the ModifyVolume call, it will pass in the mutable parameters and do the update operation based on the `VolumeAttributesClass` parameters.
 
-![ModifyVolume Flow Diagram](./VolumeAttributesClass-ModifyVolume-Flow.png)
+![ModifyVolume Flow Diagram](./VolumeAttributesClass-ModifyVolume-Flow-v2.png)
 
 ### Implementation & Handling Failure