NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl extends ServerSideApplicable

[pan3793]

Description

CreateOrReplaceable#createOrReplace is deprecated, and the suggestion is

@deprecated please user {@link ServerSideApplicable#serverSideApply()} or attempt a create and edit/patch operation.

This change allows the following usage easy to migrate

kubernetesClient.resourceList(resources).createOrReplace()

kubernetesClient.resourceList(resources).forceConflicts().serverSideApply()

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change
• Chore (non-breaking change which doesn't affect codebase;
  test, version modification, documentation, etc.)

Checklist

• Code contributed by me aligns with current project license: Apache 2.0
• I Added CHANGELOG entry regarding this change
• I have implemented unit tests to cover my changes
• I have added/updated the javadocs and other documentation accordingly
• No new bugs, code smells, etc. in SonarCloud report
• I tested my code in Kubernetes
• I tested my code in OpenShift

[pan3793]

cc @shawkins, is the following migration right?

resources.createOrReplace()

resources.forceConflicts().serverSideApply()

[rohanKanojia]

@pan3793 : Is this PR related to #5071 ?

[pan3793]

@pan3793 : Is this PR related to #5071 ?

Just a little. I found some issues(#5071) in createOrReplace, but since it was deprecated, I move to migrate instead of fix. And found this issue(independent) on investigating migration to the suggested API

[shawkins]

cc @shawkins, is the following migration right?

With the caveat that the user may choose not to use forcing if they want to know when there are conflicting changes.

Also unlike createOrReplace if the resourceVersion is set on the resource and a replace is attempted, it will be optimistically locked.

This change allows the following usage easy to migrate

I was mixed on how many of these composite operations to expose. The user also has access to resources(), so one can do whatever with resources().forEach(...) - including error handling, which is non-existent in performOperation. So here the alternative to adding these methods is to use

resourceList(list).resources().forEach(r -> r.forceConflicts().serverSideApply())  

Tests are needed for this pr - you'll notice when this is executed that the fieldManager and forceConflicts are lost. This is because a new context is created for each resource and we're specifically propagating what is applicable to the write operations in

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationContext.java

Line 506 in 7104c6f

public <C extends Client> C clientInWriteContext(Class<C> clazz) {

[pan3793]

This is because a new context is created for each resource and we're specifically propagating what is applicable to the write operations

@shawkins thanks, but where can I find the definition of "what is applicable to the write operations"?

[shawkins]

thanks, but where can I find the definition of "what is applicable to the write operations"?

In this case you are making forceConflicts and fieldManager applicable. They would need to be added to the context

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationContext.java

Line 511 in 7104c6f

.withFieldValidation(this.fieldValidation);

[pan3793]

I was mixed on how many of these composite operations to expose.

I would say it's a little hard for me to figure out how to handle the deprecated method invocations of this library, many suggestions in deprecated's description link to a totally different class, w/o examples. Not like the following common case

class Foo {

  @deprecated use `barV2` instead
  void bar() { ... }

  void barV2() { ... }
}

So, I think implementing this interface makes user easy to migrate.

[pan3793]

Tests are needed for this pr - you'll notice when this is executed that the fieldManager and forceConflicts are lost.

Thanks for the tips, UT ServerSideApplyTest is added.

[pan3793]

@shawkins could you please take another look?

[shawkins]

Was there a reason to copy the test logic, rather than calling the methods that were already on PatchTest?

[pan3793]

@shawkins moved test case into PatchTest

[pan3793]

Test failure seems irrelevant.

[shawkins]

LGTM

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

[pan3793]

@shawkins @manusa is there anything I can do before it gets in?

[manusa]

@shawkins @manusa is there anything I can do before it gets in?

Looks good, thx. Releasing today.