design of resource and resourceList methods

[shawkins]

After #3364 the implementation logic for these methods aligns well with everything else. However they are still not aligned completely with the rest of the api.

There are several possibilities:

1. Deprecate the methods fully. Advise users to use resources instead:

client.resource(resource).createOrReplace();

Would be

client.resources(resource.getClass()).inNamespace(resource.getMetadata().getNamespace()).createOrReplace(resource);

The interface for resourceList would be minimized to just a handful of operations, as any operation could be implemented by

resources.forEach(r -> client.resources(r.getClass())....);

2. Have resource return a Namespaceable / Resource - this will be a breaking change. Similar to the above you would minimize the resourceList methods. That would make the namespace behavior more concise than above.

[shawkins]

2 - is not a simple change. BaseOperation/HasMetadataOperation can't implement two Namespaceable interfaces.

The simplest thing we could do is change resource to return Resource, so

client.resource(resource)

would be implemented by something like:

HasMetadataOperation resources = resources(item.getClass(), null);
BaseOperation op = resources.withItem(item);
String namespace = KubernetesResourceUtil.getNamespace(item);
if (namespace != null) {
   op = resources.inNamespace(namespace);
}
return op;

However that means you would loose the ability to later call inNamespace as that is not provided on the Resource interface. The only drawback is that means you have to directly manipulate the resource object (or convert the string to object) to change the namespace.

[stale]

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!

[shawkins]

Also relates to #3860 - if you use the KubernetesClient.load methods they are different that the Loadable.load methods - they will default to inferring the namespace from the item, and allow for the namespace to be changed. I'd like to make everything uniform if possible for 6.0

[shawkins]

@manusa @rohanKanojia #3858 proposes to replace convert the resource method into one that returns NamespaceableResource - a Resource that also exposes an inNamespace call. The implementation is roughly doing what is described in #3407 (comment)

Other than the interface name changes, the biggest difference in support is deletingExisting, which is discussed in the pr comments.

For resourceList - there are two/three list apis - load/resourceList(String) which expose parameterizable, the other resourceList methods, and lists().load - which instead exposes a RecreateFromServerGettable. It doesn't appear that lists() logic is widely used. Looking at it more I see that lists().load(...).delete() - won't do anything, and deletingExisting isn't doing anything either. Should lists() simply be deprecated?

It appears also that parameterizable is exposed to account for the case where you are using the kubernetesclient (which is actually a DefaultOpenShiftClient) and the list may contain a template. It may be better to push that concern entirely over to the openshiftclient instead.

The core of the resourceList methods is ListVisitFromServerGetDeleteRecreateWaitApplicable. An option touched on above is to deprecate anything that doesn't make sense - and not add anything else to this interface other than have it expose a List<Resource>. The user would make use of lambdas resourceList(...).getResources().forEach(r -> r.withGracePeriod(10).delete()); - or whatever sequence of options they want.

[shawkins]

It might be good to cover namespace expectations of the various methods:

KuberntesClient.resource / resourceList - use the namespace from the item by default, override both the item and the context with inNamespace

KubernetesClient.lists.load, Loadable.load - expect the item namespace to match context namespace, there is no option to change item or the context namespace after the load call.

[shawkins]

Resolved by #3858