[SPARK-37687][K8S] Removes the compile time dependency on the OkHttp http client

[navinvishy]

What changes were proposed in this pull request?

Removes the dependency on the OkHttp http client for the kubernetes client.

Why are the changes needed?

The Kubernetes client supports plugging in different http clients via the Java service provider interface. Since we have a dependency on OkHttp it is not possible to plug in other http clients.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added a unit test

Was this patch authored or co-authored using generative AI tooling?

No

[navinvishy]

The custom dispatcher uses daemon threads. Not setting this custom dispatcher implies that we will now have non-daemon threads in the http client.
cc: @Yikun if you could please take a look.

[pan3793]

The custom dispatcher uses daemon threads.

that's the exact purpose of overwriting the HttpClientFactory

[navinvishy]

Yeah I thought so. How about adding something like this?

val clientFactory = HttpClientUtils.getHttpClientFactory
val clientBuilder = new KubernetesClientBuilder().withConfig(config)
clientFactory match {
  case _: OkHttpClientFactory =>
    val dispatcher = new Dispatcher(
      ThreadUtils.newDaemonCachedThreadPool("kubernetes-dispatcher"))
    val factory = new OkHttpClientFactory() {
      override def additionalConfig(builder: OkHttpClient.Builder): Unit = {
        builder.dispatcher(dispatcher)
      }
    }
    clientBuilder.withHttpClientFactory(factory).build()
  case _ => clientBuilder.build()
}

[pan3793]

I'm not against that, after making this change, how do you configure the K8s client builder to use another HTTP client factory without touching the code?

BTW, please restore unnecessary code changes from your patch.

[navinvishy]

In the job jar, you would include a dependency on another http client, for eg.

<dependency>
    <groupId>io.fabric8</groupId>
    <artifactId>kubernetes-httpclient-jdk</artifactId>
  </dependency>

These dependency jars contain the file META-INF/services/io.fabric8.kubernetes.client.http.HttpClient$Factory containing the name of the class implementing the provider, which the K8s client builder should pick up.

The job jar could also include META-INF/services/io.fabric8.kubernetes.client.http.HttpClient$Factory with the name of the class we want to use.

I'll restore the changes - somehow running /dev/scalafmt introduced those.

[pan3793]

then how do you choose the exact one you want to use if there are multi impls under the classpath?

[navinvishy]

You can set priority in the provider configuration file:

1: <class1>
2: <class2>

The code in fabric8 appears to be honoring that priority.

[pan3793]

make sense, thanks for sharing the information

[navinvishy]

Sorry @pan3793 minor correction to what I said earlier. The priority comes from the factory itself. So if there are multiple options, we could extend the factory we want and assign priority, like this:

class MyHttpClientFactory extends JdkHttpClientFactory {
      override def priority(): Int = -1
}

and then add MyHttpClientFactory to the configuration file.

Regardless, with this change it should be possible to choose an implementation without touching code.

[pan3793]

I am not sure if this comment still valid

[pan3793]

cc @dongjoon-hyun @yaooqinn

[navinvishy]

Hi @dongjoon-hyun @yaooqinn could you please take a look?

[dongjoon-hyun]

Let me close this PR since SPARK-37687 is resolved via

• [SPARK-50493][SPARK-37687][BUILD] Migrate kubernetes-client from 6.x to 7.x #49159

[dongjoon-hyun]

Thank you for making this PR before. It's more natural to follow the upstream Kubernetes-client change.

[navinvishy]

Thank you for making this PR before. It's more natural to follow the upstream Kubernetes-client change.

Sure, yes I agree solving upstream is better.