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

✨ chore: add ginkgolinter #3894

Merged
merged 1 commit into from
May 24, 2024
Merged

Conversation

PG2000
Copy link
Contributor

@PG2000 PG2000 commented Apr 30, 2024

In order to have checks for some common errors in test files
this enables the built-in ginkgolinter.

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

Welcome @PG2000!

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

Hi @PG2000. 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 needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 30, 2024
@camilamacedo86
Copy link
Member

/ok-to-test

@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 May 2, 2024
camilamacedo86
camilamacedo86 previously approved these changes May 5, 2024
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 🥇

I think we also need to:

  • a) add to Kubebuilder code

Can you please add it to: https://github.com/kubernetes-sigs/kubebuilder/blob/master/.golangci.yml

  • b) After any change in the scaffolds we MUST to run make generate.
    This PR will fail in the check testdata sample since the samples under testdata and used for the docs are not updated with the change.

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 5, 2024
@camilamacedo86 camilamacedo86 removed lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 5, 2024
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 6, 2024
@PG2000 PG2000 closed this May 6, 2024
@PG2000 PG2000 reopened this May 6, 2024
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.

It still missing:

a) You run make generate so that your changes are applied in the samples
b) Please squash all commits so that we can get it merge

@PG2000
Copy link
Contributor Author

PG2000 commented May 8, 2024

Unfortunately make generate does not work from my machines. Both are Macs (Intel and M1).
Any hint for this.

Console ouput
➜  kubebuilder git:(patch-1) ✗ make generate
go: creating new go.mod: module sigs.k8s.io/kubebuilder/testdata/project-v2
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.

INFO Writing scaffold for you to edit...          
INFO Get controller runtime:
$ go get sigs.k8s.io/controller-runtime@v0.6.4
INFO Update dependencies:
$ go mod tidy           
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.

INFO Writing scaffold for you to edit...          
INFO api/v1/captain_types.go                      
INFO controllers/captain_controller.go            
INFO Update dependencies:
$ go mod tidy           
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.

INFO Writing scaffold for you to edit...          
INFO api/v1/captain_types.go                      
INFO controllers/captain_controller.go            
INFO Update dependencies:
$ go mod tidy           
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.

INFO Writing scaffold for you to edit...          
INFO api/v1/captain_webhook.go                    
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.

INFO Writing scaffold for you to edit...          
INFO api/v1/firstmate_types.go                    
INFO controllers/firstmate_controller.go          
INFO Update dependencies:
$ go mod tidy           
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.

INFO Writing scaffold for you to edit...          
INFO api/v1/firstmate_webhook.go                  
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.

INFO Writing scaffold for you to edit...          
INFO api/v1/admiral_types.go                      
INFO controllers/admiral_controller.go            
INFO Update dependencies:
$ go mod tidy           
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.

INFO Writing scaffold for you to edit...          
INFO api/v1/admiral_webhook.go                    
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.

INFO Writing scaffold for you to edit...          
INFO controllers/laker_controller.go              
INFO Update dependencies:
$ go mod tidy           
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x59c72de]

goroutine 121 [running]:
go/types.(*Checker).handleBailout(0xc0002a8c00, 0xc000a33d40)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/check.go:367 +0x88
panic({0x5c887c0?, 0x6204200?})
/usr/local/Cellar/go/1.22.0/libexec/src/runtime/panic.go:770 +0x132
go/types.(*StdSizes).Sizeof(0x0, {0x5d4abc8, 0x620c460})
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/sizes.go:228 +0x31e
go/types.(*Config).sizeof(...)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/sizes.go:333
go/types.representableConst.func1({0x5d4abc8?, 0x620c460?})
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/const.go:76 +0x9e
go/types.representableConst({0x5d50998, 0x61d9380}, 0xc0002a8c00, 0x620c460, 0x0)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/const.go:92 +0x192
go/types.(*Checker).arrayLength(0xc0002a8c00, {0x5d4ee70, 0xc000a6db80?})
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/typexpr.go:510 +0x2d3
go/types.(*Checker).typInternal(0xc0002a8c00, {0x5d4d760, 0xc000f26a80}, 0x0)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/typexpr.go:299 +0x49d
go/types.(*Checker).definedType(0xc0002a8c00, {0x5d4d760, 0xc000f26a80}, 0xc000a33328?)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/typexpr.go:180 +0x37
go/types.(*Checker).varType(0xc0002a8c00, {0x5d4d760, 0xc000f26a80})
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/typexpr.go:145 +0x25
go/types.(*Checker).structType(0xc0002a8c00, 0xc000f3ca50, 0xc000f3ca50?)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/struct.go:113 +0x19f
go/types.(*Checker).typInternal(0xc0002a8c00, {0x5d4d6d0, 0xc000ae5a40}, 0xc000f24ff0)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/typexpr.go:316 +0x1345
go/types.(*Checker).definedType(0xc0002a8c00, {0x5d4d6d0, 0xc000ae5a40}, 0x5ae17d0?)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/typexpr.go:180 +0x37
go/types.(*Checker).typeDecl(0xc0002a8c00, 0xc000f24ff0, 0xc000f32100, 0x0)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/decl.go:615 +0x44d
go/types.(*Checker).objDecl(0xc0002a8c00, {0x5d55758, 0xc000f24ff0}, 0x0)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/decl.go:197 +0xa7f
go/types.(*Checker).packageObjects(0xc0002a8c00)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/resolver.go:681 +0x425
go/types.(*Checker).checkFiles(0xc0002a8c00, {0xc001080708, 0x3, 0x3})
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/check.go:408 +0x1a5
go/types.(*Checker).Files(...)
/usr/local/Cellar/go/1.22.0/libexec/src/go/types/check.go:372
sigs.k8s.io/controller-tools/pkg/loader.(*loader).typeCheck(0xc0001fa990, 0xc000e7fae0)
/Users/pg2000/go/pkg/mod/sigs.k8s.io/controller-tools@v0.3.0/pkg/loader/loader.go:283 +0x35b
sigs.k8s.io/controller-tools/pkg/loader.(*Package).NeedTypesInfo(0xc000e7fae0)
/Users/pg2000/go/pkg/mod/sigs.k8s.io/controller-tools@v0.3.0/pkg/loader/loader.go:96 +0x39
sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check(0xc000b92450, 0xc000e7fae0)
/Users/pg2000/go/pkg/mod/sigs.k8s.io/controller-tools@v0.3.0/pkg/loader/refs.go:249 +0x277
sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check.func1(0x59?)
/Users/pg2000/go/pkg/mod/sigs.k8s.io/controller-tools@v0.3.0/pkg/loader/refs.go:243 +0x53
created by sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check in goroutine 83
/Users/pg2000/go/pkg/mod/sigs.k8s.io/controller-tools@v0.3.0/pkg/loader/refs.go:241 +0x18c
make[1]: *** [generate] Error 2
make: *** [generate-testdata] Error 2
➜  kubebuilder git:(patch-1) ✗ 

@camilamacedo86
Copy link
Member

Hi @PG2000

Unfortunately make generate does not work from my machines. Both are Macs (Intel and M1).
Any hint for this.

I do not have a M1, however, it should work well as well.
The make generate will just bin the binary, and use it to re-generate the samples and update the docs.
Therefore, at the same way that you are able to use Kubebuilder CLI in your machine it only automates the usage operations with a bin built from your local env/wth the changes.

it has no reason for it does not work at all.
if you are facing an issue, could you please raise an issue with the details?

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.

Can you please squash the commits and rebase?

@camilamacedo86 camilamacedo86 removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2024
@camilamacedo86 camilamacedo86 self-requested a review May 20, 2024 08:32
@camilamacedo86 camilamacedo86 dismissed their stale review May 20, 2024 08:33

not approved yet. required to run make generate and squash commits as rebase with master

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.

Missing rebase, squash and run make generate

@PG2000
Copy link
Contributor Author

PG2000 commented May 22, 2024

Can you please squash the commits and rebase?

done. Fortunately the make generate works now.

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.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, PG2000

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 May 22, 2024
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.

@PG2000

Cool now you can check the lint erros.
We need to fix them in this PR to get it merged

You can run locally make lint as well to do the tests.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 23, 2024
@PG2000
Copy link
Contributor Author

PG2000 commented May 23, 2024

@PG2000

Cool now you can check the lint erros. We need to fix them in this PR to get it merged

You can run locally make lint as well to do the tests.

done

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.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2024
@k8s-ci-robot k8s-ci-robot merged commit 6c08ed1 into kubernetes-sigs:master May 24, 2024
17 checks passed
@PG2000 PG2000 deleted the patch-1 branch May 24, 2024 06:16
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. 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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants