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

⚠️🐛 Fix RegularPlural #3408

Closed
wants to merge 1 commit into from

Conversation

hangscer8
Copy link
Member

@hangscer8 hangscer8 commented May 12, 2023

Fix #3402

Optimize the function RegularPlural according to UnsafeGuessKindToResource in k8s.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 12, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hangscer8
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.

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
Copy link
Contributor

Welcome @hangscer8!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 12, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @hangscer8. 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.

Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution, and it makes sense.
I approved the run of the tests. However, see that it will fail:

  • a) You must run make generate to ensure that the change will be reflected in all samples under the test data and docs
  • b) After we check those we will need to consider the breaking change aspect and see how better we can address this one. But first, let's ensure that it works and pass in all tests and etc.
  • c) can you verify the unit test it seems not working

Signed-off-by: hang.jiang <hang.jiang@daocloud.io>
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 12, 2023
@hangscer8
Copy link
Member Author

hangscer8 commented May 12, 2023

Thank you for the contribution, and it makes sense. I approved the run of the tests. However, see that it will fail:

  • a) You must run make generate to ensure that the change will be reflected in all samples under the test data and docs
  • b) After we check those we will need to consider the breaking change aspect and see how better we can address this one. But first, let's ensure that it works and pass in all tests and etc.
  • c) can you verify the unit test it seems not working

But I run make generate on the clean master branch rightnow (Macos 12 && golang 1.20), it also goes with wrong mssage , does this error message matter? :

Update dependencies:
$ go mod tidy
Running make:
$ make manifests
/Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-deploy-image/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
Next: check the implementation of your new API and controller. If you do changes in the API run the manifests with:
$ make manifests
Creating Memcached webhook ...
Writing kustomize manifests for you to edit...
Writing scaffold for you to edit...
api/v1alpha1/memcached_webhook.go
api/v1alpha1/webhook_suite_test.go
Update dependencies:
$ go mod tidy
Running make:
$ make generate
/Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-deploy-image/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
Next: implement your new Webhook and generate the manifests with:
$ make manifests
/Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-deploy-image/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
/Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-deploy-image/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
~/GoProjects/kubebuilder
~/GoProjects/kubebuilder/testdata/project-v4-with-grafana ~/GoProjects/kubebuilder
Generating project project-v4-with-grafana with flags: --plugins=go/v4
go: creating new go.mod: module sigs.k8s.io/kubebuilder/testdata/project-v4-with-grafana
Initializing project ...
Writing kustomize manifests for you to edit...
Writing scaffold for you to edit...
Get controller runtime:
$ go get sigs.k8s.io/controller-runtime@v0.14.4
Update dependencies:
$ go mod tidy
Next: define a resource with:
$ kubebuilder create api
Editing project with Grafana plugin ...
Generating Grafana manifests to visualize controller status...
mkdir -p /Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-grafana/bin
test -s /Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-grafana/bin/controller-gen && /Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-grafana/bin/controller-gen --version | grep -q v0.12.0 || \
        GOBIN=/Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-grafana/bin go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.12.0
/Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-grafana/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
/Users/jh/GoProjects/kubebuilder/testdata/project-v4-with-grafana/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
~/GoProjects/kubebuilder
go run hack/docs/generate_samples.go
Generating documents...
Generating component-config tutorial...
INFO[0000] Generating the sample context of component-config... 
INFO[0000] destroying directory for sample project      
  running: docker rmi -f 
  warning: unlinkat docs/book/src/component-config-tutorial/testdata/project/bin/k8s/1.27.1-darwin-amd64/etcd: permission denied
INFO[0000] refreshing tools and creating directory...   
  cleaning up tools
  preparing testing directory: docs/book/src/component-config-tutorial/testdata/project/
INFO[0000] Initializing the project                     
  running: kubebuilder init --domain tutorial.kubebuilder.io --repo tutorial.kubebuilder.io/project --license apache2 --owner The Kubernetes authors --plugins=go/v4 --component-config
ERRO[0000] error Initializing the project: kubebuilder init --domain tutorial.kubebuilder.io --repo tutorial.kubebuilder.io/project --license apache2 --owner The Kubernetes authors --plugins=go/v4 --component-config failed with error: (exit status 1) Error: no plugin could be resolved with key "go/v4"
Usage:
  kubebuilder [flags]

Examples:
The first step is to initialize your project:
    kubebuilder init [--plugins=<PLUGIN KEYS> [--project-version=<PROJECT VERSION>]]

<PLUGIN KEYS> is a comma-separated list of plugin keys from the following table
and <PROJECT VERSION> a supported project version for these plugins.

                        Plugin keys | Supported project versions
------------------------------------+----------------------------
          base.go.kubebuilder.io/v3 |                          3
   declarative.go.kubebuilder.io/v1 |                       2, 3
               go.kubebuilder.io/v2 |                       2, 3
               go.kubebuilder.io/v3 |                          3
 kustomize.common.kubebuilder.io/v1 |                          3

For more specific help for the init command of a certain plugins and project version
configuration please run:
    kubebuilder init --help --plugins=<PLUGIN KEYS> [--project-version=<PROJECT VERSION>]

Default plugin keys: "go.kubebuilder.io/v3"
Default project version: "3"


Flags:
  -h, --help                     help for kubebuilder
      --plugins strings          plugin keys to be used for this subcommand execution
      --project-version string   project version (default "3")

2023/05/12 16:27:07 no plugin could be resolved with key "go/v4" 
exit status 1
make: *** [generate-docs] Error 1
➜  kubebuilder git:(master) ✗

@camilamacedo86
Copy link
Member

camilamacedo86 commented May 12, 2023

Hi @hangscer8,

See that we run the make generate to verify the PRs, and it is the CI testdata so it is very unlike it get breaks since it means that the CI would fail and we would not get the PR merged. See: https://github.com/kubernetes-sigs/kubebuilder/actions/runs/4947303697/jobs/8846494334?pr=3407

Anyway, I tested it locally, and it all worked fine.

Please, ensure that:

@hangscer8
Copy link
Member Author

hangscer8 commented May 16, 2023

Thank you for your notes @camilamacedo86 , And I will figure it out locally.

SKIP_KIND_CLEANUP=1 make test-e2e-local

@hangscer8 hangscer8 closed this May 19, 2023
@camilamacedo86
Copy link
Member

Hi @hangscer8,

You do not need to run make test-e2e-local for this scenario.
You just need to run make generate which will re-generate the scaffolds based on the change.

Also, I notice that you close this one.
Is that because you could not make it work after run make generate?

@cheina97
Copy link

cheina97 commented Oct 3, 2023

Some news about this fix? We are having trouble generating a CRD called PublicKey. If nobody is working on it I can take care of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kubernetes and Kubebuilder use different mechanisms to generate plurals
4 participants