Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

azuredisk-csi-driver: snapshots feature #443

Merged
merged 1 commit into from
Aug 18, 2020

Conversation

dkoshkin
Copy link
Contributor

What type of PR is this?

Chore

What this PR does/ why we need it:

Just like in AWS EBS CSI driver and GCP CSI driver enable the Snapshot feature by default.

The chart already has an option for this , here just changing the default.

Which issue(s) this PR fixes:

https://jira.d2iq.com/browse/D2IQ-70322

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

azuredisk-csi-driver: enable the Snapshot controller

Checklist

  • The commit message explains the changes and why are needed.
  • The code builds and passes lint/style checks locally.
  • The relevant subset of integration tests pass locally.
  • The core changes are covered by tests.
  • The documentation is updated where needed.

@dkoshkin dkoshkin added the ready label Aug 18, 2020
@dkoshkin dkoshkin requested a review from a team as a code owner August 18, 2020 15:38
@dkoshkin dkoshkin self-assigned this Aug 18, 2020
@dkoshkin
Copy link
Contributor Author

Tested by deploying a cluster with this branch

➜ kubectl api-resources | grep volumesnapshotclasses
volumesnapshotclasses                                                          snapshot.storage.k8s.io                        false        VolumeSnapshotClass

@dkoshkin dkoshkin force-pushed the dkoshkin-azure-csi-snapshots branch from dc4329e to 93c57e1 Compare August 18, 2020 18:21
@dkoshkin dkoshkin force-pushed the dkoshkin-azure-csi-snapshots branch from 93c57e1 to 3a7a80b Compare August 18, 2020 18:51
@dkoshkin dkoshkin requested a review from hectorj2f August 18, 2020 18:52
Copy link
Contributor

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

Keep in mind that this driver is one of the few addons left that are NOT tested automatically. This LGTM but let's make sure we put it through some manual testing before we merge.

@dkoshkin
Copy link
Contributor Author

Keep in mind that this driver is one of the few addons left that are NOT tested automatically. This LGTM but let's make sure we put it through some manual testing before we merge.

👍 yep tested locally with the latest changes

➜  dkkonvoyazure cat cluster.yaml | grep -B 1 configVersion
    - configRepository: https://github.com/mesosphere/kubernetes-base-addons
      configVersion: dkoshkin-azure-csi-snapshots
Kubernetes cluster and addons deployed successfully!

Run `konvoy apply kubeconfig` to update kubectl credentials.

Run `konvoy check` to verify that the cluster has reached a steady state and all deployments have finished.

Navigate to the URL below to access various services running in the cluster.
  https://52.247.16.196/ops/landing
And login using the credentials below.
  Username: elegant_feynman
  Password: gyyVa6B3rh5QrYpguuDorLby9PX9d24lPROfohMzYGFkLupUHRyKJ1E88YnyqLIV

If the cluster was recently created, the dashboard and services may take a few minutes to be accessible.
➜  dkkonvoyazure k api-resources | grep volumesnapshotclasses
volumesnapshotclasses

Copy link

@samba samba left a comment

Choose a reason for hiding this comment

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

LGTM, glad to see another step in parity between our supported cloud providers

@shaneutt shaneutt merged commit 14db6c5 into master Aug 18, 2020
@shaneutt shaneutt deleted the dkoshkin-azure-csi-snapshots branch August 18, 2020 21:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants