Update sidecars to newer version #707

AndyXiangLi · 2021-01-27T18:58:17Z

Is this a bug fix or adding new feature?
Fixes #604
What is this PR about? / Why do we need it?
Continue from PR #596 and update sidecar version in both helm and kustomization
** What test has been done
Validate sidecar functionality in ARM node
Validate sidecar functionality in AMD node
Kube version: 1.18

k8s-ci-robot · 2021-01-27T18:58:25Z

Hi @AndyXiangLi. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

wongma7 · 2021-01-27T19:19:04Z

/ok-to-test

coveralls · 2021-01-27T22:47:46Z

Pull Request Test Coverage Report for Build 1506

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 81.196%

Totals
Change from base Build 1486:	0.0%
Covered Lines:	1697
Relevant Lines:	2090

💛 - Coveralls

AndyXiangLi · 2021-01-28T18:15:46Z

/test pull-aws-ebs-csi-driver-e2e-single-az

AndyXiangLi · 2021-01-28T18:23:09Z

/test pull-aws-ebs-csi-driver-e2e-single-az

AndyXiangLi · 2021-01-28T21:38:35Z

/test pull-aws-ebs-csi-driver-e2e-single-az

wongma7 · 2021-01-28T23:00:04Z

charts/aws-ebs-csi-driver/Chart.yaml

@@ -3,7 +3,7 @@ appVersion: "0.8.1"
 name: aws-ebs-csi-driver
 description: A Helm chart for AWS EBS CSI Driver
 version: 0.8.2


the version should also be bumped as technicaslly this is a new helm chart with new YAMLs

It's fine if the helm chart version goes out of sync with the driver appVersion (it already is)

Hi Matthew, should we bump the driver version to 0.9.0?

hmm yes that's a good idea, never mind then, let's leave version untouched for now.

Since there is already demand, we can release driver 0.9.0 + helm chart 0.9.0 later, after this merges.

I agree with @wongma7. We should just bump to 0.8.3.

Roger, so I will just update chart version to 0.8.3 in this PR and we can sync the driver version and chart version later(in 0.9.0 release)

ayberk · 2021-01-28T23:56:16Z

charts/aws-ebs-csi-driver/values.yaml

  snapshotterImage:
-    repository: quay.io/k8scsi/csi-snapshotter
+    repository: k8s.gcr.io/sig-storage/csi-snapshotter


We need at least 3.0.0 for this one it seems: kubernetes-csi/external-snapshotter#461

@wongma7 maybe we should get rid of the snapshotter before merging this one. Not sure which one is less effort, but this would be a throw-away effort.

Hi Ayberk, just updated snapshotter sidecar and controller to version 3.0.3 and validated they are functioning as expected. Maybe we can retire snapshotter in a separate PR

ayberk · 2021-01-29T18:12:02Z

docs/README.md

@@ -109,6 +109,8 @@ kubectl create -f https://raw.githubusercontent.com/kubernetes/csi-api/release-1
 ```

 #### Deploy driver
+Latest master branch only supports driver deployment on kubernetes version >= 1.17


Can we make this more generic? I'm thinking something like "Please see the compatibility matrix above before you deploy the driver". So we don't have to keep updating this part as well.

ayberk · 2021-01-29T19:02:37Z

/lgtm
/approve

Great work! We're finally officially supporting arm without workarounds :)

k8s-ci-robot · 2021-01-29T19:02:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AndyXiangLi, ayberk

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

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

ajaykmis · 2021-01-29T22:23:15Z

@ayberk @AndyXiangLi - Thanks for merging this!

Did someone validate volume snapshotting with this on Arm64? Do we now use the alpha overlay or stable/arm64 to install the EBS CSI driver.

Ajay

AndyXiangLi · 2021-01-29T22:32:39Z

Hi @ajaykmis
Yes, I validated volume snapshotter is working on Arm64, and I recommend you to use helm to install driver for now. The arm overlay is same as stable overlay after this change and we will remove arm overlay soon to reduce confusion.
you can refer to helm part from here

ajaykmis · 2021-01-29T22:40:39Z

Thanks @AndyXiangLi

I see this from the doc:
helm upgrade --install aws-ebs-csi-driver
--namespace kube-system
--set enableVolumeScheduling=true
--set enableVolumeResizing=true
--set enableVolumeSnapshot=true
aws-ebs-csi-driver/aws-ebs-csi-driver

a couple of questions:

what would be this path ${your_chart_path}?
does helm take care of choosing the right overlay? so can I use the same helm command to install EBS driver on both ARM and x86 clusters?

AndyXiangLi · 2021-01-29T22:50:01Z

@ajaykmis Sorry I should not introduce confusion here so I just remove the second command from previous comment.

If you have this project on your machine, your_chart_path = ${project_path}/charts/aws-ebs-csi-driver
Yes you can run same command on ARM and x86 clusters as all the images we are using are multiarch supported after this change

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 27, 2021

k8s-ci-robot requested review from d-nishi and gnufied January 27, 2021 18:58

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 27, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 27, 2021

AndyXiangLi force-pushed the update-arm-image-version branch from a7b8b77 to f4b0220 Compare January 27, 2021 19:29

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 28, 2021

AndyXiangLi force-pushed the update-arm-image-version branch from 2ae562b to 56adf19 Compare January 28, 2021 01:16

AndyXiangLi changed the title ~~WIP: update csi provisioner version to v2.1.0~~ WIP: update sidecars to newer version Jan 28, 2021

AndyXiangLi force-pushed the update-arm-image-version branch 4 times, most recently from 87b7f80 to 651a015 Compare January 28, 2021 17:50

AndyXiangLi force-pushed the update-arm-image-version branch from 651a015 to 480a53b Compare January 28, 2021 18:43

wongma7 reviewed Jan 28, 2021

View reviewed changes

AndyXiangLi force-pushed the update-arm-image-version branch from 480a53b to a0be105 Compare January 28, 2021 23:40

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 28, 2021

AndyXiangLi force-pushed the update-arm-image-version branch from a0be105 to f7ed4e8 Compare January 28, 2021 23:45

k8s-ci-robot removed the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 28, 2021

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 28, 2021

AndyXiangLi changed the title ~~WIP: update sidecars to newer version~~ Update sidecars to newer version Jan 28, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 28, 2021

ayberk reviewed Jan 28, 2021

View reviewed changes

AndyXiangLi force-pushed the update-arm-image-version branch 2 times, most recently from 98655e5 to 5da4ffb Compare January 29, 2021 17:41

ayberk reviewed Jan 29, 2021

View reviewed changes

AndyXiangLi force-pushed the update-arm-image-version branch from 5da4ffb to 2bd03f9 Compare January 29, 2021 18:50

update sidecar image version and use repo /sig-storage

1a0bd40

AndyXiangLi force-pushed the update-arm-image-version branch from 2bd03f9 to 1a0bd40 Compare January 29, 2021 18:52

k8s-ci-robot assigned ayberk Jan 29, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 29, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 29, 2021

k8s-ci-robot merged commit 511dd1c into kubernetes-sigs:master Jan 29, 2021

ayberk mentioned this pull request Feb 2, 2021

add sidecar container support for arm64 for kubernetes +1.17 #596

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update sidecars to newer version #707

Update sidecars to newer version #707

AndyXiangLi commented Jan 27, 2021 •

edited

k8s-ci-robot commented Jan 27, 2021

wongma7 commented Jan 27, 2021

coveralls commented Jan 27, 2021 •

edited

AndyXiangLi commented Jan 28, 2021

AndyXiangLi commented Jan 28, 2021

AndyXiangLi commented Jan 28, 2021

wongma7 Jan 28, 2021

AndyXiangLi Jan 28, 2021

wongma7 Jan 28, 2021

ayberk Jan 28, 2021

AndyXiangLi Jan 28, 2021

ayberk Jan 28, 2021

ayberk Jan 28, 2021

AndyXiangLi Jan 29, 2021

ayberk Jan 29, 2021

AndyXiangLi Jan 29, 2021

ayberk commented Jan 29, 2021

k8s-ci-robot commented Jan 29, 2021

ajaykmis commented Jan 29, 2021

AndyXiangLi commented Jan 29, 2021 •

edited

ajaykmis commented Jan 29, 2021

AndyXiangLi commented Jan 29, 2021 •

edited

Update sidecars to newer version #707

Update sidecars to newer version #707

Conversation

AndyXiangLi commented Jan 27, 2021 • edited

k8s-ci-robot commented Jan 27, 2021

wongma7 commented Jan 27, 2021

coveralls commented Jan 27, 2021 • edited

Pull Request Test Coverage Report for Build 1506

💛 - Coveralls

AndyXiangLi commented Jan 28, 2021

AndyXiangLi commented Jan 28, 2021

AndyXiangLi commented Jan 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayberk commented Jan 29, 2021

k8s-ci-robot commented Jan 29, 2021

ajaykmis commented Jan 29, 2021

AndyXiangLi commented Jan 29, 2021 • edited

ajaykmis commented Jan 29, 2021

AndyXiangLi commented Jan 29, 2021 • edited

AndyXiangLi commented Jan 27, 2021 •

edited

coveralls commented Jan 27, 2021 •

edited

AndyXiangLi commented Jan 29, 2021 •

edited

AndyXiangLi commented Jan 29, 2021 •

edited