Support serverSideApply in KubernetesServer mock

[agateaux]

Is your enhancement related to a problem? Please describe

Now that createOrReplace will be deprecated, it will be frustrating and obviously not smooth to migrate to serverSideApply, as CREATE/POST operation is not yet managed.

Describe the solution you'd like

Support CREATE operation in serverSideApply. As per official Kubernetes documentation here https://kubernetes.io/docs/reference/using-api/server-side-apply/#field-management and https://kubernetes.io/docs/reference/using-api/server-side-apply/#rbac-and-permissions, it is possible to create resources thanks to server side apply.

Describe alternatives you've considered

Changing all our codebase to manage 4xx exceptions when using serverSideApply on non-existing resources.

Additional context

No response

[shawkins]

Now that createOrReplace will be deprecated, it will be frustrating and obviously not smooth to migrate to serverSideApply, as CREATE/POST operation is not yet managed.

I don't follow what is being requested here. If you use resource.serverSideApply() it will be created if the resource does not exist.

We did discuss in upstream kubernetes some of the situations where serverSideApply is problematic and one issue to come out of that was https://github.com/kubernetes/kubernetes/issues/118775 - which may imply in the future kubectl and kubernetes support for a set ,or createOrReplace like, operation at which point we'd reintroduce something with the same semantics to the fabric8 kuberentes client.

[agateaux]

Here is an exception raised by Fabric8 KubernetesClient while trying to create a CRD not already existing, using serverSideApply (kubernetesClient.resource(crd).serverSideApply();): Caused by: io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: PATCH at: https://localhost:53860/apis/apiextensions.k8s.io/v1/customresourcedefinitions/apps.paasas.io?fieldManager=fabric8. Message: Not Found.

It really means there is no handling of unexisting resources.

[shawkins]

It really means there is no handling of unexisting resources.

What kubernetes server version are you hitting?

[agateaux]

It's the Fabric8 KubernetesServer, embedded during JUnit tests.

[shawkins]

It's the Fabric8 KubernetesServer, embedded during JUnit tests.

The mock server does not support server side apply in crud mode. It requies the use of expectations, or an actual api server - minikube or https://github.com/java-operator-sdk/jenvtest

[agateaux]

Ok so we could rename my request to support serverSideApply in the mock server ;)

[shawkins]

Ok so we could rename my request to support serverSideApply in the mock server ;)

Unfortunately that is unlikely to happen. There's quite a lot of logic that support server side apply and porting it to java for just the mock server seem like too large of an effort. It also depends upon understanding mergeKeys and other metadata, which we do not have on our object model, but is available in go.

The goal is to soon deprecate the mock server crud mode and encourage users to use a real api-server instead. If you using quarkus there's also https://quarkus.io/guides/kubernetes-dev-services

[manusa]

In order to ease transitioning, we might want to include at least the support for the initial create operation of server side apply (SSA) in the Mock Server which might cover a bunch of user scenarios (for those who are migrating from createOrReplace).

[shawkins]

In order to ease transitioning, we might want to include at least the support for the initial create operation of server side apply

At least for usage with things like operator tests that typically won't be enough.

for those who are migrating from createOrReplace

There's still some documentation I want to add around this based upon the discussions that happened on the kubernetes issues. I think we need to state more strongly that the user can consider using some varitation of create then on conflict perform a patch, or update - whatever fits their exact scenario.

[manusa]

At least for usage with things like operator tests that typically won't be enough.

I know it's not enough, but will certainly cover many use cases. The point is that now we don't support any.

My point is that we should improve the mock server to warn/fail whenever an unsupported operation is performed while supporting the highest amount of operations that require a minimum hassle to implement (in this case, allow create and fail on update which IMHO is better than fail on create and update)

[manusa]

There's still some documentation I want to add around this based upon the discussions that happened on the kubernetes issues. I think we need to state more strongly that the user can consider using some varitation of create then on conflict perform a patch, or update - whatever fits their exact scenario.

+1 100%

[AbdelrhmanHamouda]

I would love to see the mockServer able to handle this.

One of the strong reasons for my team to chose fabric8 k8s client was for its ability to be tested using the provided mock server. It is much more convenient than a "live" k8s server.

Please don't neglect the immense value that mockServer provides.

[shawkins]

Please don't neglect the immense value that mockServer provides.

See this comment https://github.com/fabric8io/kubernetes-client/issues/5337#issuecomment-1642683157 - unfortunately the crud mock support will always be incomplete. It leads users down a path of what appears to be fully java based testing only to have significant gaps wrt an actual api server. Please evaluate devservices and jenvtest for something that is a little more light-weight than a full kubernetes that also has some java test integration.

[shawkins]

@manusa @rohanKanojia Marc is proposing above to have the crud mock logic support the initial create with server side apply. The other things we can consider are:

• simply changing the definitions of createOrReplace and replace to be inline with what could be expected - that is change replace to be update and strip the retry and non-locking behavior from createOrReplace. Yes this would be a breaking change and require additional documentation, but it would work most of the time with existing code.
• introduce a generalized createOr(Function<Resource, T> conflictAction) method - such that you could do createOr(Resource::update), createOr(Resource::patch). This could also be expressed in a more dsl-like way with resource.onConflict(Resource::update).create() or something that would be cross-cutting resource.createIfNotFound().update()

[manusa]

• introduce a generalized createOr(Function<Resource, T> conflictAction) method - such that you could do createOr(Resource::update), createOr(Resource::patch). This could also be expressed in a more dsl-like way with resource.onConflict(Resource::update).create() or something that would be cross-cutting resource.createIfNotFound().update()

I find this interesting, but I'm still in favor of supporting more PATCH types in the Mock Server.

[shawkins]

I find this interesting, but I'm still in favor of supporting more PATCH types in the Mock Server.

Adding additional, more straight-forward, dsl for this use-case does not mean we won't pursue other patch types - but realistically we are not going to provide strategic merge nor server side apply support.

[stale[bot]]

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

[horis233]

@manusa @rohanKanojia Marc is proposing above to have the crud mock logic support the initial create with server side apply. The other things we can consider are:

• simply changing the definitions of createOrReplace and replace to be inline with what could be expected - that is change replace to be update and strip the retry and non-locking behavior from createOrReplace. Yes this would be a breaking change and require additional documentation, but it would work most of the time with existing code.
• introduce a generalized createOr(Function<Resource, T> conflictAction) method - such that you could do createOr(Resource::update), createOr(Resource::patch). This could also be expressed in a more dsl-like way with resource.onConflict(Resource::update).create() or something that would be cross-cutting resource.createIfNotFound().update()

@shawkins Could I ask why we choose "resource.unlock().createOr(NonDeletingOperation::update)" instead of a simplified way like create a function called createOrUpdate() "resource.createOrUpdate()".

In our user case, we are using kubernetes-client in kotlin, so it is harder for us to use method reference from Java code.

[shawkins]

Could I ask why we choose "resource.unlock().createOr(NonDeletingOperation::update)" instead of a simplified way like create a function called createOrUpdate() "resource.createOrUpdate()".

This method signature provides the most flexibility for someone to handle the conflict in whatever they want.

In our user case, we are using kubernetes-client in kotlin, so it is harder for us to use method reference from Java code.

You should still be able to just use a simple lamda resource.unlock().createOr(r -> r.update()). If you want to propose, or contribute, adding a convenience method that could be considered as well.

[stale[bot]]

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!