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

Retry on conflict for the end-to-end test "CA Injector for api services should update data when the certificate changes" #4925

Merged
merged 1 commit into from
Mar 16, 2022
Merged

Retry on conflict for the end-to-end test "CA Injector for api services should update data when the certificate changes" #4925

merged 1 commit into from
Mar 16, 2022

Conversation

maelvls
Copy link
Member

@maelvls maelvls commented Mar 6, 2022

The end-to-end test case:

[cert-manager] CA Injector for api services should update data when the certificate changes

caused the failure of 162 different prow builds. The error message is:

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

The affected builds were:

/kind cleanup

NONE

@jetstack-bot jetstack-bot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. area/testing Issues relating to testing approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 6, 2022
@jetstack-bot jetstack-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 6, 2022
@maelvls
e2e: fix flakiness: "CA Injector should update data when the certific…
6620f4b 
…ate changes"

The error:

    Operation cannot be fulfilled on certificates.cert-manager.io "serving-certs"

has appeared in 162 different prow builds in the past.

Signed-off-by: Maël Valais <mael@vls.dev>
@irbekrm
Copy link
Contributor

irbekrm commented Mar 16, 2022

Thanks for working on this. I think it should really be fixed by patching instead of updating as mentioned on the other related PR #4924 (comment) however having CI in a working state should be a priority and I see that figuring out how to use PATCH in this case is not straightforward

/lgtm
/approve

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2022
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@jetstack-bot jetstack-bot merged commit be9f2ae into cert-manager:master Mar 16, 2022
@jetstack-bot jetstack-bot added this to the v1.8 milestone Mar 16, 2022
@irbekrm irbekrm mentioned this pull request Apr 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@irbekrm irbekrm

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/testing Issues relating to testing dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.8
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@maelvls @irbekrm @jetstack-bot