From ac7e0597f380c4ffe592a4763f4d7752f892fde0 Mon Sep 17 00:00:00 2001 From: HP Date: Wed, 25 Sep 2024 13:22:27 +0500 Subject: [PATCH 1/4] - Added configNamespace in Kubernetes settings object, so that users can create different settings based on config path - Added "Settings" object to KubernetesApiServiceDiscovery constructor so that users are able to provide their different settings for discovery --- .../KubernetesApiServiceDiscovery.scala | 7 ++- .../pekko/discovery/kubernetes/Settings.scala | 55 +++++++++++++------ 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscovery.scala b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscovery.scala index dd9f9631..5f723c80 100644 --- a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscovery.scala +++ b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscovery.scala @@ -96,13 +96,16 @@ object KubernetesApiServiceDiscovery { * An alternative implementation that uses the Kubernetes API. The main advantage of this method is that it allows * you to define readiness/health checks that don't affect the bootstrap mechanism. */ -class KubernetesApiServiceDiscovery(implicit system: ActorSystem) extends ServiceDiscovery { +class KubernetesApiServiceDiscovery(providedSettings: Option[Settings] = None)( + implicit system: ActorSystem) extends ServiceDiscovery { import system.dispatcher private val http = Http() - private val settings = Settings(system) + def this(system: ActorSystem) = this(None)(system) + + private val settings = providedSettings.getOrElse(Settings(system)) private val log = Logging(system, getClass)(LogSource.fromClass) diff --git a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala index 55d6af21..0d7f45fc 100644 --- a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala +++ b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala @@ -13,14 +13,15 @@ package org.apache.pekko.discovery.kubernetes -import java.util.Optional - -import org.apache.pekko.actor._ import com.typesafe.config.Config - +import org.apache.pekko.actor._ import org.apache.pekko.util.OptionConverters._ -final class Settings(system: ExtendedActorSystem) extends Extension { +import java.util.Optional + +final class Settings(configNameSpace: Option[String], system: ExtendedActorSystem) extends Extension { + + def this(system: ExtendedActorSystem) = this(None, system) /** * Copied from PekkoManagementSettings, which we don't depend on. @@ -35,39 +36,61 @@ final class Settings(system: ExtendedActorSystem) extends Extension { if (hasDefined(key)) Some(config.getString(key)) else None } - private val kubernetesApi = system.settings.config.getConfig("pekko.discovery.kubernetes-api") + private val customSettings = configNameSpace.map(system.settings.config.getConfig) + + private val kubernetesApi = + system.settings.config.getConfig("pekko.discovery.kubernetes-api") + + private def getString(path: String) = { + customSettings match { + case Some(customConfig) if customConfig.hasDefined(path) => + customConfig.getString(path) + case _ => kubernetesApi.getString(path) + } + } + + private def getBoolean(path: String) = { + customSettings match { + case Some(customConfig) if customConfig.hasDefined(path) => + customConfig.getBoolean(path) + case _ => kubernetesApi.getBoolean(path) + } + } + + private def getOptValue(config: String) = + customSettings.fold(kubernetesApi.optDefinedValue(config))(_.optDefinedValue(config)) val apiCaPath: String = - kubernetesApi.getString("api-ca-path") + getString("api-ca-path") val apiTokenPath: String = - kubernetesApi.getString("api-token-path") + getString("api-token-path") val apiServiceHostEnvName: String = - kubernetesApi.getString("api-service-host-env-name") + getString("api-service-host-env-name") val apiServicePortEnvName: String = - kubernetesApi.getString("api-service-port-env-name") + getString("api-service-port-env-name") val podNamespacePath: String = - kubernetesApi.getString("pod-namespace-path") + getString("pod-namespace-path") /** Scala API */ val podNamespace: Option[String] = - kubernetesApi.optDefinedValue("pod-namespace") + getOptValue("pod-namespace") /** Java API */ def getPodNamespace: Optional[String] = podNamespace.toJava val podDomain: String = - kubernetesApi.getString("pod-domain") + getString("pod-domain") def podLabelSelector(name: String): String = - kubernetesApi.getString("pod-label-selector").format(name) + getString("pod-label-selector").format(name) - lazy val rawIp: Boolean = kubernetesApi.getBoolean("use-raw-ip") + lazy val rawIp: Boolean = getBoolean("use-raw-ip") - val containerName: Option[String] = Some(kubernetesApi.getString("container-name")).filter(_.nonEmpty) + val containerName: Option[String] = Some(getString("container-name")).filter(_.nonEmpty) override def toString = s"Settings($apiCaPath, $apiTokenPath, $apiServiceHostEnvName, $apiServicePortEnvName, " + From 81b275344163decd1b5410ea16db9c6594da7a32 Mon Sep 17 00:00:00 2001 From: HP Date: Wed, 25 Sep 2024 15:36:43 +0500 Subject: [PATCH 2/4] - Added defaultConfig as fallback to user provided config --- .../pekko/discovery/kubernetes/Settings.scala | 53 +++++++------------ 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala index 0d7f45fc..9e574c21 100644 --- a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala +++ b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala @@ -13,13 +13,14 @@ package org.apache.pekko.discovery.kubernetes -import com.typesafe.config.Config +import java.util.Optional + import org.apache.pekko.actor._ -import org.apache.pekko.util.OptionConverters._ +import com.typesafe.config.Config -import java.util.Optional +import org.apache.pekko.util.OptionConverters._ -final class Settings(configNameSpace: Option[String], system: ExtendedActorSystem) extends Extension { +final class Settings(configNamespace: Option[String], system: ExtendedActorSystem) extends Extension { def this(system: ExtendedActorSystem) = this(None, system) @@ -36,61 +37,45 @@ final class Settings(configNameSpace: Option[String], system: ExtendedActorSyste if (hasDefined(key)) Some(config.getString(key)) else None } - private val customSettings = configNameSpace.map(system.settings.config.getConfig) + private val defaultConfig = system.settings.config.getConfig("pekko.discovery.kubernetes-api") private val kubernetesApi = - system.settings.config.getConfig("pekko.discovery.kubernetes-api") - - private def getString(path: String) = { - customSettings match { - case Some(customConfig) if customConfig.hasDefined(path) => - customConfig.getString(path) - case _ => kubernetesApi.getString(path) - } - } - - private def getBoolean(path: String) = { - customSettings match { - case Some(customConfig) if customConfig.hasDefined(path) => - customConfig.getBoolean(path) - case _ => kubernetesApi.getBoolean(path) + configNamespace match { + case Some(namespace) => system.settings.config.getConfig(namespace).withFallback(defaultConfig) + case _ => defaultConfig } - } - - private def getOptValue(config: String) = - customSettings.fold(kubernetesApi.optDefinedValue(config))(_.optDefinedValue(config)) val apiCaPath: String = - getString("api-ca-path") + kubernetesApi.getString("api-ca-path") val apiTokenPath: String = - getString("api-token-path") + kubernetesApi.getString("api-token-path") val apiServiceHostEnvName: String = - getString("api-service-host-env-name") + kubernetesApi.getString("api-service-host-env-name") val apiServicePortEnvName: String = - getString("api-service-port-env-name") + kubernetesApi.getString("api-service-port-env-name") val podNamespacePath: String = - getString("pod-namespace-path") + kubernetesApi.getString("pod-namespace-path") /** Scala API */ val podNamespace: Option[String] = - getOptValue("pod-namespace") + kubernetesApi.optDefinedValue("pod-namespace") /** Java API */ def getPodNamespace: Optional[String] = podNamespace.toJava val podDomain: String = - getString("pod-domain") + kubernetesApi.getString("pod-domain") def podLabelSelector(name: String): String = - getString("pod-label-selector").format(name) + kubernetesApi.getString("pod-label-selector").format(name) - lazy val rawIp: Boolean = getBoolean("use-raw-ip") + lazy val rawIp: Boolean = kubernetesApi.getBoolean("use-raw-ip") - val containerName: Option[String] = Some(getString("container-name")).filter(_.nonEmpty) + val containerName: Option[String] = Some(kubernetesApi.getString("container-name")).filter(_.nonEmpty) override def toString = s"Settings($apiCaPath, $apiTokenPath, $apiServiceHostEnvName, $apiServicePortEnvName, " + From 54afcf50b57106506b8bb7c31d84a85960c2ec55 Mon Sep 17 00:00:00 2001 From: HP Date: Wed, 25 Sep 2024 16:06:52 +0500 Subject: [PATCH 3/4] - refactored code --- .../kubernetes/KubernetesApiServiceDiscovery.scala | 6 ++---- .../kubernetes/KubernetesApiServiceDiscoverySpec.scala | 3 ++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscovery.scala b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscovery.scala index 5f723c80..279a28a5 100644 --- a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscovery.scala +++ b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscovery.scala @@ -96,16 +96,14 @@ object KubernetesApiServiceDiscovery { * An alternative implementation that uses the Kubernetes API. The main advantage of this method is that it allows * you to define readiness/health checks that don't affect the bootstrap mechanism. */ -class KubernetesApiServiceDiscovery(providedSettings: Option[Settings] = None)( +class KubernetesApiServiceDiscovery(settings: Settings)( implicit system: ActorSystem) extends ServiceDiscovery { import system.dispatcher private val http = Http() - def this(system: ActorSystem) = this(None)(system) - - private val settings = providedSettings.getOrElse(Settings(system)) + def this()(implicit system: ActorSystem) = this(Settings(system)) private val log = Logging(system, getClass)(LogSource.fromClass) diff --git a/discovery-kubernetes-api/src/test/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscoverySpec.scala b/discovery-kubernetes-api/src/test/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscoverySpec.scala index 712ac96e..f40d9001 100644 --- a/discovery-kubernetes-api/src/test/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscoverySpec.scala +++ b/discovery-kubernetes-api/src/test/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscoverySpec.scala @@ -203,8 +203,9 @@ class KubernetesApiServiceDiscoverySpec extends AnyWordSpec with Matchers { "The discovery loading mechanism" should { "allow loading kubernetes-api discovery even if it is not the default" in { - val system = ActorSystem() + implicit val system = ActorSystem() // #kubernetes-api-discovery + val d = new KubernetesApiServiceDiscovery() val discovery = Discovery(system).loadServiceDiscovery("kubernetes-api") // #kubernetes-api-discovery discovery shouldBe a[KubernetesApiServiceDiscovery] From d3c83f0046cfb7aceed69ab1493a34c91aff9f5a Mon Sep 17 00:00:00 2001 From: HP Date: Wed, 25 Sep 2024 16:15:23 +0500 Subject: [PATCH 4/4] - refactored code --- .../apache/pekko/discovery/kubernetes/Settings.scala | 12 ++---------- .../KubernetesApiServiceDiscoverySpec.scala | 3 +-- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala index 9e574c21..afbb7394 100644 --- a/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala +++ b/discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/Settings.scala @@ -20,9 +20,9 @@ import com.typesafe.config.Config import org.apache.pekko.util.OptionConverters._ -final class Settings(configNamespace: Option[String], system: ExtendedActorSystem) extends Extension { +final class Settings(kubernetesApi: Config) extends Extension { - def this(system: ExtendedActorSystem) = this(None, system) + def this(system: ExtendedActorSystem) = this(system.settings.config.getConfig("pekko.discovery.kubernetes-api")) /** * Copied from PekkoManagementSettings, which we don't depend on. @@ -37,14 +37,6 @@ final class Settings(configNamespace: Option[String], system: ExtendedActorSyste if (hasDefined(key)) Some(config.getString(key)) else None } - private val defaultConfig = system.settings.config.getConfig("pekko.discovery.kubernetes-api") - - private val kubernetesApi = - configNamespace match { - case Some(namespace) => system.settings.config.getConfig(namespace).withFallback(defaultConfig) - case _ => defaultConfig - } - val apiCaPath: String = kubernetesApi.getString("api-ca-path") diff --git a/discovery-kubernetes-api/src/test/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscoverySpec.scala b/discovery-kubernetes-api/src/test/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscoverySpec.scala index f40d9001..712ac96e 100644 --- a/discovery-kubernetes-api/src/test/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscoverySpec.scala +++ b/discovery-kubernetes-api/src/test/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscoverySpec.scala @@ -203,9 +203,8 @@ class KubernetesApiServiceDiscoverySpec extends AnyWordSpec with Matchers { "The discovery loading mechanism" should { "allow loading kubernetes-api discovery even if it is not the default" in { - implicit val system = ActorSystem() + val system = ActorSystem() // #kubernetes-api-discovery - val d = new KubernetesApiServiceDiscovery() val discovery = Discovery(system).loadServiceDiscovery("kubernetes-api") // #kubernetes-api-discovery discovery shouldBe a[KubernetesApiServiceDiscovery]