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

java-model-gen generates CRD that imports non-exist class #921

Closed
codeyq opened this issue Apr 17, 2020 · 18 comments
Closed

java-model-gen generates CRD that imports non-exist class #921

codeyq opened this issue Apr 17, 2020 · 18 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@codeyq
Copy link

codeyq commented Apr 17, 2020

I followed https://github.com/kubernetes-client/java/blob/master/docs/generate-model-from-third-party-resources.md to generate Java client for CRD. However, the generated file imports the following class which doesn't exist in generated file nor library

import io.kubernetes.client.models.IoK8sApimachineryPkgApisMetaV1ObjectMeta;

Actually, should I just replace it with the following?

import io.kubernetes.client.openapi.models.V1ObjectMeta;

By the way, any plan to productionize yue9944882/java-model-gen docker image in this repo?

@brendandburns
Copy link
Contributor

Yeah, probably. There is a pre-processor here:

https://github.com/kubernetes-client/gen/blob/master/openapi/preprocess_spec.py

That massages the swagger spec to do things like this.

We're open to productionizing that script/image but I'm not sure anyone has the cycles to do it right now.

@codeyq
Copy link
Author

codeyq commented Apr 18, 2020

@brendandburns Thanks. Replacing to V1ObjectMeta works. However, I found there is another issue with yue9944882/java-model-gen. Let's say we got the following files from the code gen

V1Crd.java
V1CrdList.java
V1CrdListMetadata.java
V1CrdSpec.java

When I try to run the controller, i got the following error message.

java.lang.ClassCastException: io.kubernetes.client.models.V1CrdListMetadata cannot be cast to io.kubernetes.client.openapi.models.V1ListMeta
	at io.kubernetes.client.util.ListAccessor.listMetadata(ListAccessor.java:18)
	at io.kubernetes.client.informer.cache.ReflectorRunnable.run(ReflectorRunnable.java:47)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)

Apparently V1CrdListMetadata cannot be cast to V1ListMeta. Replacing V1CrdListMetadatas in V1CrdList.java to V1ListMeta resolved the issue. Is this expected in yue9944882/java-model-gen?
By the way, I would be more than happy to help productionizing the script/image if there is anything that I can do.

@brendandburns
Copy link
Contributor

cc @yue9944882

Not sure what the plans to productionize it are, but if you wanted to improve it and submit a PR to this repo that would be awesome.

@yue9944882
Copy link
Member

import io.kubernetes.client.models.IoK8sApimachineryPkgApisMetaV1ObjectMeta;
Actually, should I just replace it with the following?
import io.kubernetes.client.openapi.models.V1ObjectMeta;

Exactly. There're a few manual changes have to be done after the code-generation, at least you're not only required to replace V1ObjectMeta but also V1ListMeta.

By the way, I would be more than happy to help productionizing the script/image if there is anything that I can do.

There was plan to enhance the java model generator. @brendandburns I was thinking about introducing java models for other commonly-used repos e.g. kubefed, istio, helm. etc (probably in a separated module named client-java-model-contrib) and manage/upgrade them automatically w/ the help of that generator . That will require the java model generator to be working smoothly (w/o manual follow-ups)..

@codeyq
Copy link
Author

codeyq commented Apr 27, 2020

@yue9944882 Thanks! Yeah two fields need to be fixed manually, V1ObjectMeta and V1ListMeta

  • V1ObjectMeta
    This is easy to fix because V1Crd has a ref in metadata that points to "$ref": "#/definitions/v1.ObjectMeta"
  • V1ListMeta
    This seems not straightforward cuz it doesn't have any reference to the V1ListMeta. As a result, a CRD specific V1CrdListMetadata was derived from a customized metadata field.

Do you know why a new metadata field was generated for V1CrdList but not using the existing V1ListMeta?

@erikgb
Copy link

erikgb commented Jun 10, 2020

Any updates on this issue? It seems like V1ObjectMeta is fixed, but V1ListMeta is still not applied to the metadata field of the generated CRD List type.

We would like to help fixing this, at is it currently blocking our progress. But it will require some guidance to where/how to apply a fix.

@aespinosa
Copy link

* `V1ListMeta`
  This seems not straightforward cuz it doesn't have any reference to the `V1ListMeta`. As a result, a CRD specific `V1CrdListMetadata` was derived from a customized metadata field.

Do you know why a new metadata field was generated for V1CrdList but not using the existing V1ListMeta?

For my workaround I had to edit the swagger.json by hand to add "$ref": "#/definitions/v1.ListMeta". this seems to run deeper with the swagger spec coming from the api-server as all CRDs do not have the reference.

@aespinosa
Copy link

Any updates on this issue? It seems like V1ObjectMeta is fixed, but V1ListMeta is still not applied to the metadata field of the generated CRD List type.

We would like to help fixing this, at is it currently blocking our progress. But it will require some guidance to where/how to apply a fix.

@erikgb Can you try kubernetes-client/gen#161 ? It contains a fix in pre-processing for V1ObjectMeta and V1ListMeta

@erikgb
Copy link

erikgb commented Jun 15, 2020

@erikgb Can you try kubernetes-client/gen#161 ? It contains a fix in pre-processing for V1ObjectMeta and V1ListMeta

@aespinosa Sorry for the delay. It seems like the pull request is already merged! I have been using the image create from https://github.com/yue9944882/java-model-gen, so to test your pull request would require some additional work/time that I did not have until now.

@yue9944882 Any chance for you to update your excellent java-model-gen image with the changes submitted by @aespinosa?

@yue9944882
Copy link
Member

@erikgb i opened #981 to switch the model-generation guides to the scripts by @aespinosa , any comments are welcome. thanks

@erikgb
Copy link

erikgb commented Jun 18, 2020

@erikgb i opened #981 to switch the model-generation guides to the scripts by @aespinosa , any comments are welcome. thanks

@yue9944882 It seems like a good idea to drop the container image approach for generating code for custom resources. I will try to "test" your proposed update to the docs and report back any findings.

@erikgb
Copy link

erikgb commented Jun 18, 2020

@yue9944882 I have reviewed #981, and added a couple of minor comments.

Sadly I was unable to verify the new approach with my current setup: I want to run this class generation as part of a GitLab CI/CD pipeline configured with Docker runners. Since the invoked scripts are invoking docker, I need to review my Docker-IN-Docker setup. We also have to go through a corporate Docker Registry proxy, to pull the container images, which tends to cause some pain - because we have to rewrite the Docker image URLs. I will have a closer look over the weekend....

@aespinosa
Copy link

i think @yue9944882 's workflow is meant have code generated and checked-in by hand.

For integrating with CI pipelines, you might want to tap directly to generate-sources phase. just copy off the openapi-generator-maven-plugin's settings in https://github.com/kubernetes-client/gen/blob/master/openapi/java.xml in your own pom.xml

@yue9944882
Copy link
Member

UPDATE: the CRD models now can also be written manually by properly implementing some interfaces: #1020

@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 Sep 27, 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 Oct 27, 2020
@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-testing, kubernetes/test-infra and/or fejta.
/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-testing, kubernetes/test-infra and/or fejta.
/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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

7 participants