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.

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

addons/cert_manager: retries until webhook pods has been created #7850

Conversation

rtsp
Copy link
Member

@rtsp rtsp commented Aug 4, 2021

What type of PR is this?

/kind bug

What this PR does / why we need it:

cert-manager webhook pods are deployed via the deployment that need a few second delay after applying the manifests until the pods are created.

This cause Cert Manager | Wait for Webhook pods become ready task to failed occasionally with "error: no matching resources found" if it's run too fast because kubectl wait cannot wait for non-exist resources (see kubernetes/kubectl#1516).

- name: Cert Manager | Wait for Webhook pods become ready
command: "{{ bin_dir }}/kubectl wait po --namespace={{ cert_manager_namespace }} --selector app=webhook --for=condition=Ready --timeout=600s"
register: cert_manager_webhook_pods_ready
when: inventory_hostname == groups['kube_control_plane'][0]
until: cert_manager_webhook_pods_ready is succeeded
retries: 30
delay: 10

This fix is using retries..until trick similar to #7842.

Special notes for your reviewer:

This fix should be removed in the future if the kubernetes/kubectl#1516 is resolved.

Does this PR introduce a user-facing change?:

NONE

Fix task 'Cert Manager | Wait for Webhook pods become ready' failed due to webhook pods don't exist yet by using `retries..until` trick like kubernetes-sigs#7842

This fix should be removed in the future if the kubernetes/kubernetes#83242 is resolved.

Signed-off-by: rtsp <[email protected]>
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 4, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @rtsp. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 4, 2021
@k8s-ci-robot k8s-ci-robot requested review from bozzo and oomichi August 4, 2021 22:00
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 4, 2021
@rtsp
Copy link
Member Author

rtsp commented Aug 9, 2021

/retest

failed test not related to code changes

@k8s-ci-robot
Copy link
Contributor

@rtsp: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

failed test not related to code changes

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.

Copy link
Contributor

@oomichi oomichi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ok-to-test
/lgtm

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 12, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 12, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floryut, rtsp

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2021
@k8s-ci-robot k8s-ci-robot merged commit 4c9d7de into kubernetes-sigs:master Aug 25, 2021
@floryut floryut mentioned this pull request Sep 8, 2021
LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Oct 23, 2021
…ernetes-sigs#7850)

Fix task 'Cert Manager | Wait for Webhook pods become ready' failed due to webhook pods don't exist yet by using `retries..until` trick like kubernetes-sigs#7842

This fix should be removed in the future if the kubernetes/kubernetes#83242 is resolved.

Signed-off-by: rtsp <[email protected]>
@rtsp rtsp deleted the develop/cert-manager-webhook-pods-waiting-fix branch April 12, 2022 13:33
sakuraiyuta pushed a commit to sakuraiyuta/kubespray that referenced this pull request Apr 16, 2022
…ernetes-sigs#7850)

Fix task 'Cert Manager | Wait for Webhook pods become ready' failed due to webhook pods don't exist yet by using `retries..until` trick like kubernetes-sigs#7842

This fix should be removed in the future if the kubernetes/kubernetes#83242 is resolved.

Signed-off-by: rtsp <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants