E2E: waiting for services to be deleted before proceeding

[CecileRobertMichon]

What type of PR is this?
/kind failing-test

What this PR does / why we need it: following up on this slack thread, the Delete() client doesn't actually wait for a service to be deleted, it simply triggers the delete. cloud-provider-azure has a Finalizer on the Service resource as it needs to reconcile load balancers before the service can be deleted, which means it can take several minutes (sometimes even more) for the service to actually be gone. However, cloud-provider only reconciles one service at a time. This means before the ELB service starts reconciling, the ILB service first needs to be done deleting. By not waiting for the ILB to fully delete, we run into occasional flakes as the timeout for creating the ELB service also includes deleting the previous service.

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 #

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:

• squashed commits
• includes documentation
• adds unit tests

Release note:

NONE

[CecileRobertMichon]

did this to be consistent but need to check if it adds a lot of time to e2e duration since we don't strictly need to wait on this one (it's the last service to be reconciled)

[jackfrancis]

Interesting, so we have actually not been validating ELB delete (ILBs and ELBs are ultimately the same Azure LoadBalancer type, so not super critical, but I agree it's best-practice to not make an exception here)

[CecileRobertMichon]

/assign @jackfrancis

[jackfrancis]

/lgtm

[CecileRobertMichon]

@jackfrancis I got this flake:

Timed out after 300.001s.
Expected success, but got an error:
    <*errors.fundamental | 0xc0003ffb30>: {
        msg: "no machine has started to drain",
        stack: [0x1ca6ff4, 0x4d4145, 0x4d36c5, 0x7b4931, 0x7b4c99, 0x7b54e5, 0x7b4a2b, 0x1ca6d69, 0x1ca6528, 0x1cc6fd3, 0x7a538c, 0x7b1abe, 0x1cc6f51, 0x79171a, 0x7910e5, 0x79017b, 0x796569, 0x795f47, 0x7a3005, 0x7a2d25, 0x7a2565, 0x7a4812, 0x7b1125, 0x7b0f3e, 0x1cc14de, 0x517702, 0x46d2a1],
    }
    no machine has started to drain
/home/prow/go/src/sigs.k8s.io/cluster-api-provider-azure/test/e2e/azure_machinepool_drain.go:185

which suggests a new flake after #2123 merged

/retest

[jackfrancis]

@CecileRobertMichon indeed.

I'll open a PR to disable those drain tests. Recall they were never actually running w/ validation turned on, so we need to go through the process of tuning them and actually implementing them in a useable way.

[jackfrancis]

@CecileRobertMichon see #2160

[CecileRobertMichon]

/approve

[k8s-ci-robot]

[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

Details

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