From c0e3d1a2b4848be6246cdeb4b75c4ee6ac6f4dfb Mon Sep 17 00:00:00 2001 From: Jan Ove Kongshaug Date: Fri, 27 Feb 2026 09:40:06 +0100 Subject: [PATCH] fix: recreate deployments with stale legacy-managed env vars We have observed pods keeping old env vars after spec changes, which can cause invalid runtime config (for example stale servlet/webflux base paths). Root cause is split SSA ownership in managedFields: old deployments still have env entries owned by the legacy field manager `flaisapplicationreconciler`. When desired state removes those vars, normal apply from the current reconciler may not clear them. This change does two things: - set `fieldManager = "applicationreconciler"` in ApplicationReconciler to keep future SSA ownership stable - in DeploymentDR, detect stale env vars (`actual - desired`, per container) and recreate deployment when both conditions are true: - legacy apply manager is present - stale env vars exist Added unit tests for: - recreate with legacy manager + stale env vars - no recreate without legacy manager - no recreate when env is already in sync - container-aware stale env identity Potential downsides: - affected deployments may roll once due to delete/recreate when healed - manually added env vars not present in desired state can be removed during healing (expected for desired-state reconciliation) - detection currently targets known legacy manager names; more can be added if historical manager variants are discovered --- .../application/ApplicationReconciler.kt | 3 +- .../no/fintlabs/application/DeploymentDR.kt | 47 +++++++ .../DeploymentDRLegacyFieldOwnershipTest.kt | 119 ++++++++++++++++++ 3 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 src/test/unit/kotlin/no/fintlabs/application/DeploymentDRLegacyFieldOwnershipTest.kt diff --git a/src/main/kotlin/no/fintlabs/application/ApplicationReconciler.kt b/src/main/kotlin/no/fintlabs/application/ApplicationReconciler.kt index 1945606..78de3a0 100644 --- a/src/main/kotlin/no/fintlabs/application/ApplicationReconciler.kt +++ b/src/main/kotlin/no/fintlabs/application/ApplicationReconciler.kt @@ -15,10 +15,11 @@ import org.koin.core.component.KoinComponent @GradualRetry(maxAttempts = 3) @ControllerConfiguration( + fieldManager = "applicationreconciler", informer = Informer( genericFilter = FlaisResourceReconciliationFilter::class, - ) + ), ) @Workflow( [ diff --git a/src/main/kotlin/no/fintlabs/application/DeploymentDR.kt b/src/main/kotlin/no/fintlabs/application/DeploymentDR.kt index ee4ff76..4f8063f 100644 --- a/src/main/kotlin/no/fintlabs/application/DeploymentDR.kt +++ b/src/main/kotlin/no/fintlabs/application/DeploymentDR.kt @@ -68,6 +68,15 @@ class DeploymentDR : primary: FlaisApplication, context: Context, ): Deployment { + val staleEnvVarNames = staleLegacyEnvVarNames(actual, desired) + if (shouldRecreateForLegacyFieldOwnership(actual, staleEnvVarNames)) { + logger.info( + "Recreating deployment ${actual.metadata.name} to clear stale legacy SSA-owned env vars from manager(s) $LEGACY_FIELD_MANAGERS: ${staleEnvVarNames.joinToString()}", + ) + handleDelete(primary, actual, context) + return handleCreate(desired, primary, context) + } + val kubernetesSerialization = context.client.kubernetesSerialization val desiredSelector = kubernetesSerialization.convertValue(desired.spec.selector, Map::class.java) @@ -174,4 +183,42 @@ class DeploymentDR : startsWith("/") -> this else -> "/$this" } + + companion object { + private const val APPLY_OPERATION = "Apply" + + private val LEGACY_FIELD_MANAGERS = setOf("flaisapplicationreconciler") + + private const val DEFAULT_CONTAINER_NAME = "app" + + internal fun shouldRecreateForLegacyFieldOwnership( + actual: Deployment, + staleEnvVarNames: Set, + ): Boolean { + val hasLegacyApplyManager = + actual.metadata?.managedFields.orEmpty().any { + it.operation == APPLY_OPERATION && it.manager in LEGACY_FIELD_MANAGERS + } + + return hasLegacyApplyManager && staleEnvVarNames.isNotEmpty() + } + + internal fun staleLegacyEnvVarNames(actual: Deployment, desired: Deployment): Set { + val desiredByContainer = envNamesByContainer(desired) + + return envNamesByContainer(actual) + .flatMap { (containerName, actualEnvNames) -> + val desiredEnvNames = desiredByContainer[containerName].orEmpty() + (actualEnvNames - desiredEnvNames).map { envName -> "$containerName:$envName" } + } + .toSet() + } + + private fun envNamesByContainer(deployment: Deployment): Map> = + deployment.spec?.template?.spec?.containers.orEmpty().associate { container -> + val containerName = container.name ?: DEFAULT_CONTAINER_NAME + val envNames = container.env.orEmpty().mapNotNull { env -> env.name }.toSet() + containerName to envNames + } + } } diff --git a/src/test/unit/kotlin/no/fintlabs/application/DeploymentDRLegacyFieldOwnershipTest.kt b/src/test/unit/kotlin/no/fintlabs/application/DeploymentDRLegacyFieldOwnershipTest.kt new file mode 100644 index 0000000..9a725bb --- /dev/null +++ b/src/test/unit/kotlin/no/fintlabs/application/DeploymentDRLegacyFieldOwnershipTest.kt @@ -0,0 +1,119 @@ +package no.fintlabs.application + +import io.fabric8.kubernetes.api.model.Container +import io.fabric8.kubernetes.api.model.EnvVar +import io.fabric8.kubernetes.api.model.ManagedFieldsEntry +import io.fabric8.kubernetes.api.model.ObjectMeta +import io.fabric8.kubernetes.api.model.PodSpec +import io.fabric8.kubernetes.api.model.PodTemplateSpec +import io.fabric8.kubernetes.api.model.apps.Deployment +import io.fabric8.kubernetes.api.model.apps.DeploymentSpec +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class DeploymentDRLegacyFieldOwnershipTest { + @Test + fun `should recreate when legacy manager owns any stale env vars`() { + val actual = + deployment( + envNames = setOf("JAVA_TOOL_OPTIONS", "MY_STALE_ENV"), + managers = setOf("flaisapplicationreconciler"), + ) + val desired = + deployment(envNames = setOf("JAVA_TOOL_OPTIONS"), managers = setOf("applicationreconciler")) + + val staleEnvVarNames = DeploymentDR.staleLegacyEnvVarNames(actual, desired) + assertTrue(DeploymentDR.shouldRecreateForLegacyFieldOwnership(actual, staleEnvVarNames)) + } + + @Test + fun `should not recreate when legacy manager is absent`() { + val actual = + deployment( + envNames = setOf("JAVA_TOOL_OPTIONS", "MY_STALE_ENV"), + managers = setOf("applicationreconciler"), + ) + val desired = + deployment(envNames = setOf("JAVA_TOOL_OPTIONS"), managers = setOf("applicationreconciler")) + + val staleEnvVarNames = DeploymentDR.staleLegacyEnvVarNames(actual, desired) + assertFalse(DeploymentDR.shouldRecreateForLegacyFieldOwnership(actual, staleEnvVarNames)) + } + + @Test + fun `should not recreate when stale env vars are absent`() { + val actual = + deployment( + envNames = setOf("JAVA_TOOL_OPTIONS", "MY_ENV"), + managers = setOf("flaisapplicationreconciler"), + ) + val desired = + deployment( + envNames = setOf("JAVA_TOOL_OPTIONS", "MY_ENV"), + managers = setOf("applicationreconciler"), + ) + + val staleEnvVarNames = DeploymentDR.staleLegacyEnvVarNames(actual, desired) + assertFalse(DeploymentDR.shouldRecreateForLegacyFieldOwnership(actual, staleEnvVarNames)) + } + + @Test + fun `should include container name in stale env var identity`() { + val actual = + deployment( + envNames = setOf("JAVA_TOOL_OPTIONS", "MY_STALE_ENV"), + managers = setOf("flaisapplicationreconciler"), + containerName = "my-app", + ) + val desired = + deployment( + envNames = setOf("JAVA_TOOL_OPTIONS"), + managers = setOf("applicationreconciler"), + containerName = "my-app", + ) + + val staleEnvVarNames = DeploymentDR.staleLegacyEnvVarNames(actual, desired) + assertTrue("my-app:MY_STALE_ENV" in staleEnvVarNames) + } + + private fun deployment( + envNames: Set, + managers: Set, + containerName: String = "test", + ): Deployment = + Deployment().apply { + metadata = + ObjectMeta().apply { + name = "test" + managedFields = + managers + .map { manager -> + ManagedFieldsEntry().apply { + this.manager = manager + operation = "Apply" + } + } + .toMutableList() + } + spec = + DeploymentSpec().apply { + template = + PodTemplateSpec().apply { + spec = + PodSpec().apply { + containers = + mutableListOf( + Container().apply { + name = containerName + env = + envNames + .map { envName -> EnvVar(envName, "v", null) } + .toMutableList() + } + ) + } + } + } + } +}