-
Notifications
You must be signed in to change notification settings - Fork 430
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.
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
Install cloud-provider-azure chart in e2e tests using CAAPH #3422
Conversation
e60bf36
to
5ccde2c
Compare
/retest |
1 similar comment
/retest |
866fcfd
to
7c385be
Compare
b6452f3
to
580565e
Compare
/hold I vendored CAAPH from the current |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3422 +/- ##
==========================================
+ Coverage 56.00% 56.02% +0.01%
==========================================
Files 190 190
Lines 19472 19473 +1
==========================================
+ Hits 10906 10909 +3
+ Misses 7948 7944 -4
- Partials 618 620 +2
☔ View full report in Codecov by Sentry. |
8957842
to
d7492d1
Compare
@@ -241,6 +241,7 @@ var _ = Describe("Running the Cluster API E2E tests", func() { | |||
InitWithBootstrapProviders: []string{"kubeadm:v1.0.5"}, | |||
InitWithControlPlaneProviders: []string{"kubeadm:v1.0.5"}, | |||
InitWithInfrastructureProviders: []string{"azure:v1.0.2"}, | |||
// Note: this probably won't work with CAAPH since the Helm install only works with CAPI/clusterctl v1.5.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willie-yao I saw you made some changes here, I don't think we can use the CAAPH install with CAPI v1.0.5. Is there any way we can get around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to add an exception to not use CAAPH for this test? This test is testing the upgrade from the earliest stable version of CAPI to current, so it will be useful to keep around. If not, is there a minimum version of CAPI that will work with CAAPH install?
Focusing just on the AZWI=false GINKGO_FOCUS="Creating a highly available cluster \[REQUIRED\]" \
SKIP_CLEANUP=true ./scripts/ci-e2e.sh I poked at the workload cluster during this test, and cloud-provider-azure components were all installed and running. So CAAPH seems basically to be working, but somehow this PR makes Calico fail some of the time... |
Yeah, I ran the e2e tests locally as well and it seemed to work fine for me. I wonder if the version upgrade is causing any issues. |
test/e2e/azure_test.go
Outdated
@@ -76,6 +80,17 @@ var _ = Describe("Workload cluster creation", func() { | |||
namespace, cancelWatches, err = setupSpecNamespace(ctx, clusterNamePrefix, bootstrapClusterProxy, artifactFolder) | |||
Expect(err).NotTo(HaveOccurred()) | |||
|
|||
By("Initialize bootstrap client to install add-ons") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be feasible to detect the CAPI version here and somehow fall back to the old helm install
behavior if it's not >= 1.5.0?
@Jont828: The following tests failed, say
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. |
PR needs rebase. 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. |
/close let's open a new PR for this change once it's ready |
@CecileRobertMichon: Closed this PR. In response to this:
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. |
What type of PR is this?
/kind feature
What this PR does / why we need it: Use CAAPH to install cloud-provider-azure chart instead of Helm SDK 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:
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
TODOs:
Release note: