Use code-generation for CRD API and deepcopy methods #369
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Right now we only use autogenerated code for deepcopy and only for the Postgresql and PostgreqlList objects. Moved Postgresql, PostgresqlList and other types used by them into the apis/acid.zalan.do/v1 package. Error field in the CRD has been replaced by a string, since Error didn't work nicely with the deepcopy-gen.
Remove some of the boilerplate code. Sovle the issue with PostgreSQL CRD kind not matching the expectations in the generated code due to the case difference.
Move generated code to /generated. Rename postgres-operator-configuration CRD to OperatorConfiguration for compatibility with generated code. The only missing bit that requires the old code is updating the status
golangci
reviewed
Aug 10, 2018
pkg/apis/acid.zalan.do/v1/util.go
Outdated
| return metav1.Time{}, err | ||
| } | ||
|
|
||
| return metav1.Time{tp.UTC()}, nil |
There was a problem hiding this comment.
github.com/zalando-incubator/postgres-operator/vendor/k8s.io/apimachinery/pkg/apis/meta/v1.Time composite literal uses unkeyed fields
Right now we set the status with Update, however, Kubernetes 1.11 introduces the /status subresources, allowing us to set the status with the special updateStatus call in the future. For now, we keep the code that is compatible with earlier versions of Kubernetes. As an upside, remove a lot of custom code for building the REST client for CRDs, as this is now handled by the autogenerated API. We could remove the custom unmarshaller as well, once we figure out how CRD validation. Also renamed postgresql.go to the database go (code that works with databases, to avoid confusion with the code that deals with PostgreSQL objects) and status.go to logs_and_api.go to distinguish between the code that deals with PostgreSQL object status and the one that performs logging and API utilities.
golangci
reviewed
Aug 13, 2018
pkg/cluster/cluster.go
Outdated
| c.Status = status | ||
| b, err := json.Marshal(status) | ||
| func (c *Cluster) setStatus(status acidv1.ClusterStateType) { | ||
| c.Status = acidv1.PostgresStatus{status} |
There was a problem hiding this comment.
github.com/zalando-incubator/postgres-operator/pkg/apis/acid.zalan.do/v1.PostgresStatus composite literal uses unkeyed fields
Need to maintain compatiblity with already running clusters. In addition, revert to patch for changing the status instead of a full-scale update, as there is no guarantee the update will supply the actual resourceVersion.
golangci
reviewed
Aug 14, 2018
pkg/cluster/cluster.go
Outdated
| err error | ||
| b []byte | ||
| ) | ||
| b, err = json.Marshal(status) |
Minor reformatting and renaming.
Generate only GET verbs and no status.
alexeyklyukin
changed the title
sdudoladov
requested changes
Aug 15, 2018
There was a problem hiding this comment.
- Can you please leave a short comment in the developer docs about the overall code generation idea, benefits, code structure and must-have links to the k8s docs besides other operators ?
- Let's say we introduce new parts of the API, say v2. Would it be sufficient to create v2 directory under the API group dir, describe interfaces there with appropriate annotations and then invoke code generation ?
| package acidzalando | ||
|
|
||
| const ( | ||
| // GroupName is the group name for the operator CRDs |
There was a problem hiding this comment.
is API group name implied here ?
| Error string `json:"-"` | ||
| } | ||
|
|
||
| // PostgresSpec defines the specification for the PostgreSQL TPR. |
| @@ -11,13 +11,13 @@ import: | |||
| - package: github.com/lib/pq | |||
| - package: github.com/motomux/pretty | |||
| - package: k8s.io/apimachinery | |||
| version: kubernetes-1.11.1 | |||
| version: kubernetes-1.11.3-beta.0 | |||
There was a problem hiding this comment.
- versions have to be specified as precisely as possible , e.g. kubernetes-1.11.3-beta.0 and not ^8.0.0
- versions for
client-go,code-generatorandapimachineryhave to match
sdudoladov
approved these changes
Aug 15, 2018
sdudoladov
reviewed
Aug 16, 2018
| @@ -12,7 +12,7 @@ configuration. | |||
|
|
|||
| * CRD-based configuration. The configuration is stored in the custom YAML | |||
| manifest, an instance of the custom resource definition (CRD) called | |||
| `postgresql-operator-configuration`. This CRD is registered by the operator | |||
| `OperatorConfiguration`. This CRD is registered by the operator | |||
There was a problem hiding this comment.
This renaming was necessary because the older name with dashes broke the code generation process.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Client-go provides a https://github.com/kubernetes/code-generator package in order to provide the API to work with CRDs similar to the one available for built-in types, i.e. Pods, Statefulsets and so on.
Use this package to generate deepcopy methods (required for CRDs), instead of using an external deepcopy package; we also generate APIs used to manipulate both Postgres and OperatorConfiguration CRDs, as well as informers and listers for the Postgres CRD, instead of using generic informers and CRD REST API; by using generated code we can get rid of some custom and obscure CRD-related code and use a better API.