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

Add webhook health check #2602

Closed
sbueringer opened this issue Jul 26, 2021 · 6 comments · Fixed by #2603
Closed

Add webhook health check #2602

sbueringer opened this issue Jul 26, 2021 · 6 comments · Fixed by #2603
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority

Comments

@sbueringer
Copy link
Member

/kind feature

Describe the solution you'd like
It would be nice if the controller health checks would use the readiness of the webhook server (instead of the no-op Ping).

For reference, the implementation in cluster-api: kubernetes-sigs/cluster-api#4989

Anything else you would like to add:
This requires at least controller-runtime v0.9.3.

Environment:

  • Cluster-api-provider-aws version:
  • Kubernetes version: (use kubectl version):
  • OS (e.g. from /etc/os-release):
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. needs-priority labels Jul 26, 2021
@sedefsavas sedefsavas self-assigned this Jul 26, 2021
@sbueringer
Copy link
Member Author

sbueringer commented Jul 30, 2021

Webhook health check was removed in #2629 due to a bug which lead to tls handshake errors. Fix PR has been merged in controller-runtime, but we have to wait until the next controller-runtime release (0.9.6) to integrate it.

xref:

/reopen

@k8s-ci-robot
Copy link
Contributor

@sbueringer: Reopened this issue.

In response to this:

Webhook health check was removed due to a bug found which lead to tls handshake errors in #2629. Fix PR has been merged in controller-runtime, but we have to wait until the next controller-runtime release to integrate it.

xref:

/reopen

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 reopened this Jul 30, 2021
@sbueringer
Copy link
Member Author

fyi controller-runtime v0.9.6 has been released with a fix. So we should be good to go (I also verified it via the e2e tests of the cluster-api repo).

@geetikabatra
Copy link
Contributor

@sbueringer Here is the PR that fixes this #2685

@sbueringer
Copy link
Member Author

Has been fixed in #2685

/close

@k8s-ci-robot
Copy link
Contributor

@sbueringer: Closing this issue.

In response to this:

Has been fixed in #2685

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants