Update helm install #265

lizhuqi · 2021-05-14T07:35:42Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
Can't use Helm to install driver for Windows because Windows does not support privileged containers. Change to use kubectl to install drivers.

Release note:

none

k8s-ci-robot · 2021-05-14T07:35:49Z

Hi @lizhuqi. Thanks for your PR.

I'm waiting for a kubernetes-csi 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.

andyzhangx

we should not use https://raw.githubusercontent.com/kubernetes-csi/csi-driver-smb/master/deploy/install-driver.sh for e2e test since it would install latest tag image which is not built by current code

andyzhangx · 2021-05-14T07:39:52Z

why not run the e2e test on a Linux machine?

lizhuqi · 2021-05-14T07:49:36Z

I am running the e2e test on a Linux machine. I can install the helm but when I try to use helm to install the CSI SMB driver, I got Permission denied error as below:
helm install csi-driver-smb charts/latest/csi-driver-smb --namespace kube-system --wait --timeout=15m -v=5 --debug
--set image.smb.repository=andyzhangx/smb-csi --set image.smb.tag=v0.7.0
--set windows.enabled=true
--set linux.enabled=false
--set controller.replicas=1
--set controller.logLevel=6
--set node.logLevel=6
make: helm: Permission denied
make: *** [Makefile:97: e2e-bootstrap] Error 127

coveralls · 2021-05-14T07:50:15Z

Pull Request Test Coverage Report for Build 876857451

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 86.507%

Totals
Change from base Build 871310345:	0.0%
Covered Lines:	686
Relevant Lines:	793

💛 - Coveralls

andyzhangx · 2021-05-14T07:52:36Z

I am running the e2e test on a Linux machine. I can install the helm but when I try to use helm to install the CSI SMB driver, I got Permission denied error as below:
helm install csi-driver-smb charts/latest/csi-driver-smb --namespace kube-system --wait --timeout=15m -v=5 --debug
--set image.smb.repository=andyzhangx/smb-csi --set image.smb.tag=v0.7.0
--set windows.enabled=true
--set linux.enabled=false
--set controller.replicas=1
--set controller.logLevel=6
--set node.logLevel=6
make: helm: Permission denied
make: *** [Makefile:97: e2e-bootstrap] Error 127

may add sudo? It's like config issue.

andyzhangx

/ok-to-test

andyzhangx · 2021-05-15T08:15:55Z

Makefile

@@ -18,17 +18,20 @@ REGISTRY ?= andyzhangx
 REGISTRY_NAME = $(shell echo $(REGISTRY) | sed "s/.azurecr.io//g")
 IMAGE_NAME ?= smb-csi
 IMAGE_VERSION ?= v0.7.0
+WIN_IMAGE_VERSION ?= v0.6.0-windows-1809-amd64


should be v0.7.0-windows-1809-amd64 which is ${IMAGE_VERSION}-windows-1809-amd64 ?

Done. Thanks!

andyzhangx · 2021-05-15T08:17:38Z

Makefile

@@ -113,7 +119,7 @@ install-helm:

 .PHONY: e2e-teardown
 e2e-teardown:
-	helm delete csi-driver-smb --namespace kube-system
+	sudo /usr/local/bin/helm delete csi-driver-smb --namespace kube-system --kubeconfig ~/.kube/config


that's env config issue: sudo /usr/local/bin/helm, better not hardcode in Makefile

Ok. Removed those in the Makefile

andyzhangx · 2021-05-15T08:18:08Z

Makefile

-	helm install csi-driver-smb charts/latest/csi-driver-smb --namespace kube-system --wait --timeout=15m -v=5 --debug \
-		${E2E_HELM_OPTIONS} \
+	docker pull $(WIN_IMAGE_TAG) || make container-all push-manifest	
+	sudo /usr/local/bin/helm upgrade csi-driver-smb charts/latest/csi-driver-smb --namespace kube-system --wait --timeout=15m -v=5 --debug --install \


helm upgrade

andyzhangx · 2021-05-15T08:18:43Z

Makefile

 		--set windows.enabled=true \
 		--set linux.enabled=false \
 		--set controller.replicas=1 \
 		--set controller.logLevel=6 \
-		--set node.logLevel=6
+		--set node.logLevel=6 \
+		--kubeconfig ~/.kube/config


I think ~/.kube/config is the default --kubeconfig setting, not necessary to specify here

yeah, I thought so. However if I don't specify here, helm fails to connect to. the cluster.

No idea why... Can we keep it here?

yeah, also removed --kubeconfig, it seems that it causes prow tests to fail

lizhuqi · 2021-05-19T05:22:13Z

/retest

lizhuqi · 2021-05-21T00:03:51Z

/retest

lizhuqi · 2021-05-24T22:39:42Z

/retest

andyzhangx

image.smb.tag=e2e-2e4d133997830fbbc81f1283102c805706a0a9b9-windows-1809-amd64 does not work on Linux node, would you use same tag for both Linux and Windows when building image?

helm upgrade csi-driver-smb charts/latest/csi-driver-smb --namespace kube-system --wait --timeout=15m -v=5 --debug --install \
	--set image.smb.repository=k8sprow.azurecr.io/smb-csi --set image.smb.tag=e2e-2e4d133997830fbbc81f1283102c805706a0a9b9-windows-1809-amd64 \
	--set windows.enabled=true \

andyzhangx · 2021-05-25T02:31:01Z

Makefile

@@ -18,17 +18,20 @@ REGISTRY ?= andyzhangx
 REGISTRY_NAME = $(shell echo $(REGISTRY) | sed "s/.azurecr.io//g")
 IMAGE_NAME ?= smb-csi
 IMAGE_VERSION ?= v0.7.0
+WIN_IMAGE_VERSION ?= ${IMAGE_VERSION}-windows-1809-amd64


v0.7.0 tag does not work on GKE Windows node? I think you could use one version tag for both Linux and Windows.

andyzhangx

that's quite clear now, could you squash all commits? thanks.

lizhuqi · 2021-05-26T00:53:41Z

that's quite clear now, could you squash all commits? thanks.

Finally squashed all the commits. Please take another look. Thanks!

andyzhangx

/lgtm
/approve

k8s-ci-robot · 2021-05-26T03:16:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, lizhuqi

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

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

andyzhangx · 2021-05-31T07:13:48Z

Makefile

@@ -95,15 +95,15 @@ e2e-test:
 e2e-bootstrap: install-helm
 	docker pull $(IMAGE_TAG) || make container-all push-manifest
 ifdef TEST_WINDOWS
-	helm install csi-driver-smb charts/latest/csi-driver-smb --namespace kube-system --wait --timeout=15m -v=5 --debug \
+	helm upgrade csi-driver-smb charts/latest/csi-driver-smb --namespace kube-system --wait --timeout=15m -v=5 --debug --install \


I found a bug for helm upgrade, agent node will always use v0.7.0 in azure pipeline which breaks the pipeline finally, I would revert to use original helm instal

print out csi-smb-node logs ... ====================================================================================== [pod/csi-smb-node-sqjc2/smb] I0526 01:09:40.294321 1 main.go:85] set up prometheus server on [::]:29645 [pod/csi-smb-node-sqjc2/smb] I0526 01:09:40.294699 1 smb.go:62] [pod/csi-smb-node-sqjc2/smb] DRIVER INFORMATION: [pod/csi-smb-node-sqjc2/smb] ------------------- [pod/csi-smb-node-sqjc2/smb] Build Date: "2021-05-22T01:05:27Z" [pod/csi-smb-node-sqjc2/smb] Compiler: gc [pod/csi-smb-node-sqjc2/smb] Driver Name: smb.csi.k8s.io [pod/csi-smb-node-sqjc2/smb] Driver Version: v0.7.0 [pod/csi-smb-node-sqjc2/smb] Git Commit: a03a482b5e925a77edcdfbab2ae330be8f5866ff [pod/csi-smb-node-sqjc2/smb] Go Version: go1.16 [pod/csi-smb-node-sqjc2/smb] Platform: linux/amd64

https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/kubernetes-csi_csi-driver-smb/265/pull-csi-driver-smb-e2e-windows/1397354833710682112/build-log.txt

nvm, it's not related, still try to figure out why agent node is always pulling image v0.7.0

98f23071d Merge pull request kubernetes-csi#260 from TerryHowe/update-csi-driver-version e9d8712d0 Merge pull request kubernetes-csi#259 from stmcginnis/deprecated-kind-kube-root faf79ff66 Remove --kube-root deprecated kind argument 734c2b950 Merge pull request kubernetes-csi#265 from Rakshith-R/consider-main-branch f95c855be Merge pull request kubernetes-csi#262 from huww98/golang-toolchain 3c8d966fe Treat main branch as equivalent to master branch e31de525b Merge pull request kubernetes-csi#261 from huww98/golang fd153a9e2 Bump golang to 1.23.1 a8b3d0504 pull-test.sh: fix "git subtree pull" errors 6b05f0fcc use new GOTOOLCHAIN env to manage go version 18b6ac6d2 chore: update CSI driver version to 1.15 git-subtree-dir: release-tools git-subtree-split: 98f23071d946dd3de3188a7e1f84679067003162

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. labels May 14, 2021

k8s-ci-robot requested review from andyzhangx and ZeroMagic May 14, 2021 07:35

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 14, 2021

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 14, 2021

andyzhangx reviewed May 14, 2021

View reviewed changes

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 15, 2021

lizhuqi changed the title ~~Change to use kubectl to install driver for Windows test~~ Update helm install for Windows test May 15, 2021

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 15, 2021

andyzhangx reviewed May 15, 2021

View reviewed changes

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 May 15, 2021

andyzhangx reviewed May 15, 2021

View reviewed changes

lizhuqi force-pushed the add-tolerance branch from 647c4de to b19b172 Compare May 24, 2021 23:36

andyzhangx reviewed May 25, 2021

View reviewed changes

lizhuqi changed the title ~~Update helm install for Windows test~~ Update helm install May 25, 2021

andyzhangx reviewed May 25, 2021

View reviewed changes

lizhuqi force-pushed the add-tolerance branch from f56da92 to fc1359b Compare May 25, 2021 23:41

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 25, 2021

lizhuqi force-pushed the add-tolerance branch 2 times, most recently from a3808a8 to 627f0cc Compare May 26, 2021 00:24

Update helm install

cca6fa0

lizhuqi force-pushed the add-tolerance branch from 627f0cc to cca6fa0 Compare May 26, 2021 00:51

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 26, 2021

andyzhangx approved these changes May 26, 2021

View reviewed changes

k8s-ci-robot assigned andyzhangx May 26, 2021

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

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

k8s-ci-robot merged commit 1795ebd into kubernetes-csi:master May 26, 2021

lizhuqi deleted the add-tolerance branch May 26, 2021 18:33

andyzhangx reviewed May 31, 2021

View reviewed changes

lizhuqi mentioned this pull request Jun 4, 2021

REQUEST: New membership for lizhuqi kubernetes/org#2763

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update helm install #265

Update helm install #265

lizhuqi commented May 14, 2021

k8s-ci-robot commented May 14, 2021

andyzhangx left a comment

andyzhangx commented May 14, 2021

lizhuqi commented May 14, 2021

coveralls commented May 14, 2021 •

edited

Loading

andyzhangx commented May 14, 2021

andyzhangx left a comment

andyzhangx May 15, 2021

lizhuqi May 15, 2021

andyzhangx May 15, 2021

lizhuqi May 15, 2021

andyzhangx May 15, 2021

andyzhangx May 15, 2021

lizhuqi May 15, 2021

lizhuqi May 24, 2021

lizhuqi commented May 19, 2021

lizhuqi commented May 21, 2021

lizhuqi commented May 24, 2021

andyzhangx left a comment

andyzhangx May 25, 2021

andyzhangx left a comment

lizhuqi commented May 26, 2021

andyzhangx left a comment

k8s-ci-robot commented May 26, 2021

andyzhangx May 31, 2021

andyzhangx May 31, 2021

Update helm install #265

Update helm install #265

Conversation

lizhuqi commented May 14, 2021

k8s-ci-robot commented May 14, 2021

andyzhangx left a comment

Choose a reason for hiding this comment

andyzhangx commented May 14, 2021

lizhuqi commented May 14, 2021

coveralls commented May 14, 2021 • edited Loading

Pull Request Test Coverage Report for Build 876857451

💛 - Coveralls

andyzhangx commented May 14, 2021

andyzhangx left a 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

lizhuqi commented May 19, 2021

lizhuqi commented May 21, 2021

lizhuqi commented May 24, 2021

andyzhangx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyzhangx left a comment

Choose a reason for hiding this comment

lizhuqi commented May 26, 2021

andyzhangx left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 14, 2021 •

edited

Loading