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.

Sign up for GitHub

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

Jump to bottom

AWS EBS CSI implementation #5549

Merged
merged 7 commits into from
Mar 25, 2020
Merged

AWS EBS CSI implementation #5549

merged 7 commits into from
Mar 25, 2020

Conversation

alijahnas
Copy link
Contributor

What type of PR is this?
This is the implementation of the AWS EBS CSI driver and it's storage class deployment.
/kind feature

What this PR does / why we need it:
The k8s in-tree volume provisioners are deprecated and are going to be removed from Kubernetes. Volume provisioning should now be done through CSI drivers.
This is the implementation of the AWS EBS CSI driver.

Special notes for your reviewer:
Still needs documentation that I can do as the one for Cinder CSI driver: https://github.com/kubernetes-sigs/kubespray/blob/master/docs/cinder-csi.md

Does this PR introduce a user-facing change?:
User needs to know that the old provisioner is deprecated and needs to be changed to the CSI one.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 16, 2020
@alijahnas
Copy link
Contributor Author

/assign @Miouge1
Hi Maxime, as I was talking to you about. The CSI driver for AWS EBS, to continue the transition to the out of tree volume provisioners.
If you can check it out some time. Thanks!

@Miouge1
Copy link
Contributor

Miouge1 commented Feb 26, 2020

Also nice if you can add yourself as an approver in roles/kubernetes-apps/csi_driver/aws_ebs/

@alijahnas
Copy link
Contributor Author

Also nice if you can add yourself as an approver in roles/kubernetes-apps/csi_driver/aws_ebs/

Ok, but how do I do that? >_<

@alijahnas alijahnas mentioned this pull request Mar 11, 2020
Merged
@Miouge1
Copy link
Contributor

Miouge1 commented Mar 11, 2020

Also nice if you can add yourself as an approver in roles/kubernetes-apps/csi_driver/aws_ebs/

Ok, but how do I do that? >_<

You can see an example of that in #5753

@alijahnas
Copy link
Contributor Author

@Miouge1 I made the necessary changes and tested on AWS that everything was working fine and that EBS volumes were created and assigned to pods when PVCs are created, then destroyed and removed from AWS when deleted.

@bl0m1 @StevenReitsma Can you please see if the code looks legit? As it touches some parts both of you wrote.

@huxcrux huxcrux mentioned this pull request Mar 13, 2020
Merged
huxcrux
huxcrux suggested changes Mar 13, 2020
View reviewed changes
Copy link

@huxcrux huxcrux left a comment

Choose a reason for hiding this comment

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

The EBS part looks good.
I have created a pr to make is easier to apply csi-drivers using a new tag. also I guess you accidentally put your name in the wrong section in the owners file

roles/kubernetes-apps/persistent_volumes/aws-ebs-csi/OWNERS Show resolved Hide resolved
roles/kubernetes-apps/meta/main.yml Show resolved Hide resolved
StevenReitsma
StevenReitsma approved these changes Mar 13, 2020
View reviewed changes
Copy link
Contributor

@StevenReitsma StevenReitsma left a comment

Choose a reason for hiding this comment

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

Haven't really checked the EBS code but the changes to the CSI download role look good to me!

@LuckySB
Copy link
Contributor

LuckySB commented Mar 13, 2020

It would be great if you could add some documentation.
as in cinder-csi.md

@alijahnas
Copy link
Contributor Author

It would be great if you could add some documentation.
as in cinder-csi.md

Documentation added 👍

@LuckySB
Copy link
Contributor

LuckySB commented Mar 17, 2020

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alijahnas, LuckySB

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 17, 2020
@alijahnas
Copy link
Contributor Author

@Miouge1 Hi, can you please lgtm this so that it is in? Thanks

Miouge1
Miouge1 approved these changes Mar 25, 2020
View reviewed changes
Copy link
Contributor

@Miouge1 Miouge1 left a comment

Choose a reason for hiding this comment

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

Well done @alijahnas !

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 25, 2020
@k8s-ci-robot k8s-ci-robot merged commit a8a05a2 into kubernetes-sigs:master Mar 25, 2020
@alijahnas alijahnas mentioned this pull request Mar 26, 2020
Merged
@alijahnas alijahnas deleted the aws_ebs_csi_driver branch March 26, 2020 08:09
@alijahnas
Copy link
Contributor Author

Thanks a lot @Miouge1
The Azure CSI driver is here: #5833

@alijahnas alijahnas mentioned this pull request Mar 30, 2020
Merged
LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Apr 9, 2020
@alijahnas @LuckySB
AWS EBS CSI implementation (kubernetes-sigs#5549)
3881efc 
* AWS EBS CSI implementation

* Fixing image repos

* Add OWNERS file

* Fix expressions

* Add csi-driver tag

* Add AWS EBS prefix to variables

* Add AWS EBS CSI Driver documentation
@Miouge1 Miouge1 mentioned this pull request Apr 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@huxcrux huxcrux huxcrux requested changes

@LuckySB LuckySB LuckySB requested changes

@Miouge1 Miouge1 Miouge1 approved these changes

@StevenReitsma StevenReitsma StevenReitsma approved these changes

@Atoms Atoms Awaiting requested review from Atoms

@holmsten holmsten Awaiting requested review from holmsten

Assignees

@Miouge1 Miouge1

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@alijahnas @Miouge1 @LuckySB @k8s-ci-robot @StevenReitsma @huxcrux