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

Install azure disk CSI driver Helm chart with CAAPH #4109

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Install azure disk CSI driver Helm chart with CAAPH #4109

merged 1 commit into from
Nov 3, 2023

Conversation

Jont828
Copy link
Contributor

@Jont828 Jont828 commented Oct 9, 2023

What type of PR is this?
/kind feature

What this PR does / why we need it: Install azure disk CSI driver Helm chart with CAAPH in e2e tests.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #3736

Special notes for your reviewer:

  • cherry-pick candidate

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

Install azure disk CSI driver Helm chart with CAAPH

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Oct 9, 2023
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 9, 2023
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 9, 2023
@Jont828 Jont828 mentioned this pull request Oct 9, 2023
Closed
4 tasks
@codecov
Copy link

codecov bot commented Oct 9, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (46cf208) 58.31% compared to head (4da5493) 58.31%.
Report is 13 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #4109   +/-   ##
=======================================
  Coverage   58.31%   58.31%           
=======================================
  Files         187      187           
  Lines       19231    19231           
=======================================
  Hits        11214    11214           
  Misses       7387     7387           
  Partials      630      630

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CecileRobertMichon
Copy link
Contributor

Add template to all execept AKS and windows containerd

windows test templates should install the CSI driver AFAIK

@Jont828 Jont828 force-pushed the install-azuredisk-csi-driver-with-caaph branch from 4e28ee5 to fd9ba6e Compare October 10, 2023 21:36
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 10, 2023
@Jont828
Copy link
Contributor Author

Jont828 commented Oct 10, 2023

@CecileRobertMichon Yep, I couldn't figure out how to change the windows containerd template so I added that as a note to come back to it.

@k8s-ci-robot 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 Oct 11, 2023
@Jont828
Copy link
Contributor Author

Jont828 commented Oct 12, 2023

@CecileRobertMichon It looks like the conformance flavor is failing. Does that use the same flavor as the prow templates? And is there anything I need to change for ci-entrypoint like how we removed the installation in the Calico PR?

@Jont828 Jont828 force-pushed the install-azuredisk-csi-driver-with-caaph branch from f28b354 to 2845e90 Compare October 12, 2023 16:44
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 12, 2023
@Jont828
Copy link
Contributor Author

Jont828 commented Oct 18, 2023

/retest

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2023
@Jont828 Jont828 force-pushed the install-azuredisk-csi-driver-with-caaph branch 2 times, most recently from 8faf397 to 88f288c Compare October 23, 2023 21:23
@Jont828
Copy link
Contributor Author

Jont828 commented Oct 23, 2023

/retest

@Jont828 Jont828 force-pushed the install-azuredisk-csi-driver-with-caaph branch from 88f288c to e91353f Compare October 24, 2023 17:27
@Jont828 Jont828 force-pushed the install-azuredisk-csi-driver-with-caaph branch from 24a5701 to 9db7aee Compare October 30, 2023 16:42
CecileRobertMichon
CecileRobertMichon reviewed Oct 30, 2023
View reviewed changes
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 30, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 8c680d15f5f0c38ae2e240421f9003560d8c829f

@Jont828 Jont828 force-pushed the install-azuredisk-csi-driver-with-caaph branch from 9db7aee to 4da5493 Compare October 31, 2023 21:24
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 31, 2023
@CecileRobertMichon
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 31, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 3fee9365d492ac4353d326490a137f8b900f61ef

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

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 Oct 31, 2023
@CecileRobertMichon
Copy link
Contributor

/hold for tests

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 31, 2023
@Jont828
Copy link
Contributor Author

Jont828 commented Nov 1, 2023

/retest pull-cluster-api-provider-azure-conformance-custom-builds

@k8s-ci-robot
Copy link
Contributor

@Jont828: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test pull-cluster-api-provider-azure-build
  • /test pull-cluster-api-provider-azure-ci-entrypoint
  • /test pull-cluster-api-provider-azure-e2e
  • /test pull-cluster-api-provider-azure-e2e-aks
  • /test pull-cluster-api-provider-azure-test
  • /test pull-cluster-api-provider-azure-verify

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-provider-azure-apidiff
  • /test pull-cluster-api-provider-azure-apiversion-upgrade
  • /test pull-cluster-api-provider-azure-capi-e2e
  • /test pull-cluster-api-provider-azure-conformance
  • /test pull-cluster-api-provider-azure-conformance-custom-builds
  • /test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts
  • /test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
  • /test pull-cluster-api-provider-azure-conformance-with-ci-artifacts
  • /test pull-cluster-api-provider-azure-e2e-optional
  • /test pull-cluster-api-provider-azure-e2e-workload-upgrade
  • /test pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts-serial-slow
  • /test pull-cluster-api-provider-azure-windows-custom-builds
  • /test pull-cluster-api-provider-azure-windows-with-ci-artifacts

Use /test all to run the following jobs that were automatically triggered:

  • pull-cluster-api-provider-azure-apidiff
  • pull-cluster-api-provider-azure-apiversion-upgrade
  • pull-cluster-api-provider-azure-build
  • pull-cluster-api-provider-azure-capi-e2e
  • pull-cluster-api-provider-azure-ci-entrypoint
  • pull-cluster-api-provider-azure-conformance
  • pull-cluster-api-provider-azure-conformance-custom-builds
  • pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts
  • pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
  • pull-cluster-api-provider-azure-conformance-with-ci-artifacts
  • pull-cluster-api-provider-azure-e2e
  • pull-cluster-api-provider-azure-e2e-aks
  • pull-cluster-api-provider-azure-test
  • pull-cluster-api-provider-azure-verify
  • pull-cluster-api-provider-azure-windows-custom-builds
  • pull-cluster-api-provider-azure-windows-with-ci-artifacts

In response to this:

/retest pull-cluster-api-provider-azure-conformance-custom-builds

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.

@willie-yao
Copy link
Contributor

/retest

@Jont828
Copy link
Contributor Author

Jont828 commented Nov 2, 2023

Thanks @willie-yao!

@Jont828
Copy link
Contributor Author

Jont828 commented Nov 2, 2023

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 2, 2023
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Nov 2, 2023 •
edited
Loading

@Jont828: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-provider-azure-windows-containerd-upstream-custom-builds e91353f link false /test pull-cluster-api-provider-azure-windows-containerd-upstream-custom-builds

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@CecileRobertMichon
Copy link
Contributor

/retest

@k8s-ci-robot k8s-ci-robot merged commit 5670794 into kubernetes-sigs:main Nov 3, 2023
10 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.12 milestone Nov 3, 2023
@Jont828
Copy link
Contributor Author

Jont828 commented Nov 3, 2023

🚀🚀🚀

@andyzhangx
Copy link
Member

@Jont828 can you remove -oot in all csi driver addons, and cherrypick to v1.12 release? All CSI drver e2e tests are broken:

I hit following error after this change when running csi driver e2e tests:

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_azuredisk-csi-driver/2059/pull-azuredisk-csi-driver-e2e-capz/1731520693641678848

kubernetes-sigs/azuredisk-csi-driver#2059

Error: INSTALLATION FAILED: Unable to continue with install: ServiceAccount "csi-azuredisk-controller-sa" in namespace "kube-system" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-name" must equal "azuredisk-csi-driver": current value is "azuredisk-csi-driver-oot"
helm.go:84: [debug] ServiceAccount "csi-azuredisk-controller-sa" in namespace "kube-system" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-name" must equal "azuredisk-csi-driver": current value is "azuredisk-csi-driver-oot"
Unable to continue with install
helm.sh/helm/v3/pkg/action.(*Install).RunWithContext
	helm.sh/helm/v3/pkg/action/install.go:340
main.runInstall
	helm.sh/helm/v3/cmd/helm/install.go:306
9 skipped lines...
	helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
	runtime/proc.go:250
runtime.goexit
	runtime/asm_amd64.s:1598

or maybe we should revert to v1.12 release in test-infra now? @jackfrancis

@jackfrancis jackfrancis mentioned this pull request Dec 4, 2023
Closed
@jackfrancis
Copy link
Contributor

@andyzhangx a fix is here: #4334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackfrancis jackfrancis Awaiting requested review from jackfrancis

@willie-yao willie-yao Awaiting requested review from willie-yao

@CecileRobertMichon CecileRobertMichon Awaiting requested review from CecileRobertMichon

Assignees

@CecileRobertMichon CecileRobertMichon

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. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
CAPZ Planning
Archived in project
Milestone
v1.12
Development

Successfully merging this pull request may close these issues.

Use CAAPH to install Helm charts in e2e tests
6 participants
@Jont828 @CecileRobertMichon @k8s-ci-robot @willie-yao @andyzhangx @jackfrancis