kubebuilder
kubebuilder copied to clipboard
kubernetes-sigs
:book: update webhooks for core types to match controller-runtime v0.15
Hi @mythi. 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.
@camilamacedo86 I believe it's still needed. pkg/inject is also removed so the example makes the webhook to crash. This moves the book to use the builder approach following the CR change and my fixes
Hi @mythi,
@camilamacedo86 anything I can/need to do here?
By using the master branch and checking the samples code in the master, what issues are you still face? Can you list them out and let us know what changes are required then, I can help you with where then should be made.
I believe it's still needed. pkg/inject is also removed so the example makes the webhook to crash.
We do not use sigs.k8s.io/controller-runtime/pkg/runtime/inject. However, if we need to change something it seems that it must be done in. the default scaffolds. So, can you let us know when you create a project, then api and a webhook what is wrong?
So, can you let us know when you create a project, then api and a webhook what is wrong?
@camilamacedo86 hmm but this PR is not about the scaffolding process but this page https://book.kubebuilder.io/reference/webhook-for-core-types.html which is wrong.
Hi @mythi,
@camilamacedo86 hmm but this PR is not about the scaffolding process but this page https://book.kubebuilder.io/reference/webhook-for-core-types.html which is wrong.
It seems make sense. When it no longer be a draft could you please either ping in the slack so that we won a help from the community in the review?
@erikgb
I think you are very skilled in this area, could you please give a hand in the review of this PR?
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: mythi Once this PR has been reviewed and has the lgtm label, please assign varshaprasad96 for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@erikgb @camilamacedo86 the review comments are addressed, and this is ready for another review
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle rotten - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten
@mythi: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| pull-kubebuilder-e2e-k8s-1-27-10 | 7bbc5f5377b5c4c1d3b50cbd8f7568a9fddba10d | link | true | /test pull-kubebuilder-e2e-k8s-1-27-10 |
| pull-kubebuilder-e2e-k8s-1-28-6 | 7bbc5f5377b5c4c1d3b50cbd8f7568a9fddba10d | link | true | /test pull-kubebuilder-e2e-k8s-1-28-6 |
| pull-kubebuilder-e2e-k8s-1-29-0 | 7bbc5f5377b5c4c1d3b50cbd8f7568a9fddba10d | link | true | /test pull-kubebuilder-e2e-k8s-1-29-0 |
| pull-kubebuilder-e2e-k8s-1-28-0 | 3781a81d5a60f14ce9a46c3339d75eae210107f5 | link | true | /test pull-kubebuilder-e2e-k8s-1-28-0 |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
Would love to see docs updated. The example code works nicely, but the docs are misleading (to me). They don't align with the code example.
The example code works nicely, but the docs are misleading (to me). They don't align with the code example.
Just to clarify: when you say "example", you mean the controller-runtime example the book points to? This PR is exactly about getting those aligned.
HI @mythi
How, we should change this doc?
- Just ensure that the current example provided in the docs (same kubebuilder example) will be changed to address breakchanges
- Ensure that the example provide can properly work and matches with kubebuilder layout by creating a project and checking it out
We should not:
- Copy and paste the examples from controller-runtime (it does not bring value for the users, if we want to use their example we can just link it. Make no sense we duplicate things) Furthermore, just copy a controller-runtime in the doc does not mean that will work. We must to describe to users how to do the same with Kubebuilder layout, describe the steps, and ensure that the info provided is accurate.
- Change the example in away that it does not work with the default kubebuilder layout or not fits well on it.
- Change the logic of the example provided without any good reason to do so.
I hope that clarifies.
We should not:
I don't think the current example meets any of these bullets either. However, if that is desired, we could close this and someone can pick up re-writing this page according to the new ask.