Update snapshot docs for beta #260

xing-yang · 2020-01-09T04:44:21Z

This PR updates snapshot CSI docs for beta.

Note: The deployment section (https://github.com/kubernetes-csi/csi-driver-host-path#deployment) needs to be modified. That should be in a separate PR.

NONE

xing-yang · 2020-01-09T18:17:00Z

book/src/snapshot-restore-feature.md

+--|--|--|--|--|--
+Alpha | 1.12 | 1.12 | | 0.4+ | 0.4+
+Alpha | 1.13 | 1.16 | | 1.0+ | 1.0+
+Beta  | 1.17 | - | 2.0+ | 2.0+ | 1.5+


Should we remove this table from this section? This is redundant and also not consistent with the table in external-snapshotter.md.

xing-yang · 2020-01-09T18:19:33Z

/assign @msau42

msau42 · 2020-01-11T01:40:02Z

book/src/external-snapshotter.md


 Definitions of the min/max/recommended Kubernetes versions can be found on the
 [sidecar page](sidecar-containers.md#versioning)

+### Snapshot Controller


How about we have a new page just for the snapshot controller? I want to clearly distinguish between what belongs to a CSI driver and what belongs to a Kubernetes cluster.

Can we put the snapshot controller part in a new .md page? I want to have a separate section for "Kubernetes cluster controllers" vs CSI driver sidecars.

msau42 · 2020-01-11T01:40:20Z

book/src/external-snapshotter.md

-The deletion of a `VolumeSnapshot` object bound to a `VolumeSnapshotContent` corresponding to this driver with a `delete` reclaim policy causes the sidecar container to trigger a `DeleteSnapshot` operation against the specified CSI endpoint to delete the snapshot. Once the snapshot is successfully deleted, the sidecar container also deletes the `VolumeSnapshotContent` object representing the snapshot.
+The deletion of a `VolumeSnapshot` object bound to a `VolumeSnapshotContent` corresponding to this driver with a `delete` deletion policy causes the sidecar container to trigger a `DeleteSnapshot` operation against the specified CSI endpoint to delete the snapshot. Once the snapshot is successfully deleted, the sidecar container also deletes the `VolumeSnapshotContent` object representing the snapshot.
+
+### Snapshot Beta


Prefer putting the beta section first, then alpha

Can we organize this page to put all the beta sections together, then all the alpha sections together? Right now, you have to skip over many sections to get the complete picture for a single release

Yeah, that makes sense.

msau42 · 2020-01-11T01:41:42Z

book/src/external-snapshotter.md

+
+The snapshot controller will be updating the status field of the `VolumeSnapshot` object accordingly based on the status field of the `VolumeSnapshotContent` object to indicate the new snapshot is ready to be used.
+
+The deletion event of a `VolumeSnapshot` object bound to a `VolumeSnapshotContent` corresponding to this driver with a `delete` deletion policy causes the snapshot controller to start deleting the `VolumeSnapshotContent` object and add an annotation to the object to indicate it is being deleted. Note that both the `VolumeSnapshot` object and the `VolumeSnapshotContent` object will not be deleted immediately due to the finalizers. When the sidecar container detects this update on the `VolumeSnapshotContent` object, it triggers a `DeleteSnapshot` operation against the specified CSI endpoint to delete the snapshot. Once the snapshot is successfully deleted, the sidecar container removes the finalizer on the `VolumeSnapshotContent` object which leads to the deletion of the object from Kubernetes. The snapshot controller then removes the finalizer on the `VolumeSnapshot` object and as a result the object will be deleted from Kubernetes.


This is pretty detailed and getting into implementation/design territory. Can we put this into the design document and point that it from here instead?

msau42 · 2020-02-01T03:09:31Z

book/src/external-snapshotter.md


 Latest stable release | Branch | Min CSI Version | Max CSI Version | Container Image | Min K8s Version | Max K8s Version | Recommended K8s Version
 --|--|--|--|--|--|--|--
-[external-snapshotter v1.2.2](https://github.com/kubernetes-csi/external-snapshotter/releases/tag/v1.2.2) | [release-1.2](https://github.com/kubernetes-csi/external-snapshotter/tree/release-1.2) | [v1.0.0](https://github.com/container-storage-interface/spec/releases/tag/v1.0.0) | - | quay.io/k8scsi/csi-snapshotter:v1.2.2 | v1.13 | - | v1.14
-[external-snapshotter v0.4.1](https://github.com/kubernetes-csi/external-snapshotter/releases/tag/v0.4.1) | [release-0.4](https://github.com/kubernetes-csi/external-snapshotter/tree/release-0.4) | [v0.3.0](https://github.com/container-storage-interface/spec/releases/tag/v0.3.0) | [v0.3.0](https://github.com/container-storage-interface/spec/releases/tag/v0.3.0) | quay.io/k8scsi/csi-snapshotter:v0.4.1 | v1.10 | -v1.16 | v1.10
+[external-snapshotter v2.0.0](https://github.com/kubernetes-csi/external-snapshotter/releases/tag/v2.0.0) | [release-2.0](https://github.com/kubernetes-csi/external-snapshotter/tree/release-2.0) | [v1.0.0](https://github.com/container-storage-interface/spec/releases/tag/v1.0.0) | - | quay.io/k8scsi/csi-snapshotter:v2.0.0 | v1.17 | - | v1.17


We have 2.0.1 now?

msau42 · 2020-02-01T03:11:27Z

book/src/external-snapshotter.md


-The deletion of a `VolumeSnapshot` object bound to a `VolumeSnapshotContent` corresponding to this driver with a `delete` reclaim policy causes the sidecar container to trigger a `DeleteSnapshot` operation against the specified CSI endpoint to delete the snapshot. Once the snapshot is successfully deleted, the sidecar container also deletes the `VolumeSnapshotContent` object representing the snapshot.
+In the Beta version, the snapshot controller will be watching the Kubernetes API server for `VolumeSnapshot` and `VolumeSnapshotContent` CRD objects. The CSI `external-snapshotter` sidecar only watches the Kubernetes API server for `VolumeSnapshotContent` CRD objects.


Maybe also add a line saying "the sidecar is responsible for calling the CSI ControllerCreateSnapshot, Delete, and List calls"

msau42 · 2020-02-01T03:13:35Z

book/src/external-snapshotter.md

@@ -4,34 +4,61 @@

 **Git Repository:** [https://github.com/kubernetes-csi/external-snapshotter](https://github.com/kubernetes-csi/external-snapshotter)

-**Status:** Alpha
+**Status:** Beta


Should we also have information on which versions were Beta and which were Alpha?

msau42 · 2020-02-01T03:14:39Z

book/src/kubernetes-cluster-controllers.md

@@ -0,0 +1,7 @@
+# Kubernetes Cluster Controllers
+
+The Kubernetes cluster controllers should be bundled and deployed by the Kubernetes distributors as part of their Kubernetes cluster management process (independent of any CSI Driver).


maybe also mention that the controllers are responsible for managing snapshot objects and operations across multiple CSI drivers, so that's why they should not be deployed by the driver.

msau42 · 2020-02-01T03:14:56Z

book/src/snapshot-controller.md

+
+**Git Repository:** [https://github.com/kubernetes-csi/external-snapshotter](https://github.com/kubernetes-csi/external-snapshotter)
+
+**Status:** Beta


clarify which version is Beta

msau42 · 2020-02-01T03:15:23Z

book/src/snapshot-controller.md

+
+Latest stable release | Branch | Min CSI Version | Max CSI Version | Container Image | Min K8s Version | Max K8s Version | Recommended K8s Version
+--|--|--|--|--|--|--|--
+[external-snapshotter v2.0.0](https://github.com/kubernetes-csi/external-snapshotter/releases/tag/v2.0.0) | [release-2.0](https://github.com/kubernetes-csi/external-snapshotter/tree/release-2.0) | [v1.0.0](https://github.com/container-storage-interface/spec/releases/tag/v1.0.0) | - | quay.io/k8scsi/snapshot-controller:v2.0.0 | v1.17 | - | v1.17


Yes. updated.

msau42 · 2020-02-01T03:16:28Z

book/src/snapshot-controller.md

+
+## Description
+
+In the Beta version, the snapshot controller will be watching the Kubernetes API server for `VolumeSnapshot` and `VolumeSnapshotContent` CRD objects. The CSI `external-snapshotter` sidecar only watches the Kubernetes API server for `VolumeSnapshotContent` CRD objects.


Do we want to mention something about it creating VolumeSnapshotContent objects to actuate the sidecar for provisioning?

Sure. I added one line there. Details are in the snapshot KEP.

msau42 · 2020-02-01T03:17:22Z

book/src/snapshot-restore-feature.md

+Status | Min K8s Version | Max K8s Version | snapshot-controller Version | CSI external-snapshotter sidecar Version | external-provisioner Version
+--|--|--|--|--|--
+Alpha | 1.12 | 1.12 | | 0.4+ | 0.4+
+Alpha | 1.13 | 1.16 | | 1.0+ | 1.0+


These versions have upper bounds now

Should we just remove this table and refer to the tables in external-snapshotter.md and snapshot-controller.md? I found it very confusing to maintain tables with duplicate information in different files.

Ok, we probably still need this as this has everything together including the external-provisioner.

msau42 · 2020-02-01T03:18:48Z

book/src/snapshot-restore-feature.md


 ## Snapshot APIs

-Similar to the API for managing [Kubernetes Persistent Volumes](https://kubernetes.io/docs/concepts/storage/persistent-volumes/), the Kubernetes Volume Snapshots introduce three new API objects for managing snapshots: `VolumeSnapshot`, `VolumeSnapshotContent`, and `VolumeSnapshotClass`. See [Kubernetes Snapshot documentation](https://kubernetes.io/docs/concepts/storage/volume-snapshots/) for more details.
+### Alpha APIs


Put Beta API section first?

msau42 · 2020-02-01T03:19:10Z

book/src/snapshot-restore-feature.md

 ## Kubernetes Cluster Setup
-Since volume snapshot is an alpha feature in Kubernetes v1.12, you need to enable a new alpha feature gate called `VolumeSnapshotDataSource` in the Kubernetes master.
+
+### Snapshot Alpha


Maybe same thing as the other pages, have Beta, Alpha be the top level sections so all the details are together and you don't have to skip around to read all the information.

msau42 · 2020-02-06T02:33:36Z

book/src/snapshot-restore-feature.md

+
+Note that this could happen if the `PersistentVolumeClaim` spec and the `VolumeSnapshot` spec are in the same YAML file. In this case, when the `VolumeSnapshot` object is created, the `PersistentVolumeClaim` object is created but volume creation is not complete and therefore the `PersistentVolumeClaim` is not yet bound. You must wait until the `PersistentVolumeClaim` is bound and then create the snapshot.
+
+## Snapshot Beta


Can we put beta first before alpha?

msau42 · 2020-02-06T02:34:19Z

book/src/snapshot-restore-feature.md

+
+If your cluster does not come pre-installed with the correct components, you may manually install these components by executing the following steps.
+
+Install Snapshot Beta CRDs per cluster:


This is kind of repeated information. Can we link to snapshot-controller deployment page instead?

Yes, linked to snapshot-controller page now.

msau42 · 2020-02-07T20:55:22Z

book/src/snapshot-restore-feature.md

+
+#### PersistentVolumeClaim not Bound
+
+If a `PersistentVolumeClaim` is not bound, the attempt to create a volume snapshot from that `PersistentVolumeClaim` will fail. No retries will be attempted. An event will be logged to indicate that the `PersistentVolumeClaim` is not bound.


How come retries are not attempted? This seems counter to the general reconciliation principles in Kubernetes controllers.

This is because the exact time when the snapshot is taken is important for the applications. We have decided to change this behavior and keep retrying when working on the create snapshot timeout issue, but that work is not complete yet. So right now, this stays the same. Will change after we have fixed that issue.

Maybe add a reference to the issue saying that this behavior is planned to change

msau42 · 2020-02-10T17:10:52Z

/lgtm
/approve
Thanks!

k8s-ci-robot · 2020-02-10T17:11:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msau42, xing-yang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [msau42]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from msau42 and saad-ali January 9, 2020 04:44

xing-yang force-pushed the snapshot_beta branch from f4f3688 to 24f27b5 Compare January 9, 2020 17:44

xing-yang commented Jan 9, 2020

View reviewed changes

xing-yang changed the title ~~WIP: Update snapshot docs for beta~~ Update snapshot docs for beta Jan 9, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 9, 2020

k8s-ci-robot assigned msau42 Jan 9, 2020

msau42 reviewed Jan 11, 2020

View reviewed changes

msau42 mentioned this pull request Jan 24, 2020

fix broken links #265

Closed

xing-yang force-pushed the snapshot_beta branch from 24f27b5 to c977193 Compare January 25, 2020 15:10

msau42 reviewed Feb 1, 2020

View reviewed changes

xing-yang force-pushed the snapshot_beta branch 2 times, most recently from 082c533 to 9b76fbc Compare February 3, 2020 16:50

msau42 reviewed Feb 6, 2020

View reviewed changes

xing-yang force-pushed the snapshot_beta branch from 9b76fbc to 1a0f0cf Compare February 7, 2020 15:22

msau42 reviewed Feb 7, 2020

View reviewed changes

Update snapshot docs for beta

9d4011f

xing-yang force-pushed the snapshot_beta branch from 1a0f0cf to 9d4011f Compare February 8, 2020 02:35

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 10, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 10, 2020

k8s-ci-robot merged commit 1f7dce8 into kubernetes-csi:master Feb 10, 2020


		The snapshot controller will be updating the status field of the `VolumeSnapshot` object accordingly based on the status field of the `VolumeSnapshotContent` object to indicate the new snapshot is ready to be used.

		The deletion event of a `VolumeSnapshot` object bound to a `VolumeSnapshotContent` corresponding to this driver with a `delete` deletion policy causes the snapshot controller to start deleting the `VolumeSnapshotContent` object and add an annotation to the object to indicate it is being deleted. Note that both the `VolumeSnapshot` object and the `VolumeSnapshotContent` object will not be deleted immediately due to the finalizers. When the sidecar container detects this update on the `VolumeSnapshotContent` object, it triggers a `DeleteSnapshot` operation against the specified CSI endpoint to delete the snapshot. Once the snapshot is successfully deleted, the sidecar container removes the finalizer on the `VolumeSnapshotContent` object which leads to the deletion of the object from Kubernetes. The snapshot controller then removes the finalizer on the `VolumeSnapshot` object and as a result the object will be deleted from Kubernetes.


		The deletion of a `VolumeSnapshot` object bound to a `VolumeSnapshotContent` corresponding to this driver with a `delete` reclaim policy causes the sidecar container to trigger a `DeleteSnapshot` operation against the specified CSI endpoint to delete the snapshot. Once the snapshot is successfully deleted, the sidecar container also deletes the `VolumeSnapshotContent` object representing the snapshot.
		In the Beta version, the snapshot controller will be watching the Kubernetes API server for `VolumeSnapshot` and `VolumeSnapshotContent` CRD objects. The CSI `external-snapshotter` sidecar only watches the Kubernetes API server for `VolumeSnapshotContent` CRD objects.

		@@ -0,0 +1,7 @@
		# Kubernetes Cluster Controllers

		The Kubernetes cluster controllers should be bundled and deployed by the Kubernetes distributors as part of their Kubernetes cluster management process (independent of any CSI Driver).


		Git Repository: [https://github.com/kubernetes-csi/external-snapshotter](https://github.com/kubernetes-csi/external-snapshotter)

		Status: Beta


		## Description

		In the Beta version, the snapshot controller will be watching the Kubernetes API server for `VolumeSnapshot` and `VolumeSnapshotContent` CRD objects. The CSI `external-snapshotter` sidecar only watches the Kubernetes API server for `VolumeSnapshotContent` CRD objects.


		Note that this could happen if the `PersistentVolumeClaim` spec and the `VolumeSnapshot` spec are in the same YAML file. In this case, when the `VolumeSnapshot` object is created, the `PersistentVolumeClaim` object is created but volume creation is not complete and therefore the `PersistentVolumeClaim` is not yet bound. You must wait until the `PersistentVolumeClaim` is bound and then create the snapshot.

		## Snapshot Beta


		If your cluster does not come pre-installed with the correct components, you may manually install these components by executing the following steps.

		Install Snapshot Beta CRDs per cluster:


		#### PersistentVolumeClaim not Bound

		If a `PersistentVolumeClaim` is not bound, the attempt to create a volume snapshot from that `PersistentVolumeClaim` will fail. No retries will be attempted. An event will be logged to indicate that the `PersistentVolumeClaim` is not bound.

Update snapshot docs for beta #260

Update snapshot docs for beta #260

Uh oh!

Conversation

xing-yang commented Jan 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xing-yang commented Jan 9, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

xing-yang commented Jan 9, 2020 •

edited

Loading