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

Update controller-gen and remove our code generation workarounds #4015

Merged
merged 8 commits into from
Oct 25, 2024

Conversation

cbandy
Copy link
Member

@cbandy cbandy commented Oct 19, 2024

Recent controller-gen does everything we had to do with Ruby and Kustomize.

This removes the build/crd, config/singlenamespace, and installers directories. In their place, I've added two validation rules to most of our PVC specs. The pgAdmin APIs don't have these rules, and adding them causes a test to fail.

Checklist:

  • Have you added an explanation of what your changes do and why you'd like them to be included?
  • Have you updated or added documentation for the change, as applicable?
  • Have you tested your changes on all related environments with successful results, as applicable?
    • Have you added automated tests?

Type of Changes:

  • Other

Other Information:

$ make install
kubectl apply --server-side -k ./config/crd
customresourcedefinition.apiextensions.k8s.io/crunchybridgeclusters.postgres-operator.crunchydata.com serverside-applied
customresourcedefinition.apiextensions.k8s.io/pgadmins.postgres-operator.crunchydata.com serverside-applied
customresourcedefinition.apiextensions.k8s.io/pgupgrades.postgres-operator.crunchydata.com serverside-applied
customresourcedefinition.apiextensions.k8s.io/postgresclusters.postgres-operator.crunchydata.com serverside-applied

# or make deploy-dev

$ kubectl get crd -l app.kubernetes.io/name=pgo -L app.kubernetes.io/version
NAME                                                      CREATED AT             VERSION
crunchybridgeclusters.postgres-operator.crunchydata.com   2024-10-19T02:36:33Z   latest
pgadmins.postgres-operator.crunchydata.com                2024-10-19T02:36:33Z   latest
pgupgrades.postgres-operator.crunchydata.com              2024-10-19T02:36:33Z   latest
postgresclusters.postgres-operator.crunchydata.com        2024-10-19T02:36:33Z   latest

internal/patroni/rbac.go Show resolved Hide resolved
config/crd/kustomization.yaml Show resolved Hide resolved
@cbandy cbandy enabled auto-merge (rebase) October 22, 2024 14:01
Copy link
Contributor

@benjaminjb benjaminjb left a comment

Choose a reason for hiding this comment

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

Seems a little funny not to mention singlenamespace, but I suppose we do that elsewhere, e.g., pgo-examples

@cbandy cbandy disabled auto-merge October 23, 2024 15:55
Comment on lines -3104 to +3113
required:
- accessModes
- resources
type: object
x-kubernetes-validations:
- message: missing accessModes
rule: has(self.accessModes) && size(self.accessModes)
> 0
- message: missing storage request
rule: has(self.resources) && has(self.resources.requests)
&& has(self.resources.requests.storage)
Copy link
Member Author

@cbandy cbandy Oct 23, 2024

Choose a reason for hiding this comment

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

🤔 This affects the API reference documentation. The required column changes from yes to no.

kubectl explain also calls out required:

$ kubectl explain postgresclusters.spec.instances.dataVolumeClaimSpec

FIELDS:
  accessModes   <[]string> -required-
    accessModes contains the desired access modes the volume should have.

andrewlecuyer and others added 8 commits October 23, 2024 17:59
Updates to the latest controller-gen release.  CRDs and RBAC have been
regenerated, and "namespace" has been removed from the markers in the
Patroni and pgBackRest Go files (it was no longer providing much benefit
since the go code already cleanly organizes the RBAC, and changes to controller
controller-gen had the potential to break RBAC generation as a result of its use).

Issue: PGO-1748
These are not compatible with Red Hat's current requirements.

Issue: PGO-1046
Recent controller-gen does the resource consolidation for us, and recent
Kustomize can change ClusterRole to Role with a patch directive.

Ruby is no longer required during development!

Issue: PGO-1748
Recent controller-gen does this for us.

Issue: PGO-1748
The JSON patch was awkward to maintain, and we forgot to update it when
we added PVCs to our APIs.

I considered defining these rules on a shared Go type in our API
package, but I did not like the type conversion it requires in our
controller and test code.

Issue: PGO-1748
This is easier to manage in one place and is the last modification we're
making to CRDs as they are generated. A future commit is free to remove
the Kutomiziations entirely.

Issue: PGO-1046
Issue: PGO-1748
Recent versions of controller-gen are able to describe our CRDs!

Issue: PGO-1748
@cbandy cbandy merged commit 446c9c7 into CrunchyData:master Oct 25, 2024
16 checks passed
@cbandy cbandy deleted the controller-gen branch October 25, 2024 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants