kubebuilder
kubebuilder copied to clipboard
🐛 Synchonized plural generation with api-machinery
Fixes https://github.com/kubernetes-sigs/kubebuilder/issues/3402
Hi @lauchokyip. 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.
@lauchokyip
Could you please squash the commits ?
@varshaprasad96 @everettraven wdyt about this one? Could you please give a hand on the review?
/hold
Definitely feel like a breaking change
Hi @lauchokyip,
Could you please squash the commits? Also, see that it is not passing in the lint you can run locally make lint locally to check it out.
By last, we need to provide a guidance for those that:
- Scaffold the project with an previous version, create an api and then get a new kubebuilder version to create another API Could you please add the steps in the description of this PR? What guidance/steps should we provide in this case?
It seems for me that would not work at all and this one is a breaking change. For other hand it seems a valid / reasonable request. I am just trying to think if it could be addressed for v4 or if we should only perform this change for an future version like go/v5.
c/c @everettraven @varshaprasad96 @Kavinjsir wdyt about this one?
Hi @camilamacedo86 , do you mean providing the migration documents? Any guidance on how I could do that?
I don't know the code well enough to answer this question - if the user generate a CRD named Helmswoman and the plural is set to Helmswomen. Now, the user wants to generate another version for the same CRD for let's say v2, kind Helmswoman. When the new code is merged, would the plural be set to Helmswomans ? If the plural were to set to Helmswomans, will that break anything besides the generated file name?
I will be voting towards putting this into v5 but given the lack of knowledge and context of the source code, definitely need more inputs
Hi @everettraven @varshaprasad96 @Kavinjsir @lauchokyip,
We have encountered several deprecations, and it's essential that we deprecate the declarative plugin as soon as possible. You can find the details in this pull request: https://github.com/kubernetes-sigs/kubebuilder/pull/3395
To ensure a cleaner and more maintainable codebase as we continue to grow, we plan to remove all deprecations at the beginning of next year, leading to a major bump for Kubebuilder 4.0.0.
However, since this change will have a significant impact on projects that are already scaffolded, I suggest we consider the following:
- Let's carefully evaluate whether this change is necessary, taking into account the comments shared by @Kavinjsir in this pull request: https://github.com/kubernetes-sigs/kubebuilder/pull/3433#pullrequestreview-1453231505.
- In the pull request, it would be helpful to document the manual steps required to update existing projects once this change is merged. @lauchokyip, could you please include these steps in the PR description? We need to provide guidance on how to modify projects built prior to this change, ensuring they won't be affected when a new version of Kubebuilder is released. Please specify the necessary file modifications and steps to be taken. (It is essential because it will help us properly document it in the release and migration guides)
- @lauchokyip, please ensure that the new code is only used for go/v4 and doesn't impact go/v3, which is already deprecated. By doing so, when we eventually bump to 4.x, the condition will keep the code as the default.
- Then, I think we might want to consider whether to keep this PR for future go/v5 changes. Once @lauchokyip implements the changes for impact only go/v4 as described above, we can modify the PR for go/v5 easily and merge it at the appropriate time.
Please share your thoughts on these suggestions.
@lauchokyip We are interested in resolving this issue. Could you please rebase this so that we revisit it again. Thanks for taking the time for working on this.
@lauchokyip We are interested in resolving this issue. Could you please rebase this so that we revisit it again. Thanks for taking the time for working on this.
Let me get back to this in a bit
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: everettraven, lauchokyip Once this PR has been reviewed and has the lgtm label, please assign camilamacedo86 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
HI @varshaprasad96 , make generate are failing, can you please check it? I even git clone a brand new repo and it still failed
TODO: update tutorial
ERRO[0009] error fixing cronjob_webhook.go: unable to find the content to be replaced
exit status 1
make: *** [Makefile:76: generate-docs] Error 1
Does make generate work for you?
PR needs rebase.
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.
HI @varshaprasad96 ,
make generateare failing, can you please check it? I evengit clonea brand new repo and it still failedTODO: update tutorial ERRO[0009] error fixing cronjob_webhook.go: unable to find the content to be replaced exit status 1 make: *** [Makefile:76: generate-docs] Error 1Does
make generatework for you?
@lauchokyip Would you rebase the branch based off the latest master branch? There might be an issue for make generate previously. The current master branch should have it fixed. Could you try and see if it works? Feel free to let me know if further questions rasied.
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
@lauchokyip: 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-25-3 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-25-3 |
| pull-kubebuilder-e2e-k8s-1-27-1 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-27-1 |
| pull-kubebuilder-e2e-k8s-1-27-10 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-27-10 |
| pull-kubebuilder-e2e-k8s-1-28-6 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-28-6 |
| pull-kubebuilder-e2e-k8s-1-27-3 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-27-3 |
| pull-kubebuilder-e2e-k8s-1-26-6 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-26-6 |
| pull-kubebuilder-e2e-k8s-1-28-0 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-28-0 |
| pull-kubebuilder-e2e-k8s-1-29-0 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-29-0 |
| pull-kubebuilder-e2e-k8s-1-27-11 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-27-11 |
| pull-kubebuilder-e2e-k8s-1-28-7 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-28-7 |
| pull-kubebuilder-e2e-k8s-1-29-2 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-29-2 |
| pull-kubebuilder-e2e-k8s-1-30-0 | b31f4cf066bf4911012836aff79f5b55f7484613 | link | true | /test pull-kubebuilder-e2e-k8s-1-30-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-sigs/prow repository. I understand the commands that are listed here.
Closed based on:
- Huge breaking change https://github.com/kubernetes-sigs/kubebuilder/pull/3433#issuecomment-1585538375 without a good justification. Why the change is not proposed to apimachinery?
- https://github.com/kubernetes-sigs/kubebuilder/issues/3402#issuecomment-2121490772
- https://github.com/kubernetes-sigs/kubebuilder/issues/3402#issuecomment-2122171734