Support generating java classes from CRD that implement the Editable interface. about kubernetes-client HOT 13 CLOSED

There is a PR here: #5879

from kubernetes-client.

Thanks a lot for your input @matteriben , before moving on with this one I need to ask a few points:

• what's the real world use case?
• have you evaluated alternatives? It should be possible to do new CronTabBuilder(oldCronTabObj) providing a very similar API to the one enabled by Editable
• the integration with sundrio features has always been a little tricky(maintenance-wise) and I'm not excited about expanding on that front

from kubernetes-client.

what's the real world use case?

@andreaTP this is in part to provide consistency with what is being done in the core client. It would be great if at some point we were using a single set of java generation logic :(

have you evaluated alternatives? It should be possible to do new CronTabBuilder(oldCronTabObj) providing a very similar API to the one enabled by Editable

The closest would probably be to use the lombok Builder annotation with toBuilder=true. The downside there is that lombok builder logic doesn't provide nested fluent methods.

from kubernetes-client.

this is in part to provide consistency with what is being done in the core client.

Thanks for the context 🙏

It would be great if at some point we were using a single set of java generation logic

Agreed

What do you think about making it a breaking change:

• we postpone this to 7.X
• we change the current implementation of ExtraAnnotations(as opposed to an extra configuration in the matrix) to better match the core client

from kubernetes-client.

What do you think about making it a breaking change:

You mean this change, or trying to align the generation logic? I think we can only do the former.

we change the current implementation of ExtraAnnotations(as opposed to an extra configuration in the matrix) to better match the core client

At this point this cannot be annotation driven alone, but aligning sounds good.

from kubernetes-client.

You mean this change, or trying to align the generation logic? I think we can only do the former.

Correct, this change, but we re-use the existing option to avoid increasing the matrix of possibilities to be maintained.

At this point this cannot be annotation driven alone, but aligning sounds good.

Would be nice to have a diff(e.g. comparing with the Approval Tests) of the target desired outcome.

from kubernetes-client.

@andreaTP Could you please help me understand in what context or scenario this could be considered a breaking change? I understand you are considering the possibility of enabling this whenever extraAnnotations is enabled, but even in that case I'm not understanding what existing use cases would break.

from kubernetes-client.

My current thinking is that adding the Editable interface is a change "large enough" that it would make sense to roll out carefully even if it doesn't break the user code (extraAnnotations has been stable for quite some time now).
We are planning to switch main to version 7.X shortly, so I would take the opportunity to stay on the safe side.

I'm open to the challenge though, and, in the presence of evidence that this is an extremely safe change I'm happy to re-evaluate 🙂 .
A good proof that everything works would be to apply the changes to this project:
https://github.com/skodjob/opendatahub-crds
and verify that https://github.com/skodjob/odh-e2e compiles without code changes.

from kubernetes-client.

version 7.X shortly

Do you have an estimate or target date for this?

A good proof that everything works would be to apply the changes to this project: https://github.com/skodjob/opendatahub-crds and verify that https://github.com/skodjob/odh-e2e compiles without code changes.

I tried this locally, I got it to build on the main branches.

• I ended up blowing away my local maven repository because I was initially not successful even without this change.
• I had to build the fabric8 client with java-v11 (not 1.8) to get kubernetes-client-bom-with-deps installed locally.
• I updated opendatahub-crds to use version 6.12-SNAPSHOT, and enabled the implementsEditable feature.
• I also had to build test-metadata-generator.
• I also commented out remote repositories sections in odh-e2e/pom.xml

In the end I was able to build it.

Though I made no code changes to get this to build, this change could potentially break an existing build if this feature is enabled (or extraAnnotations if we tie Editable to it), and the project doesn't already have the proper dependency for Editable (kubernetes-model-common), both at the time of annotation processing and compiling. I'm not sure how much of an issue that could be in general, but for this test project it was not an issue.

from kubernetes-client.

Thanks for the analysis!
In the context of this work, we should update the documentation regarding adding the kubernetes-model-common dependency.
Happy to move forward on 7.X though!

Do you have an estimate or target date for this?

Not yet.

from kubernetes-client.

Do you have an estimate or target date for this?

Not yet.

Will it be possible to get an early snapshot build of 7.x? The reason I'm asking is because we would like to take advantage of this change as soon as possible.

from kubernetes-client.

Thanks for the analysis! In the context of this work, we should update the documentation regarding adding the kubernetes-model-common dependency. Happy to move forward on 7.X though!

I tried rebuilding to verify the dependency requirements. Here's what I see:

Without extraAnnotations:

package io.fabric8.kubernetes.api.model does not exist
package io.fabric8.generator.annotation does not exist
package io.fabric8.kubernetes.client.utils does not exist

Additionally with extraAnnotations:

package lombok does not exist
package io.sundr.builder.annotations does not exist

All submodules under kubernetes-model-generator except kubernetes-model include a dependency on kubernetes-model-common.

It seems to me adding Editable which does depend on kubernetes-model-common, should not cause any breaking change since it seems the preexisting dependency relationship would have already provided this dependency.

I'll update the documentation to list these required dependencies for java-generator:

        <dependency>
            <groupId>io.fabric8</groupId>
            <artifactId>openshift-client</artifactId>
        </dependency>
        <dependency>
            <groupId>io.fabric8</groupId>
            <artifactId>kubernetes-model</artifactId>
        </dependency>
        <dependency>
            <groupId>io.fabric8</groupId>
            <artifactId>generator-annotations</artifactId>
        </dependency>
<!--        extraAnnotations requires these additional dependencies -->
        <dependency>
            <groupId>io.sundr</groupId>
            <artifactId>builder-annotations</artifactId>
            <scope>provided</scope>
        </dependency>
        <dependency>
            <groupId>org.projectlombok</groupId>
            <artifactId>lombok</artifactId>
            <scope>provided</scope>
        </dependency>

from kubernetes-client.

I updated the documentation here: #5918

from kubernetes-client.