Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Add AWS EBS and AzureDisk CSI Drivers #2875

Merged
merged 1 commit into from
Jul 15, 2022
Merged

Add AWS EBS and AzureDisk CSI Drivers #2875

merged 1 commit into from
Jul 15, 2022

Conversation

jeffwubj
Copy link
Contributor

@jeffwubj jeffwubj commented Jul 8, 2022 •
edited
Loading

Add AWS EBS CSI Driver and AzureDisk CSI Driver

What this PR does / why we need it

Add AWS EBS CSI Driver and AzureDisk CSI Driver for coming k8s 1.23.x release

Which issue(s) this PR fixes

Fixes #2889

Describe testing done for PR

  • Create AWS cluster, check ebs csi driver
  • Create Azure cluster, check azure disk driver
  • Check storageclass is using csi driver for 1.23.x, and using in-tree driver for 1.22.x and 1.21.x

Release note

Add AWS EBS CSI Driver and AzureDisk CSI Driver

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

  • After 1.23.x, AWS EBS and Azure Disk in-tree driver will retire
  • Install CSI drivers from Tanzu 1.6
  • Set default storage class to in-tree driver for 1.21.x or 1.22.x
  • Set default storage class to csi driver for 1.23.x (and later version)

@jeffwubj jeffwubj requested review from a team as code owners July 8, 2022 12:30
@github-actions
Copy link

github-actions bot commented Jul 8, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2875/20220708123919/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@jeffwubj jeffwubj force-pushed the topic/jeffwubj/support-ebs-azuredisk-csi-driver branch from 4612cc6 to 9de3f61 Compare July 9, 2022 06:52
@github-actions
Copy link

github-actions bot commented Jul 9, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2875/20220709070147/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@codecov
Copy link

codecov bot commented Jul 9, 2022 •
edited
Loading

Codecov Report

Merging #2875 (074d84c) into main (5e8076d) will increase coverage by 0.05%.
The diff coverage is 88.57%.

@@            Coverage Diff             @@
##             main    #2875      +/-   ##
==========================================
+ Coverage   43.72%   43.77%   +0.05%     
==========================================
  Files         415      415              
  Lines       41444    41479      +35     
==========================================
+ Hits        18120    18158      +38     
+ Misses      21620    21617       -3     
  Partials     1704     1704
Impacted Files Coverage Δ
pkg/v1/tkg/aws/client.go 20.33% <88.57%> (+6.23%) ⬆️
addons/controllers/clusterbootstrap_controller.go 63.54% <0.00%> (+0.61%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e8076d...074d84c. Read the comment docs.

@jeffwubj jeffwubj force-pushed the topic/jeffwubj/support-ebs-azuredisk-csi-driver branch from 9de3f61 to 94d165c Compare July 9, 2022 13:22
@github-actions
Copy link

github-actions bot commented Jul 9, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2875/20220709133204/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@jeffwubj jeffwubj force-pushed the topic/jeffwubj/support-ebs-azuredisk-csi-driver branch from 94d165c to e8e963b Compare July 11, 2022 03:38
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2875/20220711034712/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@jeffwubj jeffwubj requested review from pydctw and 12345lcr July 11, 2022 09:29
@jeffwubj jeffwubj force-pushed the topic/jeffwubj/support-ebs-azuredisk-csi-driver branch from e8e963b to 30f31b6 Compare July 12, 2022 01:22
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2875/20220712013341/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@jeffwubj jeffwubj force-pushed the topic/jeffwubj/support-ebs-azuredisk-csi-driver branch from 30f31b6 to d640249 Compare July 13, 2022 00:28
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2875/20220713044407/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@sedefsavas
Copy link
Contributor

sedefsavas commented Jul 13, 2022 •
edited
Loading

LGTM for the permissions added for EBS CSI as a workaround to upcoming fix.

@sedefsavas
Copy link
Contributor

cc @vijaykatam for reviewing overlays.

@jeffwubj jeffwubj force-pushed the topic/jeffwubj/support-ebs-azuredisk-csi-driver branch from d640249 to 074d84c Compare July 14, 2022 01:10
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2875/20220714012053/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

imikushin
imikushin reviewed Jul 15, 2022
View reviewed changes
pkg/v1/providers/ytt/lib/helpers.star
Comment on lines +348 to +351
if compare_semver_versions(tkrVersion, "v1.23.0") >= 0:
return True
end
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Could this be rewritten as the following?

Suggested change
if compare_semver_versions(tkrVersion, "v1.23.0") >= 0:
return True
end
return False
return compare_semver_versions(tkrVersion, "v1.23.0") >= 0

(not nitpicking, just curious)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks and Yes !
(as FC is soon, allow me to refactor it along with coming bug fix)

imikushin
imikushin approved these changes Jul 15, 2022
View reviewed changes
Copy link
Contributor

@imikushin imikushin left a comment

Choose a reason for hiding this comment

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

Looks good!

@imikushin imikushin added the ok-to-merge PRs should be labelled with this before merging label Jul 15, 2022
@jeffwubj jeffwubj merged commit 8bd97cb into vmware-tanzu:main Jul 15, 2022
@jeffwubj jeffwubj deleted the topic/jeffwubj/support-ebs-azuredisk-csi-driver branch October 31, 2022 07:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sedefsavas sedefsavas sedefsavas left review comments

@vijaykatam vijaykatam vijaykatam left review comments

@imikushin imikushin imikushin approved these changes

@pydctw pydctw Awaiting requested review from pydctw

@12345lcr 12345lcr Awaiting requested review from 12345lcr

Assignees
No one assigned
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support AWS EBS CSI Driver and Azure Disk CSI Driver
5 participants
@jeffwubj @sedefsavas @vijaykatam @imikushin @vmwclabot