optional flatcar tests will create a loadbalancer #3386

nawazkh · 2023-04-03T23:01:08Z

What type of PR is this?
/kind bug
/kind flake

What this PR does / why we need it:

Flatcar's optional tests should be failing if the cloud-controller-manager-x-x pod fails or errors out due to a panic.

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 #3387

Special notes for your reviewer:
This PR:

Will add caCertDir while installing cloud provider manager on a workload cluster.
- Updates Tiltfile and
- Updates E2E tests
Updates external-cloud-provider documentation
Updates Azure Flatcar tests to check the workload cluster after it has been brought up.

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

flatcar: Specify `caCertDir` location while installing “external” cloud provider for Azure

nawazkh · 2023-04-03T23:01:23Z

/test ls

k8s-ci-robot · 2023-04-03T23:01:25Z

@nawazkh: The specified target(s) for /test were not found.
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-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
/test pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts-serial-slow

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

pull-cluster-api-provider-azure-apidiff
pull-cluster-api-provider-azure-build
pull-cluster-api-provider-azure-ci-entrypoint
pull-cluster-api-provider-azure-conformance
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-containerd-upstream-with-ci-artifacts

In response to this:

/test ls

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.

nawazkh · 2023-04-03T23:08:01Z

/test pull-cluster-api-provider-azure-e2e-optional

nawazkh · 2023-04-04T00:30:52Z

Hmm, as expected. cloud-controller-manager-74f84b7fb5-4589r restarted 4 times.

Restart Count:  4

 [PANICKED] in [AfterEach] - /usr/local/go/src/runtime/panic.go:260 @ 04/03/23 23:39:03.297
  << Timeline
   [FAILED] Timed out after 180.001s.
  Failed to get capz-e2e-wk6i3c/capz-e2e-e

invidian · 2023-04-04T09:51:21Z

I've created #3387 to track the investigation, as we will need both test and a fix for this.

I can reproduce it on my cluster, so I'm investigating, will share findings in the issue.

willie-yao · 2023-04-04T17:43:37Z

test/e2e/azure_test.go

@@ -294,10 +294,9 @@ var _ = Describe("Workload cluster creation", func() {
 		})
 	})

-	Context("Creating a Flatcar cluster [OPTIONAL]", func() {
+	Context("Creating a Flatcar cluster [OPTIONAL]", Ordered, func() {


This looks great to me! Just for my own understanding, what does the Ordered argument mean in this case?

Ordered containers will always run in the order they have been defined.
So in our scenario, all the It() will always run in the order they have been defined and not messed up by Ginkgo's parallelization approach.
Also note that I converted By() -> It().
By() acts mostly as a logger. By converting By() -> It() I can ask ginkgo to show that Log line as a test case.

More reference on ordered containers : https://onsi.github.io/ginkgo/#ordered-containers

Hey @willie-yao , Wanted to share something I learned the hard way.
Splitting the tests, as I did in here albeit preserving the order of the tests, does not help the testing. Unless the The tests warrants splitting the It spec into multiple Its, its better to stick with the existing code flow of using By inside It.
The problem with splitting Its in my case is that, AfterEach() gets triggered after each It and it cleans up all the setup the tests does in the first It.

TL;DR: Use By in our tests over splitting it into It since AfterEach() cleans up setup and other artifacts after each It.

nawazkh · 2023-04-04T21:54:03Z

/test pull-cluster-api-provider-azure-e2e-optional

codecov-commenter · 2023-04-04T22:16:18Z

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (c125458) 51.64% compared to head (1372819) 51.63%.

❗ Current head 1372819 differs from pull request most recent head efacbb0. Consider uploading reports for the commit efacbb0 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3386      +/-   ##
==========================================
- Coverage   51.64%   51.63%   -0.02%     
==========================================
  Files         182      182              
  Lines       18066    18066              
==========================================
- Hits         9331     9329       -2     
- Misses       8208     8210       +2     
  Partials      527      527

see 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

test/e2e/cloud-provider-azure.go

nawazkh · 2023-04-05T23:03:48Z

/test pull-cluster-api-provider-azure-e2e-optional

nawazkh · 2023-04-06T19:21:03Z

/test pull-cluster-api-provider-azure-e2e-optional

invidian · 2023-04-13T12:50:13Z

Any clues why Flatcar tests are still failing? If not, I'll continue investigating.

nawazkh · 2023-04-13T20:35:36Z

/test pull-cluster-api-provider-azure-e2e-optional

nawazkh · 2023-04-14T18:15:28Z

Any clues why Flatcar tests are still failing? If not, I'll continue investigating.

From the artifacts folder, looks like the workload cluster isn't even coming up.

k8s-ci-robot · 2023-05-12T21:00:40Z

LGTM label has been added.

Git tree hash: 67c766eb3f72afcec7662addf060332e5734218b

k8s-ci-robot · 2023-05-12T21:00:40Z

LGTM label has been added.

Git tree hash: 67c766eb3f72afcec7662addf060332e5734218b

k8s-ci-robot · 2023-05-12T21:00:41Z

[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:

~~OWNERS~~ [CecileRobertMichon]

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 · 2023-05-12T21:00:41Z

[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:

~~OWNERS~~ [CecileRobertMichon]

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

- Set cloudControllerManager.caCertDir to `/usr/share/ca-certificates` - and check if loadbalancer can spin up successfully - add caCertDir to CCM in tiltfile - add Note about caCertDir to CCM in azure external docs

nawazkh · 2023-05-18T16:28:02Z

Rebased and resolved the Tiltfile conflict. Please take a look. @CecileRobertMichon @willie-yao

nawazkh · 2023-05-18T16:30:41Z

/hold cancel

willie-yao · 2023-05-18T20:44:15Z

/lgtm

k8s-ci-robot · 2023-05-18T20:44:20Z

LGTM label has been added.

Git tree hash: 450a92ac51af589000ff0e5f451de0111062e774

willie-yao · 2023-05-18T20:45:06Z

/test pull-cluster-api-provider-azure-e2e-optional

nawazkh · 2023-05-19T01:36:03Z

/cherry-pick release-1.9
/cherry-pick release-1.8

k8s-infra-cherrypick-robot · 2023-05-19T01:36:49Z

@nawazkh: #3386 failed to apply on top of branch "release-1.9":

Applying: optional flatcar tests will create a loadbalancer
.git/rebase-apply/patch:30: trailing whitespace.
- **Note**: 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	Tiltfile
M	test/e2e/azure_test.go
Falling back to patching base and 3-way merge...
Auto-merging test/e2e/azure_test.go
Auto-merging Tiltfile
CONFLICT (content): Merge conflict in Tiltfile
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 optional flatcar tests will create a loadbalancer
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.9
/cherry-pick release-1.8

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.

invidian

Sorry for late review. LGTM. Thanks again for the efforts @nawazkh!

invidian · 2023-05-20T19:07:30Z

Tiltfile

@@ -394,6 +394,8 @@ def get_addons(flavor_name):

    if "intree-cloud-provider" not in flavor_name:
        addon_cmd += "; " + helm_cmd + " --kubeconfig ./${CLUSTER_NAME}.kubeconfig install --repo https://raw.githubusercontent.com/kubernetes-sigs/cloud-provider-azure/master/helm/repo cloud-provider-azure --generate-name --set infra.clusterName=${CLUSTER_NAME}"
+        if "flatcar" in flavor_name:  # append caCetDir location to the cloud-provider-azure helm install command for flatcar flavor


typo: caCetDir -> caCertDir

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. kind/flake Categorizes issue or PR as related to a flaky test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 3, 2023

k8s-ci-robot requested review from jackfrancis and sonasingh46 April 3, 2023 23:01

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 3, 2023

This comment was marked as outdated.

Sign in to view

nawazkh force-pushed the add_lb_to_flatcar_tests branch from a2435bd to 954678d Compare April 3, 2023 23:05

invidian mentioned this pull request Apr 4, 2023

CCM is in CrashLoopBackOff on Flatcar flavor #3387

Closed

willie-yao reviewed Apr 4, 2023

View reviewed changes

nawazkh force-pushed the add_lb_to_flatcar_tests branch from 46ea255 to f8ad961 Compare April 4, 2023 21:49

invidian reviewed Apr 5, 2023

View reviewed changes

test/e2e/cloud-provider-azure.go Show resolved Hide resolved

nawazkh force-pushed the add_lb_to_flatcar_tests branch from 1372819 to efacbb0 Compare April 5, 2023 23:03

jackfrancis added this to the v1.9 milestone Apr 6, 2023

nawazkh force-pushed the add_lb_to_flatcar_tests branch from efacbb0 to dcd9881 Compare April 6, 2023 19:18

nawazkh force-pushed the add_lb_to_flatcar_tests branch from dcd9881 to bef278b Compare April 12, 2023 22:02

This comment was marked as resolved.

Sign in to view

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 12, 2023

k8s-ci-robot assigned CecileRobertMichon May 12, 2023

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 12, 2023

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 12, 2023

nawazkh force-pushed the add_lb_to_flatcar_tests branch from 7af32d1 to f685b8f Compare May 13, 2023 03:00

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 13, 2023

k8s-ci-robot requested a review from CecileRobertMichon May 13, 2023 03:00

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2023

optional flatcar tests will create a loadbalancer

1ed2a41

- Set cloudControllerManager.caCertDir to `/usr/share/ca-certificates` - and check if loadbalancer can spin up successfully - add caCertDir to CCM in tiltfile - add Note about caCertDir to CCM in azure external docs

nawazkh force-pushed the add_lb_to_flatcar_tests branch from f685b8f to 1ed2a41 Compare May 16, 2023 16:12

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 18, 2023

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

k8s-ci-robot merged commit 4082e44 into kubernetes-sigs:main May 18, 2023

This was referenced May 19, 2023

[release-1.9] optional flatcar tests will create a loadbalancer #3578

Merged

[release-1.8] optional flatcar tests will create a loadbalancer #3579

Merged

invidian reviewed May 20, 2023

View reviewed changes

nawazkh deleted the add_lb_to_flatcar_tests branch August 31, 2023 20:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optional flatcar tests will create a loadbalancer #3386

optional flatcar tests will create a loadbalancer #3386

nawazkh commented Apr 3, 2023 •

edited

Loading

nawazkh commented Apr 3, 2023

k8s-ci-robot commented Apr 3, 2023

This comment was marked as outdated.

nawazkh commented Apr 3, 2023

nawazkh commented Apr 4, 2023

invidian commented Apr 4, 2023

willie-yao Apr 4, 2023

nawazkh Apr 4, 2023

nawazkh May 3, 2023

nawazkh commented Apr 4, 2023

codecov-commenter commented Apr 4, 2023 •

edited

Loading

nawazkh commented Apr 5, 2023

nawazkh commented Apr 6, 2023

invidian commented Apr 13, 2023

This comment was marked as resolved.

nawazkh commented Apr 13, 2023

nawazkh commented Apr 14, 2023

k8s-ci-robot commented May 12, 2023

k8s-ci-robot commented May 12, 2023

k8s-ci-robot commented May 12, 2023

k8s-ci-robot commented May 12, 2023

nawazkh commented May 18, 2023 •

edited

Loading

nawazkh commented May 18, 2023

willie-yao commented May 18, 2023

k8s-ci-robot commented May 18, 2023

willie-yao commented May 18, 2023

nawazkh commented May 19, 2023

k8s-infra-cherrypick-robot commented May 19, 2023

invidian left a comment

invidian May 20, 2023

optional flatcar tests will create a loadbalancer #3386

optional flatcar tests will create a loadbalancer #3386

Conversation

nawazkh commented Apr 3, 2023 • edited Loading

nawazkh commented Apr 3, 2023

k8s-ci-robot commented Apr 3, 2023

This comment was marked as outdated.

nawazkh commented Apr 3, 2023

nawazkh commented Apr 4, 2023

invidian commented Apr 4, 2023

willie-yao Apr 4, 2023

Choose a reason for hiding this comment

nawazkh Apr 4, 2023

Choose a reason for hiding this comment

nawazkh May 3, 2023

Choose a reason for hiding this comment

nawazkh commented Apr 4, 2023

codecov-commenter commented Apr 4, 2023 • edited Loading

Codecov Report

nawazkh commented Apr 5, 2023

nawazkh commented Apr 6, 2023

invidian commented Apr 13, 2023

This comment was marked as resolved.

nawazkh commented Apr 13, 2023

nawazkh commented Apr 14, 2023

k8s-ci-robot commented May 12, 2023

k8s-ci-robot commented May 12, 2023

k8s-ci-robot commented May 12, 2023

k8s-ci-robot commented May 12, 2023

nawazkh commented May 18, 2023 • edited Loading

nawazkh commented May 18, 2023

willie-yao commented May 18, 2023

k8s-ci-robot commented May 18, 2023

willie-yao commented May 18, 2023

nawazkh commented May 19, 2023

k8s-infra-cherrypick-robot commented May 19, 2023

invidian left a comment

Choose a reason for hiding this comment

invidian May 20, 2023

Choose a reason for hiding this comment

nawazkh commented Apr 3, 2023 •

edited

Loading

codecov-commenter commented Apr 4, 2023 •

edited

Loading

nawazkh commented May 18, 2023 •

edited

Loading