-
Notifications
You must be signed in to change notification settings - Fork 806
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
CSI image version and deployment manifests updates #171
Conversation
Hi @dkoshkin. Thanks for your PR. I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
/retest |
9db3556
to
b3bd25f
Compare
There are a lot of details here, will do another around of review |
9a23e39
to
c043668
Compare
4faf09e
to
cbce7ce
Compare
63e0ca7
to
23aeccf
Compare
Ok @leakingtapan I think this is ready for a final review.
|
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.
Thx for the quick turn around. Minor comment
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dkoshkin, leakingtapan 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:
Approvers can indicate their approval by writing |
Is this a bug fix or adding new feature?
Fixes #158
What is this PR about? / Why do we need it?
This PR replaces the deprecated https://github.com/kubernetes-csi/driver-registrar with:
driver-registrar
is now replaced with 2 different images onecluster-driver-registrar
and onenode-driver-registrar
it no longer makes sense to have both of those pods in thecsi-node
DS asnode-driver-registrar
is a cluster level component and should register the driver once, ie in a single pod.To support this, this PR also merged the
attacher.yaml
andprovisioner.yaml
into a singlecontroller.yaml
StatefulSet
(this is also consistent how other CSI drivers including DOs and GCPs does it)I also went through all the RBAC policies and tested them to confirm there aren't any extra/missing ones.
quay.io/k8scsi/csi-attacher:v1.0-canary
andexternal-provisioner
images with a stable latest releasev1.0.1
- having version predictability is always good - Interested on everyone else's thoughts on this.priorityClassName
which requires the pods to run inkube-system
namespace.v1.13
uses CSI 1.0 and although this driver has a release https://github.com/kubernetes-sigs/aws-ebs-csi-driver/releases with support for CSI 0.3, that release has some bugs and users can still use that version for older k8s versions but they should be encouraged to use newer, more stable versions. - Interested on everyone else's thoughts on this.secret.yaml
file, this allows users to rely on EC2 instance roles and still runkubectl apply -f
without having to remove that file.What testing is done?
Locally ran e2e and sanity tests.