RawCustomResourceOperationsImpl#delete(String) invalid JavaDoc and variable names

[manusa]

Description

The method public Map<String, Object> delete(String namespace) has a wrong description ar at least a partial description.

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

Lines 421 to 429 in e5e48ca

	/**
	* Delete all custom resources in a specific namespace
	*
	* @param namespace desired namespace
	* @return deleted objects as HashMap
	*/
	public Map<String, Object> delete(String namespace) {
	return makeCall(fetchUrl(namespace, null, null), null, HttpCallMethod.DELETE);
	}

If this method is invoked for a Cluster scoped CR, it will delete a CR with that name.

Not sure if the rest of the JavaDoc is actually correct for Namespaced scoped CRs.

[rohanKanojia]

I tested this and seems like it's actually for bulk deletion. If I have these resources in my kubernetes cluster:

kubernetes-resource-yamls : $ kubectl get dummies
NAME           AGE
first-dummy    2m54s
second-dummy   2m48s

If I run this code, all dummies in default namespace get deleted:

boolean result = client.customResource(context).delete("default", new DeleteOptionsBuilder()
                    .withPropagationPolicy(DeletionPropagation.BACKGROUND.toString())
                    .build());

I think for ClusterScope resources it doesn't even matter if user provides a namespace or not. Since while building the API endpoint for CustomResource we check the scope of CustomResource and skip adding namespace to it:

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

Lines 738 to 740 in 5df8dae

	if(customResourceDefinition.getScope().equals("Namespaced") && namespace != null) {
	urlBuilder.append("namespaces/").append(namespace).append("/");
	}

Right now for deleting a Cluster scoped custom resource, this seems to work:

CustomResourceDefinitionContext context = new CustomResourceDefinitionContext.Builder()
                    .withGroup("demos.fabric8.io")
                    .withScope("Cluster")
                    .withPlural("satellites")
                    .withVersion("v1alpha1")
                    .build();
boolean result = client.customResource(context).delete(null, "callisto");

Even this is working(which is not good):

boolean result = client.customResource(context).delete("idontexist", "callisto");

[manusa]

The problem is that for cluster scoped resources, using the following method:

boolean result = client.customResource(context).delete(null, "callisto");

doesn't make sense because the resource is cluster scoped.

And using the single argument method

boolean result = client.customResource(context).delete("callisto");

makes sense in terms of UX, but not in terms of JavaDoc or variable naming. Since the argument is named namespace instead of name or nameOrNamespace. On top of this, JavaDoc described the behavior for Namespace scoped Custom Resources, but not for Cluster scoped resources.

[manusa]

As agreed during our call.

In order to start providing a consistent API with the rest of the Client Operation implementations, we'll start by
implementing the Deletable interface on top of adding the withName(String name) and withNamespace(String namespace)
methods.

Besides, the current methods:

• public Map<String, Object> delete(String nameOrNamespace) {
• public Map<String, Object> delete(String namespace, boolean cascading) throws IOException {
• public Map<String, Object> delete(String namespace, DeleteOptions deleteOptions) throws IOException {

Will be reimplemented to consider both situations Cluster and Namespaced scoped CustomResources in a similar fashion as:

  public Map<String, Object> delete(String nameOrNamespace) {
    if (customResourceDefinition.getScope().equals("Namespaced") && namespace != null) {
      return makeCall(fetchUrl(nameOrNamespace, null, null), null, HttpCallMethod.DELETE);
    }
    return makeCall(fetchUrl(null, nameOrNamespace, null), null, HttpCallMethod.DELETE);
  }

and JavaDocs will be updated accordingly to describe the possible outcomes of the method invocation depending on the
scope of the Custom Resource Definition.