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

Add docs for preventing volume mode conversion #32673

Merged
merged 5 commits into from
Apr 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions content/en/docs/concepts/storage/volume-snapshots.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ spec:
driver: hostpath.csi.k8s.io
source:
volumeHandle: ee0cfb94-f8d4-11e9-b2d8-0242ac110002
sourceVolumeMode: Filesystem
Copy link
Contributor

Choose a reason for hiding this comment

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

It's possible that a user may be reading this doc but is using an older version of CRDs and controllers. So we need to add notes right next to this field to clarify how it can be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

Choose a reason for hiding this comment

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

@RaunakShah : This conversation was marked as resolved and I did not see @xing-yang 's comments actually addressed. Could you clarify if this was agreed upon to not be added eventually, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't it agreed that users need not be developers and therefore introducing version specific information was too much effort for them? I added a note in Converting the volume mode of a Snapshot that describes how a user can determine if their API supports this field.

volumeSnapshotClassName: csi-hostpath-snapclass
volumeSnapshotRef:
name: new-snapshot-test
Expand All @@ -141,13 +142,59 @@ spec:
driver: hostpath.csi.k8s.io
source:
snapshotHandle: 7bdd0de3-aaeb-11e8-9aae-0242ac110002
sourceVolumeMode: Filesystem
volumeSnapshotRef:
name: new-snapshot-test
namespace: default
```

`snapshotHandle` is the unique identifier of the volume snapshot created on the storage backend. This field is required for the pre-provisioned snapshots. It specifies the CSI snapshot id on the storage system that this `VolumeSnapshotContent` represents.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this addressed either, ptal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, I thought we agreed not to include release specific information in this doc.
I'm referring to these comments:

`sourceVolumeMode` is the mode of the volume whose snapshot is taken. The value
of the `sourceVolumeMode` field can be either `Filesystem` or `Block`. If the
source volume mode is not specified, Kubernetes treats the snapshot as if the
source volume's mode is unknown.

## Converting the volume mode of a Snapshot {#convert-volume-mode}
RaunakShah marked this conversation as resolved.
Show resolved Hide resolved

If the `VolumeSnapshots` API installed on your cluster supports the `sourceVolumeMode`
field, then the API has the capability to prevent unauthorized users from converting
the mode of a volume.

To check if your cluster has capability for this feature, run the following command:

```yaml
$ kubectl get crd volumesnapshotcontent -o yaml
```
Comment on lines +166 to +168
Copy link
Contributor

Choose a reason for hiding this comment

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


Copy link
Contributor

@sftim sftim Apr 22, 2022

Choose a reason for hiding this comment

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

After running kubectl get crd volumesnapshotcontent, does a success exit indicate support for the sourceVolumeMode field? If not, we should document how to actually check.

If you want to allow users to create a `PersistentVolumeClaim` from an existing
`VolumeSnapshot`, but with a different volume mode than the source, the annotation
`snapshot.storage.kubernetes.io/allowVolumeModeChange: "true"`needs to be added to
the `VolumeSnapshotContent` that corresponds to the `VolumeSnapshot`.

For pre-provisioned snapshots, `Spec.SourceVolumeMode` needs to be populated
by the cluster administrator.

An example `VolumeSnapshotContent` resource with this feature enabled would look like:

```yaml
apiVersion: snapshot.storage.k8s.io/v1
kind: VolumeSnapshotContent
metadata:
name: new-snapshot-content-test
annotations:
- snapshot.storage.kubernetes.io/allowVolumeModeChange: "true"
spec:
deletionPolicy: Delete
driver: hostpath.csi.k8s.io
source:
snapshotHandle: 7bdd0de3-aaeb-11e8-9aae-0242ac110002
sourceVolumeMode: Filesystem
volumeSnapshotRef:
name: new-snapshot-test
namespace: default
```

## Provisioning Volumes from Snapshots

You can provision a new volume, pre-populated with data from a snapshot, by using
Expand Down
14 changes: 14 additions & 0 deletions content/en/docs/reference/labels-annotations-taints/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,20 @@ you through the steps you follow to apply a seccomp profile to a Pod or to one o
its containers. That tutorial covers the supported mechanism for configuring seccomp in Kubernetes,
based on setting `securityContext` within the Pod's `.spec`.

### snapshot.storage.kubernetes.io/allowVolumeModeChange

Example: `snapshot.storage.kubernetes.io/allowVolumeModeChange: "true"`

Used on: VolumeSnapshotContent

Value can either be `true` or `false`.
This determines whether a user can modify the mode of the source volume when a
{{< glossary_tooltip text="PersistentVolumeClaim" term_id="persistent-volume-claim" >}} is being
created from a VolumeSnapshot.

Copy link
Contributor

@xing-yang xing-yang Apr 14, 2022

Choose a reason for hiding this comment

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

@sftim Does it make sense to only keep this brief section for labels-annotations-taints and refer to https://kubernetes-csi.github.io/docs/ for details? In that case, the doc link "/docs/concepts/storage/volume-snapshots/#convert-volume-mode" will be replaced with "https://kubernetes-csi.github.io/docs/".

Copy link
Contributor

Choose a reason for hiding this comment

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

@xing-yang : From a personal standpoint, I'd err on the side of too much information rather than too less.
In the spirit of what I just said, what we could potentially rewrite it as is suggested above.

Copy link
Contributor

Choose a reason for hiding this comment

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

Which specific page on https://kubernetes-csi.github.io/docs/ would we link to?

Refer to [Converting the volume mode of a Snapshot](/docs/concepts/storage/volume-snapshots/#convert-volume-mode)
and the [Kubernetes CSI Developer Documentation](https://kubernetes-csi.github.io/docs/) for more information.

## Annotations used for audit

<!-- sorted by annotation -->
Expand Down