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.

Sign up for GitHub

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

Jump to bottom

Rethinking metadata schema generation in CRD yamls #385

Closed
tamalsaha opened this issue Jan 3, 2020 · 19 comments
Closed

Rethinking metadata schema generation in CRD yamls #385

tamalsaha opened this issue Jan 3, 2020 · 19 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@tamalsaha
Copy link
Contributor

Currently controller-tools does not generate any properties for metadata fields at any level in CRD yamls to accommodate for structural schema.

https://github.com/kubernetes-sigs/controller-tools/blob/master/pkg/crd/known_types.go#L29-L33

My it is my understanding that only top level metadata field is restricted to define name & generatedName fields. Any other nested metadata field can have detailed schema. Some common cases are embedding PodTemplate, PersistentVolumeClaim in CRDs.

Currently we are creating our own ObjectMeta types are using them in our CRDs to work around this issue.

Do you think controller-tools could just strip the properties for top level metadata field? One way to do that will be to remove ObjectMeta from known types and then clean the generated yaml before writing to disk.

Any thoughts?
@tamalsaha
Copy link
Contributor Author

/sig api-machinery

cc: @sttts @munnerz @DirectXMan12

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jan 3, 2020
@sttts
Copy link
Contributor

sttts commented Jan 3, 2020

I would prefer to see a stripped down EmbeddedObjectMeta if possible, or at least let controller-gen do the restriction. ObjectMeta has so many fields which do not make sense for embedded ObjectMeta. Also the schema is super huge.

@tamalsaha
Copy link
Contributor Author

I like the idea of EmbeddedObjectMeta. We kind of doing that in our custom type. https://github.com/kmodules/offshoot-api/blob/master/api/v1/pvc.go#L24-L98

@DirectXMan12
Copy link
Contributor

/help

@k8s-ci-robot
Copy link
Contributor

@DirectXMan12:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

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 the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jan 6, 2020
@arjunrn arjunrn mentioned this issue Jan 26, 2020
Closed
@pgier pgier mentioned this issue Feb 25, 2020
Closed
@pgier pgier mentioned this issue Mar 27, 2020
Closed
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 6, 2020
@pgier pgier mentioned this issue Apr 8, 2020
Merged
@DirectXMan12
Copy link
Contributor

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 20, 2020
@DirectXMan12 DirectXMan12 mentioned this issue Jun 10, 2020
Closed
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 19, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 18, 2020
@mikkeloscar
Copy link

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 26, 2020
@yiqigao217 yiqigao217 mentioned this issue Sep 1, 2020
Closed
@DirectXMan12
Copy link
Contributor

I think #395 is actually close to being mergable, it's just a bit out of date. If someone wants to pick that up, I've left comments on it.

@sttts
Copy link
Contributor

sttts commented Sep 14, 2020

@DirectXMan12 see my comment in 395. IMO we should not do that. Instead let's add a sensible EmbeddedObjectMeta to API machinery and make controller-tools warn if that is not used instead of ObjectMeta.

@DirectXMan12
Copy link
Contributor

@sttts that doesn't work for when you embed things with ObjectMeta in them already though -- e.g. PodTemplateSpec, which is probably the most common case. We'd have to swap that out on demand or something.

@sttts
Copy link
Contributor

sttts commented Sep 22, 2020

True. So we must add special handling for embedded ObjectMeta to only specifiy sensible fields (more than just type: object but considerably less than a full ObjectMeta. It's ugly. As written in the other issue: sin of the past :-/

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 21, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 20, 2021
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

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.

@ese ese mentioned this issue Jan 17, 2022
Merged
amisevsk added a commit to amisevsk/devworkspace-api that referenced this issue Jun 16, 2022
@amisevsk
Duplicate metadata fields for pods spec override
dfc7f70 
Controller-gen cannot generate embedded metadata fields, resulting in
those fields deserializing to empty objects. In order to embed metadata
in a CRD, it is necessary to duplicate metadata fields where
appropriate.

See issue: kubernetes-sigs/controller-tools#385
for details

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
amisevsk added a commit to amisevsk/devworkspace-api that referenced this issue Jul 27, 2022
@amisevsk
Duplicate metadata fields for pods spec override
a7d2a69 
Controller-gen cannot generate embedded metadata fields, resulting in
those fields deserializing to empty objects. In order to embed metadata
in a CRD, it is necessary to duplicate metadata fields where
appropriate.

See issue: kubernetes-sigs/controller-tools#385
for details

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@foot foot mentioned this issue Aug 1, 2022
Merged
amisevsk added a commit to amisevsk/devworkspace-api that referenced this issue Sep 19, 2022
@amisevsk
Duplicate metadata fields for pods spec override
66a50c0 
Controller-gen cannot generate embedded metadata fields, resulting in
those fields deserializing to empty objects. In order to embed metadata
in a CRD, it is necessary to duplicate metadata fields where
appropriate.

See issue: kubernetes-sigs/controller-tools#385
for details

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
amisevsk added a commit to devfile/api that referenced this issue Sep 19, 2022
@amisevsk
Duplicate metadata fields for pods spec override
955ffb6 
Controller-gen cannot generate embedded metadata fields, resulting in
those fields deserializing to empty objects. In order to embed metadata
in a CRD, it is necessary to duplicate metadata fields where
appropriate.

See issue: kubernetes-sigs/controller-tools#385
for details

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@EronWright
Copy link

For posterity: if you're developing a CRD with a spec field that uses corev1.PodTemplate, add the generateEmbeddedObjectMeta=true option to your makefile as shown here: openshift/boilerplate#291

This will ensure that your CRD has a schema for ObjectMeta, so that your users may apply labels/annotations to the pod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@tamalsaha @DirectXMan12 @mikkeloscar @sttts @EronWright @k8s-ci-robot @fejta-bot