Deployment spec cleanup #364

verult · 2019-08-02T23:24:56Z

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

Deployment is now in a standalone namespace rather than default to isolate the driver from arbitrary workloads.
Removed unnecessary driver-registrar role. The events permission is unused
Bumping snapshotter version to latest
Changed resizer image to gcr.io
Added "gce-pd-csi-driver-node" and "gce-pd-csi-driver-controller" priorityClasses and set them in node DaemonSet and controller StatefulSet.
Changed the label key to "k8s-app" to follow the GKE convention that this deployment is a system app.
Fixed the name of the resizer RoleBinding

Fixes: #321
Fixes: #37
Fixes: #357

Deployment spec updates:
* The deployment is no longer in namespace `default`
* Changed "app" label key to "k8s-app"
* csi-snapshotter version has been changed to v1.2.0-gke.0
* The resizer role binding has been renamed to "csi-controller-resizer-binding"
* Removed driver-registrar role.

/assign @davidz627 @msau42

deploy/kubernetes/base/setup-cluster.yaml

deploy/kubernetes/base/kustomization.yaml

davidz627 · 2019-08-05T23:25:32Z

Integration tests are failing, driver deployment is failing

davidz627 · 2019-08-06T01:53:10Z

Also add: priorityClassName: system-cluster-critical and priorityClassName: system-node-critical to controller and node YAMLs respectively please
#357

Once you do that please add it to the "Fixed" section in your description so it auto-closes the issue

verult · 2019-08-07T23:03:52Z

Addressed comments

davidz627 · 2019-08-08T18:29:43Z

examples/kubernetes/demo-pod.yaml

@@ -1,17 +1,3 @@
-kind: PersistentVolumeClaim


why delete this?

Sorry, an artifact of manual testing stuff. Putting it back

davidz627 · 2019-08-08T18:30:08Z

deploy/kubernetes/base/kustomization.yaml

 namespace:
-  default
+  gce-pd-csi-driver


add to readme

I don't think the specific namespace name should be documented as it could become out of sync, but I'll mention that the deployment exists in a separate namespace

davidz627 · 2019-08-08T18:30:12Z

deploy/kubernetes/base/controller.yaml

@@ -14,6 +14,7 @@ spec:
        app: gcp-compute-persistent-disk-csi-driver
    spec:
      serviceAccountName: csi-controller-sa
+      priorityClassName: gce-pd-csi-driver-controller


add to readme

I don't think the specific priorityClassName should be documented as it could become out of sync, but I'll mention the behavior of this priorityclass

davidz627 · 2019-08-08T18:30:19Z

deploy/kubernetes/base/kustomization.yaml

@@ -1,7 +1,7 @@
 commonLabels:
-  app: gcp-compute-persistent-disk-csi-driver
+  k8s-app: gcp-compute-persistent-disk-csi-driver


add note to readme

I don't think this needs to be documented - it's easy for the entry to be out of date with documentation, and users typically don't have to interact with this label

…shotter version; added controller and node priorityclasses

verult · 2019-08-12T23:02:31Z

Addressed comments.

@davidz627 I think it'd be nice to expand upon the the driver deployment documentation a little more, maybe with an explanation of how the deployment is structured and what to run to deploy/delete the deployment. It should be done outside this PR, but if you think this is useful I can create an issue for it

davidz627 · 2019-08-12T23:16:46Z

It should be done outside this PR, but if you think this is useful I can create an issue for it

Please do. Make sure to capture specific points of difficulty or confusion. Thanks!

davidz627 · 2019-08-12T23:39:35Z

/lgtm
/approve
/cc @msau42
PTAL as well

k8s-ci-robot · 2019-08-12T23:39:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidz627, verult

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~~ [davidz627]

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

davidz627 · 2019-08-13T20:31:31Z

Looks like this PR broke resize. @verult could you look into that?

davidz627 · 2019-10-11T21:04:20Z

/kind cleanup

k8s-ci-robot assigned davidz627 and msau42 Aug 2, 2019

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 2, 2019

k8s-ci-robot requested review from jingxu97 and saad-ali August 2, 2019 23:25

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 2, 2019

davidz627 reviewed Aug 5, 2019

View reviewed changes

deploy/kubernetes/base/setup-cluster.yaml Outdated Show resolved Hide resolved

deploy/kubernetes/base/kustomization.yaml Outdated Show resolved Hide resolved

verult force-pushed the namespace-leaderelection branch from 96f212d to c93be82 Compare August 7, 2019 22:47

verult force-pushed the namespace-leaderelection branch from c93be82 to 1ab66e8 Compare August 7, 2019 23:01

verult changed the title ~~Deployment spec: added separate namespace; removed driver-registrar role; bumped snapshotter version; changed deployment label; added priorityClasses; fixed name of an object to align with convention~~ Deployment spec cleanup Aug 7, 2019

davidz627 reviewed Aug 8, 2019

View reviewed changes

Adding separate namespace; removed driver-registrar role; bumped snap…

4a8247d

…shotter version; added controller and node priorityclasses

verult force-pushed the namespace-leaderelection branch from 1ab66e8 to 4a8247d Compare August 12, 2019 22:58

k8s-ci-robot requested a review from msau42 August 12, 2019 23:39

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 12, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2019

k8s-ci-robot merged commit a383029 into kubernetes-sigs:master Aug 12, 2019

davidz627 mentioned this pull request Aug 14, 2019

Snapshotter failing to come up in dev overlay #374

Closed

verult mentioned this pull request Aug 20, 2019

Fix resize test failure #379

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deployment spec cleanup #364

Deployment spec cleanup #364

verult commented Aug 2, 2019 •

edited

Loading

davidz627 commented Aug 5, 2019

davidz627 commented Aug 6, 2019 •

edited

Loading

verult commented Aug 7, 2019

davidz627 Aug 8, 2019

verult Aug 12, 2019

davidz627 Aug 8, 2019

verult Aug 12, 2019

davidz627 Aug 8, 2019

verult Aug 12, 2019 •

edited

Loading

davidz627 Aug 8, 2019

verult Aug 12, 2019

verult commented Aug 12, 2019

davidz627 commented Aug 12, 2019

davidz627 commented Aug 12, 2019

k8s-ci-robot commented Aug 12, 2019

davidz627 commented Aug 13, 2019

davidz627 commented Oct 11, 2019

Deployment spec cleanup #364

Deployment spec cleanup #364

Conversation

verult commented Aug 2, 2019 • edited Loading

davidz627 commented Aug 5, 2019

davidz627 commented Aug 6, 2019 • edited Loading

verult commented Aug 7, 2019

davidz627 Aug 8, 2019

Choose a reason for hiding this comment

verult Aug 12, 2019

Choose a reason for hiding this comment

davidz627 Aug 8, 2019

Choose a reason for hiding this comment

verult Aug 12, 2019

Choose a reason for hiding this comment

davidz627 Aug 8, 2019

Choose a reason for hiding this comment

verult Aug 12, 2019 • edited Loading

Choose a reason for hiding this comment

davidz627 Aug 8, 2019

Choose a reason for hiding this comment

verult Aug 12, 2019

Choose a reason for hiding this comment

verult commented Aug 12, 2019

davidz627 commented Aug 12, 2019

davidz627 commented Aug 12, 2019

k8s-ci-robot commented Aug 12, 2019

davidz627 commented Aug 13, 2019

davidz627 commented Oct 11, 2019

verult commented Aug 2, 2019 •

edited

Loading

davidz627 commented Aug 6, 2019 •

edited

Loading

verult Aug 12, 2019 •

edited

Loading