Code organization round 2 including no more duplicate CRD files by lauralorenz · Pull Request #35 · kubernetes-sigs/about-api

lauralorenz · 2026-03-04T04:55:41Z

This PR

"unrolls" things that were still in the clusterproperty/ directory that had not been elevated to the root directory during PR Organizes API structure and enable Go module #12
- More info: At the time of PR Organizes API structure and enable Go module #12, a bunch of config/ directories, main.go and the other controller files, Dockerfile and Makefile did not get moved out into the root folder, assumedly because they are all only useful/used by the controller so were not immediately important to users of the CRD. Also ONLY the output CRD yaml came out of config/crd , not any of the other coexisting stuff, which includes the ways to add kustomize patches, the most important of which (for now at least) is the k8s.io protection patch since IIRC the CRD should be rejected by API server as invalid when applied without it.
Maintains the changes that were made by @holgerson97 in the clusterproperty/ directory over the past few PRs of theirs (including to the Makefile, Dockerfile, and types) and by @qiujian16 and @RainbowMango to the pkg/apis/v1alpha1 version. I did this by going commit by commit through https://github.com/kubernetes-sigs/about-api/commits/master/clusterproperty and https://github.com/kubernetes-sigs/about-api/commits/master/pkg since 2022 and personally making sure those changes were incorporated. (All other files had not been touched in 4+ years and since prior to the code reorganization in Organizes API structure and enable Go module #12, or were not going to be touched by this change, like OWNERS).
I've confirmed that most of the Makefile functions, including make run, deploy/undeploy, install/uninstall, docker-build and manifests/generate.
At times, particularly related to the controller files, I needed to make a judgement call on code organization, and I defaulted to the current Kubebuilder v4 of which I generated a clean init project to compare the code tree with, looks like this:

laura:~/go/src/kubernetes-sigs/test-kubebuilder$ 
tree -L 2
.
├── Dockerfile
├── Makefile
├── PROJECT
├── README.md
├── api
│   └── v1alpha1
├── bin
│   ├── controller-gen -> /home/lauralorenz_google_com/go/src/kubernetes-sigs/test-kubebuilder/bin/controller-gen-v0.18.0
│   └── controller-gen-v0.18.0
├── cmd
│   └── main.go
├── config
│   ├── crd
│   ├── default
│   ├── manager
│   ├── network-policy
│   ├── prometheus
│   ├── rbac
│   └── samples
├── go.mod
├── go.sum
├── hack
│   └── boilerplate.go.txt
├── internal
│   └── controller
└── test
    ├── e2e
    └── utils

There are still a few things wrong with the organization of the tests and make test, but I think this is good enough on its own and we can follow on with that
The weirdest thing still going on right now is that there are two different code generation toolchains being used, one for v1alpha1 (k/k code generation pattern, using a ./hack/update-codegen.sh kind of pattern) and one for v1beta1 (controller-runtime CLI based). I've left them both in for now and the Makefile does both, but I think that could be the next thing to tidy up.

Fixes #28

Also pulls up all changes that had been made into clusterproperty/ since 2022, which happened to all have been in that directory. Signed-off-by: Laura Lorenz <lauralorenz@google.com>

Signed-off-by: Laura Lorenz <lauralorenz@google.com>

k8s-ci-robot · 2026-03-04T04:55:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lauralorenz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [lauralorenz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

lauralorenz

A few notes for context for reviewers.

lauralorenz · 2026-03-04T04:57:41Z

api/v1alpha1/clusterproperty_types.go

 }

+//+genclient
+//+genclient:nonNamespaced


From PR #22 and #15

lauralorenz · 2026-03-04T04:58:18Z

api/v1alpha1/clusterproperty_types.go

 //+kubebuilder:resource:scope=Cluster

-// ClusterProperty is the Schema for the clusterproperties API
+// ClusterProperty is a resource provides a way to store identification related,


Noticed these were expanded in #12

lauralorenz · 2026-03-04T04:59:27Z

cmd/main.go

-		MetricsBindAddress:     metricsAddr,
-		Port:                   9443,
+		Scheme: scheme,
+		Metrics: metricsserver.Options{


Struct edited because of bump to controller-runtime v0.17.0

lauralorenz · 2026-03-04T05:00:51Z

config/crd/kustomization.yaml

@@ -5,7 +5,6 @@ resources:
 - bases/about.k8s.io_clusterproperties.yaml
 #+kubebuilder:scaffold:crdkustomizeresource

-patchesStrategicMerge:


Automated changes from kustomize edit fix due to update of kustomize

lauralorenz · 2026-03-04T05:01:08Z

config/default/kustomization.yaml

@@ -12,10 +12,6 @@ namePrefix: clusterproperty-
 #commonLabels:
 #  someName: someValue

-bases:


Also all kustomize edit fix generated updates

lauralorenz · 2026-03-04T05:02:01Z

config/manager/manager.yaml

        args:
        - --leader-elect
        image: controller:latest
+        imagePullPolicy: IfNotPresent


While testing easier to force it to use local image I've build/kind loaded.

lauralorenz · 2026-03-04T05:03:23Z

hack/update-codegen.sh

-GO111MODULE=on go install "k8s.io/code-generator/cmd/client-gen"
-GO111MODULE=on go install "k8s.io/code-generator/cmd/lister-gen"
-GO111MODULE=on go install "k8s.io/code-generator/cmd/informer-gen"
+GO111MODULE=on go install "sigs.k8s.io/controller-tools/cmd/controller-gen@v0.16.5"


Lots of back and forth trying not to update controller-gen but in the end I did have to since go.mod had already updated apimachinery and client-go to v0.29.1 and there were incompatabilities; also pinned the others to 0.29.1 based on recommendation from gemini for compatibility with go 1.22 (which is our current min version)

lauralorenz · 2026-03-04T05:06:24Z

go.mod

@@ -1,69 +1,81 @@
-module sigs.k8s.io/about-api


Module should be sigs.k8s.io/about-api actually.... will have to come back for this one

By “come back for this one”, do you mean revisit in a later PR, or revisit in an update to this PR? I would rather avoid such an intrusive change in this PR.

I wasn't committed to either way but I will follow your advice and do it separate, though I have started. I have a separate branch, one can see the diff with this branch at lauralorenz#1 for convenience. It's getting a little hairy over there because the v1alpha1 codegen is a lot pickier about paths, so it's best to have it for a later time anyways.

Oh, I meant I’d rather fix the module name in this PR!

I've got this change now in another-another branch, visible at lauralorenz#2, but it's pretty big and I'm not 100% sure all of the code generation is correct but I could use a new set of eyes on it.

Riffing off that branch, I’ve updated the codegen and the clientset/informers etc. are now correct: https://github.com/skitt/about-api/tree/code-organization-round-2b

Could cherry pick this into this branch <== i will do this

Could merge this and then stephen open a PR

lauralorenz · 2026-03-10T17:05:44Z

/hold

Per SIG-MC discussion 3/10, I'm addressing the module path concern in #35 (comment), and likely collapsing the separate codegen formats into one for sanity, before looking for any more reviews on this right now.

MrFreezeex · 2026-03-31T17:19:05Z

This looks pretty good to me overall happy to leave a lgtm once the go.mod thing / this "sub PR" lauralorenz#2 have been sorted out 🎉!

Thanks Laura for the work and for splitting this in comprehensive commits and leaving some context comments about a few changes 🙏

lauralorenz · 2026-04-01T16:07:14Z

TODO:

Code organization round 2 including no more duplicate CRD files #35 (comment) to update codegen
double check about make test / env test and if we want to clean that up rn

lauralorenz added 8 commits March 4, 2026 04:14

Unroll the rest of clusterproperty/ to match kubebuilder v4

9134594

Also pulls up all changes that had been made into clusterproperty/ since 2022, which happened to all have been in that directory. Signed-off-by: Laura Lorenz <lauralorenz@google.com>

Make generate, was forced to update some deps

4406743

Signed-off-by: Laura Lorenz <lauralorenz@google.com>

Kustomize automatic migration to newer version

8c10f5c

Signed-off-by: Laura Lorenz <lauralorenz@google.com>

Update controller due to new deps, also to v1beta1

ac2a241

Signed-off-by: Laura Lorenz <lauralorenz@google.com>

go fmt

4c84d17

Signed-off-by: Laura Lorenz <lauralorenz@google.com>

Remove excess directory now

2d8fd8c

Signed-off-by: Laura Lorenz <lauralorenz@google.com>

Add in docstrings from PR kubernetes-sigs#12

4fb58ac

Signed-off-by: Laura Lorenz <lauralorenz@google.com>

Merge prior pkg/api/v1alpha1 changes and regenerate

470444e

Signed-off-by: Laura Lorenz <lauralorenz@google.com>

k8s-ci-robot requested review from JeremyOT and skitt March 4, 2026 04:55

k8s-ci-robot added the sig/multicluster Categorizes an issue or PR as relevant to SIG Multicluster. label Mar 4, 2026

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 4, 2026

lauralorenz commented Mar 4, 2026

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 10, 2026

Conversation

lauralorenz commented Mar 4, 2026

Uh oh!

k8s-ci-robot commented Mar 4, 2026

Uh oh!

lauralorenz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lauralorenz commented Mar 10, 2026

Uh oh!

MrFreezeex commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lauralorenz commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MrFreezeex commented Mar 31, 2026 •

edited

Loading