From a8dbe4827c29f8ab18dd57dd071f3533ddbfabc7 Mon Sep 17 00:00:00 2001 From: Mattis Bratland Date: Sat, 19 Oct 2024 17:51:53 +0200 Subject: [PATCH 1/7] feat: Add support for configuring ConfigurationService via OperatorConfigHandlers in DI - Introduced OperatorConfigHandlers to enable ConfigurationService customization. - Replaced (Operator) -> Unit with OperatorPostConfigHandler to avoid storing lambdas in Koin. --- src/main/kotlin/no/fintlabs/Application.kt | 15 ++++++++--- .../no/fintlabs/OperatorConfigHandlers.kt | 8 ++++++ .../extensions/KubernetesOperatorExtension.kt | 26 ++++++++++++++----- 3 files changed, 38 insertions(+), 11 deletions(-) create mode 100644 src/main/kotlin/no/fintlabs/OperatorConfigHandlers.kt diff --git a/src/main/kotlin/no/fintlabs/Application.kt b/src/main/kotlin/no/fintlabs/Application.kt index b3fd515..dccb0be 100644 --- a/src/main/kotlin/no/fintlabs/Application.kt +++ b/src/main/kotlin/no/fintlabs/Application.kt @@ -35,14 +35,21 @@ val baseModule = module { .withKubernetesSerialization(KubernetesSerialization(get(), true)) .build() } - single<(Operator) -> Unit> { - { operator -> + single { + OperatorPostConfigHandler { operator -> getAll>().forEach { operator.register(it) } } } single { - Operator(ConfigurationService.newOverriddenConfigurationService { it.withKubernetesClient(get()) }).apply { - get<(Operator) -> Unit>().invoke(this) + OperatorConfigHandler { config -> config.withKubernetesClient(get()) } + } + single { + val config = ConfigurationService.newOverriddenConfigurationService { config -> + getAll().reversed().forEach { it.accept(config) } + } + + Operator(config).apply { + get().accept(this) } } } diff --git a/src/main/kotlin/no/fintlabs/OperatorConfigHandlers.kt b/src/main/kotlin/no/fintlabs/OperatorConfigHandlers.kt new file mode 100644 index 0000000..b4f1f8f --- /dev/null +++ b/src/main/kotlin/no/fintlabs/OperatorConfigHandlers.kt @@ -0,0 +1,8 @@ +package no.fintlabs + +import io.javaoperatorsdk.operator.Operator +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider +import java.util.function.Consumer + +fun interface OperatorConfigHandler : Consumer +fun interface OperatorPostConfigHandler : Consumer \ No newline at end of file diff --git a/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorExtension.kt b/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorExtension.kt index 3a9c306..05b76dc 100644 --- a/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorExtension.kt +++ b/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorExtension.kt @@ -15,12 +15,17 @@ import io.fabric8.kubernetes.client.utils.KubernetesSerialization import io.javaoperatorsdk.operator.Operator import io.javaoperatorsdk.operator.api.reconciler.Reconciler import io.javaoperatorsdk.operator.junit.DefaultNamespaceNameSupplier +import no.fintlabs.OperatorConfigHandler +import no.fintlabs.OperatorPostConfigHandler import org.awaitility.kotlin.atMost import org.awaitility.kotlin.await import org.awaitility.kotlin.until import org.junit.jupiter.api.extension.* import org.koin.core.component.KoinComponent import org.koin.core.component.get +import org.koin.core.context.loadKoinModules +import org.koin.core.qualifier.named +import org.koin.dsl.module import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.time.Duration @@ -66,6 +71,7 @@ private constructor(private val crdClass: List>>) cleanupKubernetes(kubernetesClient, kubernetesOperatorContext.namespace) get().stop() + kubernetesClient.close() } override fun supportsParameter(pContext: ParameterContext, eContext: ExtensionContext): Boolean = @@ -98,15 +104,21 @@ private constructor(private val crdClass: List>>) private fun prepareKoin(kubernetesClient: KubernetesClient) { getKoin().apply { - declare(kubernetesClient) - declare<(Operator) -> Unit>( - { operator -> - getAll>().forEach { - operator.register(it) { config -> - config.settingNamespace(kubernetesClient.namespace) + loadKoinModules( + module { + single { kubernetesClient } + single(named("test")) { OperatorConfigHandler { config -> config.withCloseClientOnStop(false) } } + single { + OperatorPostConfigHandler { operator -> + getAll>().forEach { + operator.register(it) { config -> + config.settingNamespace(kubernetesClient.namespace) + } + } } } - }) + } + ) } } From b66a9bdb9907a1416a38bc24ec6986b5ccdb11ac Mon Sep 17 00:00:00 2001 From: Mattis Bratland Date: Sat, 19 Oct 2024 17:53:12 +0200 Subject: [PATCH 2/7] test: Make operator and kubernetesClient accessable from KubernetesOperatorContext --- .../fintlabs/extensions/KubernetesOperatorContext.kt | 4 +++- .../fintlabs/extensions/KubernetesOperatorExtension.kt | 10 ++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorContext.kt b/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorContext.kt index db8d759..25faa43 100644 --- a/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorContext.kt +++ b/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorContext.kt @@ -2,10 +2,12 @@ package no.fintlabs.extensions import io.fabric8.kubernetes.api.model.HasMetadata import io.fabric8.kubernetes.client.KubernetesClient +import io.javaoperatorsdk.operator.Operator class KubernetesOperatorContext( val namespace: String, - private val kubernetesClient: KubernetesClient + val kubernetesClient: KubernetesClient, + val operator: Operator ) { inline fun get(name: String): T? { return get(T::class.java, name) diff --git a/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorExtension.kt b/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorExtension.kt index 05b76dc..196a5dd 100644 --- a/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorExtension.kt +++ b/src/test/integration/kotlin/no/fintlabs/extensions/KubernetesOperatorExtension.kt @@ -57,20 +57,22 @@ private constructor(private val crdClass: List>>) prepareKoin(kubernetesClient) prepareKubernetes(kubernetesClient, namespace) applyAdditionalResources(kubernetesClient, namespace) + + val operator = get() context.store() - .put(KubernetesOperatorContext::class.simpleName, KubernetesOperatorContext(namespace, kubernetesClient)) + .put(KubernetesOperatorContext::class.simpleName, KubernetesOperatorContext(namespace, kubernetesClient, operator)) - get().start() + operator.start() } override fun afterEach(context: ExtensionContext) { - val kubernetesClient = get() val kubernetesOperatorContext = context.store().get(KubernetesOperatorContext::class.simpleName) as KubernetesOperatorContext + val kubernetesClient = kubernetesOperatorContext.kubernetesClient cleanupKubernetes(kubernetesClient, kubernetesOperatorContext.namespace) - get().stop() + kubernetesOperatorContext.operator.stop() kubernetesClient.close() } From 73a5cb00803e85da70578d3572785e47fb46a60c Mon Sep 17 00:00:00 2001 From: Mattis Bratland Date: Sat, 19 Oct 2024 17:54:47 +0200 Subject: [PATCH 3/7] test: Add helper functions for waiting on Kubernetes resource deployment - Added waitUntilIsDeployed for checking FlaisApplicationCrd deployment state. - Added generic waitUntil function to wait for any resource condition using HasMetadata. --- .../kotlin/no/fintlabs/operator/Utils.kt | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/test/integration/kotlin/no/fintlabs/operator/Utils.kt b/src/test/integration/kotlin/no/fintlabs/operator/Utils.kt index d9b83fd..07e19e1 100644 --- a/src/test/integration/kotlin/no/fintlabs/operator/Utils.kt +++ b/src/test/integration/kotlin/no/fintlabs/operator/Utils.kt @@ -24,12 +24,24 @@ import java.time.Duration object Utils { inline fun KubernetesOperatorContext.createAndGetResource(app: FlaisApplicationCrd, nameSelector: (FlaisApplicationCrd) -> String = { it.metadata.name }): T? { create(app) - await atMost Duration.ofSeconds(10) until { - get(app.metadata.name)?.status?.state == FlaisApplicationState.DEPLOYED - } + waitUntilIsDeployed(app) return get(nameSelector(app)) } + fun KubernetesOperatorContext.waitUntilIsDeployed(app: FlaisApplicationCrd) { + waitUntil( + app.metadata.name, + ) { it.status?.state == FlaisApplicationState.DEPLOYED } + } + + inline fun KubernetesOperatorContext.waitUntil(resourceName: String, timeout: Duration = Duration.ofSeconds(30), crossinline condition: (T) -> Boolean) { + await atMost timeout until { + get(resourceName)?.let { condition(it) } ?: false + } + } + + + fun createTestFlaisApplication(): FlaisApplicationCrd { return FlaisApplicationCrd().apply { metadata = ObjectMeta().apply { From 5e9c37b4147a9d0ab4d5bb617bdc18e6c2d693a6 Mon Sep 17 00:00:00 2001 From: Mattis Bratland Date: Sat, 19 Oct 2024 17:55:58 +0200 Subject: [PATCH 4/7] test: Add test for recreating deployment on pod selector change - Added test to verify deployment is recreated when pod selector is modified. - Ensures deployment is properly updated and selector labels are correctly handled. --- .../no/fintlabs/operator/DeploymentDRTest.kt | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/test/integration/kotlin/no/fintlabs/operator/DeploymentDRTest.kt b/src/test/integration/kotlin/no/fintlabs/operator/DeploymentDRTest.kt index 9aed5a0..e9ee8c0 100644 --- a/src/test/integration/kotlin/no/fintlabs/operator/DeploymentDRTest.kt +++ b/src/test/integration/kotlin/no/fintlabs/operator/DeploymentDRTest.kt @@ -13,6 +13,7 @@ import no.fintlabs.operator.Utils.createAndGetResource import no.fintlabs.operator.Utils.createKoinTestExtension import no.fintlabs.operator.Utils.createKubernetesOperatorExtension import no.fintlabs.operator.Utils.createTestFlaisApplication +import no.fintlabs.operator.Utils.waitUntilIsDeployed import no.fintlabs.operator.api.LOKI_LOGGING_LABEL import no.fintlabs.operator.api.v1alpha1.* import no.fintlabs.v1alpha1.kafkauserandaclspec.Acls @@ -465,6 +466,36 @@ class DeploymentDRTest { } //endregion + //region PodSelector + @Test + fun `should recreate deployment on pod selector change selector`(context: KubernetesOperatorContext) { + val flaisApplication = createTestFlaisApplication() + + var deployment = context.createAndGetDeployment(flaisApplication) + assertNotNull(deployment) + + context.operator.stop() + context.delete(deployment) + + deployment.spec.apply { + selector.matchLabels["another"] = "another" + template.metadata.labels["another"] = "another" + } + deployment.metadata.resourceVersion = null + + context.create(deployment) + + context.operator.start() + context.waitUntilIsDeployed(flaisApplication) + deployment = context.get(deployment.metadata.name) + + assertNotNull(deployment) + assertEquals(1, deployment.spec.selector.matchLabels.size) + assert(deployment.spec.selector.matchLabels.containsKey("app")) + assertEquals(deployment.metadata.name, deployment.spec.selector.matchLabels["app"]) + } + //endregion + private fun KubernetesOperatorContext.createAndGetDeployment(app: FlaisApplicationCrd) = createAndGetResource(app) From d3ad46e6cddd44ac8e44007b4c31ee86192fed64 Mon Sep 17 00:00:00 2001 From: Mattis Bratland Date: Sat, 19 Oct 2024 17:56:45 +0200 Subject: [PATCH 5/7] feat: Recreate deployment when pod selector changes - Added logic to compare pod selectors between actual and desired deployments. - If pod selectors do not match, the deployment is deleted and recreated to apply the changes. - Ensures accurate reconciliation when pod selector updates are detected. --- .../kotlin/no/fintlabs/operator/DeploymentDR.kt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/main/kotlin/no/fintlabs/operator/DeploymentDR.kt b/src/main/kotlin/no/fintlabs/operator/DeploymentDR.kt index e3d570b..6d9146e 100644 --- a/src/main/kotlin/no/fintlabs/operator/DeploymentDR.kt +++ b/src/main/kotlin/no/fintlabs/operator/DeploymentDR.kt @@ -21,6 +21,7 @@ import org.koin.core.component.inject ) class DeploymentDR : CRUDKubernetesDependentResource(Deployment::class.java), KoinComponent { private val config: Config by inject() + private val logger = getLogger() override fun desired(primary: FlaisApplicationCrd, context: Context) = Deployment().apply { @@ -41,6 +42,19 @@ class DeploymentDR : CRUDKubernetesDependentResource().matches(actual, desired, context), desired); } + override fun handleUpdate(actual: Deployment, desired: Deployment, primary: FlaisApplicationCrd, context: Context): Deployment { + val kubernetesSerialization = context.client.kubernetesSerialization + val desiredSelector = kubernetesSerialization.convertValue(desired.spec.selector, Map::class.java) + val actualSelector = kubernetesSerialization.convertValue(actual.spec.selector, Map::class.java) + val podSelectorMatch = desiredSelector == actualSelector + + if (podSelectorMatch) return handleUpdate(actual, desired, primary, context) + + logger.info("Pod selector does not match, recreating deployment ${actual.metadata.name}") + handleDelete(primary, actual, context) + return handleCreate(desired, primary, context) + } + private fun cretePodMetadata(primary: FlaisApplicationCrd) = createObjectMeta(primary).apply { annotations["kubectl.kubernetes.io/default-container"] = primary.metadata.name labels["observability.fintlabs.no/loki"] = primary.spec.observability?.logging?.loki?.toString() ?: "true" From 3cd903f4b020ca6c343bceac3dfa2e9ff10c1a52 Mon Sep 17 00:00:00 2001 From: Mattis Bratland Date: Sat, 19 Oct 2024 18:38:11 +0200 Subject: [PATCH 6/7] test: Add utility function for updating and retrieving Kubernetes resource - Added updateAndGetResource function to update a resource, wait for deployment, and retrieve the updated resource. - Supports custom name selectors with a default based on metadata name. --- src/test/integration/kotlin/no/fintlabs/operator/Utils.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/integration/kotlin/no/fintlabs/operator/Utils.kt b/src/test/integration/kotlin/no/fintlabs/operator/Utils.kt index 07e19e1..9e4f7c4 100644 --- a/src/test/integration/kotlin/no/fintlabs/operator/Utils.kt +++ b/src/test/integration/kotlin/no/fintlabs/operator/Utils.kt @@ -28,6 +28,12 @@ object Utils { return get(nameSelector(app)) } + inline fun KubernetesOperatorContext.updateAndGetResource(app: FlaisApplicationCrd, nameSelector: (FlaisApplicationCrd) -> String = { it.metadata.name }): T? { + update(app) + waitUntilIsDeployed(app) + return get(nameSelector(app)) + } + fun KubernetesOperatorContext.waitUntilIsDeployed(app: FlaisApplicationCrd) { waitUntil( app.metadata.name, From 38b006a772f12464b1292f92346de8173a8507d0 Mon Sep 17 00:00:00 2001 From: Mattis Bratland Date: Sat, 19 Oct 2024 18:39:05 +0200 Subject: [PATCH 7/7] test: Add test to verify deployment is not recreated on pod selector match - Added test to ensure that deployment is updated without recreation when pod selector remains unchanged. - Asserts that the log does not contain the message indicating pod selector mismatch. --- build.gradle.kts | 1 + gradle/libs.versions.toml | 6 ++++- .../no/fintlabs/operator/DeploymentDRTest.kt | 25 +++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 6324a01..bf12d64 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -49,6 +49,7 @@ dependencies { testImplementation(libs.operator.framework.junit5) testImplementation(libs.bundles.fabric8test) testImplementation(libs.bundles.koinTest) + testImplementation(libs.bundles.logunit) } testing { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 431599b..2ae5e49 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -9,6 +9,7 @@ hoplite = "2.8.2" logback = "1.5.8" logstashEncoder = "8.0" jackson = "2.17.0" +logunit = "2.0.0" [libraries] fabric8-kubernetes-client = { module = "io.fabric8:kubernetes-client", version.ref = "fabric8" } @@ -26,6 +27,8 @@ fabric8-kubernetes-server-mock = { module = "io.fabric8:kubernetes-server-mock", fabric8-kube-api-test = { module = "io.fabric8:kube-api-test", version.ref = "fabric8" } operator-framework-junit5 = { module = "io.javaoperatorsdk:operator-framework-junit-5", version.ref = "operatorSdk" } awaitility-kotlin = { module = "org.awaitility:awaitility-kotlin", version.ref = "awaitility" } +logunit-core = { module = "io.github.netmikey.logunit:logunit-core", version.ref = "logunit"} +logunit-logback = { module = "io.github.netmikey.logunit:logunit-logback", version.ref = "logunit"} koin-bom = { module = "io.insert-koin:koin-bom", version.ref = "koin" } koin-core = { module = "io.insert-koin:koin-core" } @@ -42,4 +45,5 @@ koinTest = ["koin-test", "koin-test-junit5"] fabric8 = ["fabric8-kubernetes-client", "fabric8-crd-generator-apt"] fabric8test = ["fabric8-kubernetes-server-mock", "fabric8-kube-api-test"] logging = ["logback-classic", "logstash-logback-encoder"] -hoplite = ["hoplite-core", "hoplite-yaml"] \ No newline at end of file +hoplite = ["hoplite-core", "hoplite-yaml"] +logunit = ["logunit-core", "logunit-logback"] \ No newline at end of file diff --git a/src/test/integration/kotlin/no/fintlabs/operator/DeploymentDRTest.kt b/src/test/integration/kotlin/no/fintlabs/operator/DeploymentDRTest.kt index e9ee8c0..926c56a 100644 --- a/src/test/integration/kotlin/no/fintlabs/operator/DeploymentDRTest.kt +++ b/src/test/integration/kotlin/no/fintlabs/operator/DeploymentDRTest.kt @@ -6,6 +6,7 @@ import io.fabric8.kubernetes.api.model.apps.Deployment import io.fabric8.kubernetes.api.model.apps.DeploymentStrategy import io.fabric8.kubernetes.api.model.apps.RollingUpdateDeployment import io.fabric8.kubernetes.client.KubernetesClientException +import io.github.netmikey.logunit.api.LogCapturer import no.fintlabs.extensions.KubernetesOperatorContext import no.fintlabs.extensions.KubernetesResources import no.fintlabs.loadConfig @@ -13,6 +14,7 @@ import no.fintlabs.operator.Utils.createAndGetResource import no.fintlabs.operator.Utils.createKoinTestExtension import no.fintlabs.operator.Utils.createKubernetesOperatorExtension import no.fintlabs.operator.Utils.createTestFlaisApplication +import no.fintlabs.operator.Utils.updateAndGetResource import no.fintlabs.operator.Utils.waitUntilIsDeployed import no.fintlabs.operator.api.LOKI_LOGGING_LABEL import no.fintlabs.operator.api.v1alpha1.* @@ -494,12 +496,35 @@ class DeploymentDRTest { assert(deployment.spec.selector.matchLabels.containsKey("app")) assertEquals(deployment.metadata.name, deployment.spec.selector.matchLabels["app"]) } + + @Test + fun `should not recreate deployment on pod selector match`(context: KubernetesOperatorContext) { + val flaisApplication = createTestFlaisApplication() + + var deployment = context.createAndGetDeployment(flaisApplication) + assertNotNull(deployment) + + flaisApplication.apply { + spec = spec.copy( + image = "test-image:234567890" + ) + } + + + deployment = context.updateAndGetResource(flaisApplication) + assertNotNull(deployment) + logs.assertDoesNotContain("Pod selector does not match, recreating deployment") + + } //endregion private fun KubernetesOperatorContext.createAndGetDeployment(app: FlaisApplicationCrd) = createAndGetResource(app) + @RegisterExtension + val logs: LogCapturer = LogCapturer.create().captureForType(DeploymentDR::class.java) + companion object { @RegisterExtension val koinTestExtension = createKoinTestExtension(module {