-
Notifications
You must be signed in to change notification settings - Fork 108
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
Support v1beta1 snapshots #322
Conversation
07ee849
to
ede7d28
Compare
c2dbe5f
to
7845c3f
Compare
4ea5966
to
f3e7c9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, one small nit. But, I can see that the changes reflect the intended goal as described in the PR description.
README.md
Outdated
--- | ||
**Note:** | ||
|
||
The [DigitalOcean Kubernetes](https://www.digitalocean.com/products/kubernetes/) products comes with the CSI driver pre-installed and no further steps are required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: products product
kind: VolumeSnapshot | ||
metadata: | ||
name: csi-do-test-snapshot | ||
spec: | ||
source: | ||
name: csi-do-test-pvc | ||
kind: PersistentVolumeClaim | ||
persistentVolumeClaimName: csi-do-test-pvc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
metadata: | ||
name: do-block-storage | ||
namespace: kube-system |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this no longer in the kube-system namespace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VolumeSnapshotClass
never really was in a namespace because the object is non-namespaced. I removed it to make things more clear.
54a8cbb
to
4c6f8e6
Compare
This change adds support for v1beta1 snapshots. The change is breaking, meaning that v1alpha1 snapshots cannot be used anymore. Consequently, we also stop testing for Kubernetes releases below 1.17 which do did not support v1beta1 snapshots. Changes in detail: - Update manifests, including the addition of the new snapshot controller and update of the csi-snapshotter and csi-provisioner sidecars. - Split up release manifests into multiple files to limit kustomize usage to manifests that require modification for testing. In particular, the snapshot controller is excluded which does not support running in a dev and non-dev version concurrently. - Update examples. - Update CI and build references to Kubernetes releases. - Remove VolumeSnapshotDataSource feature gate which is enabled by default since 1.17. - Remove integration tests that were needed for Kubernetes <1.14 only. - Extend documentation.
4c6f8e6
to
f37cdb1
Compare
This change adds support for v1beta1 snapshots. The change is breaking, meaning that v1alpha1 snapshots cannot be used anymore. Consequently, we also stop testing for Kubernetes releases below 1.17 which do did not support v1beta1 snapshots.
Changes in detail:
kustomize
usage to manifests that require modification for testing. In particular, the snapshot controller is excluded which does not support running in a dev and non-dev version concurrently.VolumeSnapshotDataSource
feature gate which is enabled by default since 1.17.