Resolve test flakes "the object has been modified"

[inteon]

What this PR does / why we need it:
This PR should resolve most of the flakes that look similar to "Operation cannot be fulfilled on certificates.cert-manager.io "serving-certs": the object has been modified; please apply your changes to the latest version and try again".
This flake is caused by the Issuing=True certificate condition that is removed by the issuing controller.
The best fix is to wait for the Issuing=True certificate condition to get removed when waiting for the certificate to become ready.

Release note:

/kind bug

[jetstack-bot]

Hi @inteon. Thanks for your PR.

I'm waiting for a jetstack or cert-manager 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.

[irbekrm]

/ok-to-test

[maelvls]

nit: would be great to have a bit more details in the commit message 🤔

Current:

commit e307c95d3acc5380d284e193c8e83755a71c02f2
Author: Inteon <42113979+inteon@users.noreply.github.com>
Date:   Wed Jul 21 23:57:17 2021 +0200

    resolve flakes

    Signed-off-by: Inteon <42113979+inteon@users.noreply.github.com>

When going through the git log, it would be nice to have "an idea" of what this commit achieves (keeping in mind the 72 chars width limit for the body, and 50 chars for the title)

[irbekrm]

@inteon thank you for working on this.
I had a brief look at the flakes, i.e here https://storage.googleapis.com/jetstack-logs/pr-logs/pull/jetstack_cert-manager/4218/pull-cert-manager-e2e-v1-21/1417896276934004736/build-log.txt .

Looking at one of the failures:

�[91m�[1m• Failure [12.629 seconds]�[0m
[cert-manager] CA Injector
�[90mtest/e2e/framework/framework.go:283�[0m
  for api services
  �[90mtest/e2e/suite/serving/cainjector.go:57�[0m
    �[91m�[1mshould update data when the certificate changes [It]�[0m
    �[90mtest/e2e/suite/serving/cainjector.go:149�[0m

    �[91mTimed out after 10.141s.
    Expected success, but got an error:
        <*errors.StatusError | 0xc0012b6be0>: {
            ErrStatus: {
                TypeMeta: {Kind: "", APIVersion: ""},
                ListMeta: {
                    SelfLink: "",
                    ResourceVersion: "",
                    Continue: "",
                    RemainingItemCount: nil,
                },
                Status: "Failure",
                Message: "Operation cannot be fulfilled on certificates.cert-manager.io \"serving-certs\": the object has been modified; please apply your changes to the latest version and try again",
                Reason: "Conflict",
                Details: {
                    Name: "serving-certs",
                    Group: "cert-manager.io",
                    Kind: "certificates",
                    UID: "",
                    Causes: nil,
                    RetryAfterSeconds: 0,
                },
                Code: 409,
            },
        }
        Operation cannot be fulfilled on certificates.cert-manager.io "serving-certs": the object has been modified; please apply your changes to the latest version and try again�[0m

    test/e2e/suite/serving/cainjector.go:161

Do you not think it's potentially a timeout from line 161 where we're attempting to apply an update? I see that this PR removes the 'Eventually' from that update operation altogether.

[irbekrm]

A few more test runs

/test pull-cert-manager-e2e-v1-21
/test pull-cert-manager-e2e-v1-20
/test pull-cert-manager-e2e-v1-19
/test pull-cert-manager-e2e-v1-18
/test pull-cert-manager-e2e-v1-17
/test pull-cert-manager-e2e-v1-16

[JoshVanL]

/test pull-cert-manager-e2e-v1-21
/test pull-cert-manager-e2e-v1-20
/test pull-cert-manager-e2e-v1-19
/test pull-cert-manager-e2e-v1-18
/test pull-cert-manager-e2e-v1-17
/test pull-cert-manager-e2e-v1-16
/lgtm

[irbekrm]

deployment.apps/sample-external-issuer-controller-manager created
Running e2e test suite...
$TEST_TMPDIR defined: output root default is '/bazel-scratch/.cache/bazel' and max_idle_secs default is '15'.
WARNING: Waiting for server process to terminate (waited 5 seconds, waiting at most 10)
WARNING: Waiting for server process to terminate (waited 10 seconds, waiting at most 10)
INFO: Waited 10 seconds for server process (pid=24445) to terminate.
FATAL: Attempted to kill stale server process (pid=24445) using SIGKILL, but it did not die in a timely fashion.

This is interesting.

But anyway the failures seem to be other flakes, not the ones this PR is fixing
/test pull-cert-manager-e2e-v1-19
/test pull-cert-manager-e2e-v1-21

[irbekrm]

/approve
/release-note-none

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: inteon, irbekrm

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 [irbekrm]

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