diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperator.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperator.kt index f9d19ec5b..5392c0bd0 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperator.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperator.kt @@ -14,6 +14,7 @@ import com.redhat.devtools.intellij.kubernetes.model.client.ClientAdapter import com.redhat.devtools.intellij.kubernetes.model.util.ResourceException import com.redhat.devtools.intellij.kubernetes.model.util.hasGenerateName import com.redhat.devtools.intellij.kubernetes.model.util.hasName +import com.redhat.devtools.intellij.kubernetes.model.util.runWithoutServerSetProperties import io.fabric8.kubernetes.api.model.APIResource import io.fabric8.kubernetes.api.model.GenericKubernetesResource import io.fabric8.kubernetes.api.model.GenericKubernetesResourceList @@ -26,6 +27,8 @@ import io.fabric8.kubernetes.client.Watcher import io.fabric8.kubernetes.client.WatcherException import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation import io.fabric8.kubernetes.client.dsl.Resource +import io.fabric8.kubernetes.client.dsl.base.PatchContext +import io.fabric8.kubernetes.client.dsl.base.PatchType import io.fabric8.kubernetes.client.dsl.base.ResourceDefinitionContext import io.fabric8.kubernetes.client.utils.ApiVersionUtil import io.fabric8.kubernetes.client.utils.Serialization @@ -54,7 +57,7 @@ class NonCachingSingleResourceOperator( if (!hasName(resource)) { return null } - val genericKubernetesResource = toGenericKubernetesResource(resource) + val genericKubernetesResource = toGenericKubernetesResource(resource, false) val op = createOperation(resource) return op .withName(genericKubernetesResource.metadata.name) @@ -77,12 +80,12 @@ class NonCachingSingleResourceOperator( * @return the resource that was created */ fun replace(resource: HasMetadata): HasMetadata? { - val genericKubernetesResource = toGenericKubernetesResource(resource) + // force clone, patch changes the given resource + val genericKubernetesResource = toGenericKubernetesResource(resource, true) val op = createOperation(resource) return if (hasName(genericKubernetesResource)) { - op.resource(genericKubernetesResource) - .createOrReplace() + patch(genericKubernetesResource, op) } else if (hasGenerateName(genericKubernetesResource)) { op.resource(genericKubernetesResource) .create() @@ -91,6 +94,22 @@ class NonCachingSingleResourceOperator( } } + private fun patch( + genericKubernetesResource: GenericKubernetesResource, + op: NonNamespaceOperation> + ): HasMetadata? { + return runWithoutServerSetProperties(genericKubernetesResource) { + op + .resource(genericKubernetesResource) + .patch( + PatchContext.Builder() + .withForce(true) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build() + ) + } + } + /** * Creates a watch for the given resource. * Watches the resource on the cluster only if the given resource has a [io.fabric8.kubernetes.api.model.ObjectMeta.name]. @@ -101,7 +120,7 @@ class NonCachingSingleResourceOperator( * @return the resource that was created */ fun watch(resource: HasMetadata, watcher: Watcher): Watch? { - val genericKubernetesResource = toGenericKubernetesResource(resource) + val genericKubernetesResource = toGenericKubernetesResource(resource, false) if (!hasName(genericKubernetesResource)) { return null } @@ -152,10 +171,12 @@ class NonCachingSingleResourceOperator( * The given resource is returned as if it is a [GenericKubernetesResource]. * * @param resource a HasMetadata to convert + * @param clone force cloning the given resource * @return a GenericKubernetesResource */ - private fun toGenericKubernetesResource(resource: HasMetadata): GenericKubernetesResource { - return if (resource is GenericKubernetesResource) { + private fun toGenericKubernetesResource(resource: HasMetadata, clone: Boolean): GenericKubernetesResource { + return if (resource is GenericKubernetesResource + && !clone) { resource } else { val yaml = Serialization.asYaml(resource) @@ -210,4 +231,4 @@ class NonCachingSingleResourceOperator( watcher.onClose(cause) } } -} \ No newline at end of file +} diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperatorTest.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperatorTest.kt index 9f5ebd0dc..dd1172ad6 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperatorTest.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperatorTest.kt @@ -35,11 +35,14 @@ import io.fabric8.kubernetes.client.NamespacedKubernetesClient import io.fabric8.kubernetes.client.dsl.MixedOperation import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation import io.fabric8.kubernetes.client.dsl.Resource +import io.fabric8.kubernetes.client.dsl.base.PatchContext +import io.fabric8.kubernetes.client.dsl.base.PatchType import io.fabric8.kubernetes.client.dsl.base.ResourceDefinitionContext import io.fabric8.kubernetes.client.utils.ApiVersionUtil import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Test +import org.mockito.ArgumentMatcher import org.mockito.ArgumentMatchers.anyString class NonCachingSingleResourceOperatorTest { @@ -55,20 +58,12 @@ class NonCachingSingleResourceOperatorTest { .build() } - private val clusterCustomResource = GenericKubernetesResource().apply { - this.apiVersion = "rebels/tatooine" - this.kind = "Jedi" - this.metadata = ObjectMetaBuilder() - .withName("luke") - .build() - } - private val namespacedCoreResource = PodBuilder() .withApiVersion("v1") .withKind("GrandJedi") .withNewMetadata() - .withName("obwian") - .withNamespace("jedis") + .withName("obwian") + .withNamespace("jedis") .endMetadata() .build() @@ -76,26 +71,39 @@ class NonCachingSingleResourceOperatorTest { .withApiVersion("v1") .withKind("GrandJedi") .withNewMetadata() - // no namespace - .withName("Mace Windu") + // no namespace + .withName("Mace Windu") .endMetadata() .build() - /* client.genericKubernetesResources() */ - private val genericResourceOperation = - createMixedOperation(namespacedCustomResource) + /** .withName(name) */ + private val withNameOp: Resource = mock() + /** .resource(resource) */ + private val resourceOp: Resource = mock() + /** .inNamespace(namespace) */ + private val inNamespaceOp = + mock>>() { + on { withName(any()) } doReturn withNameOp + on { resource(any()) } doReturn resourceOp + } + /** client.genericKubernetesResources() */ + private val genericKubernetesResourcesOp = + mock>> { + on { inNamespace(any()) } doReturn inNamespaceOp + on { resource(any()) } doReturn resourceOp + } private val client = createClient( clientNamespace, - genericResourceOperation + genericKubernetesResourcesOp ) private val clientAdapter: KubeClientAdapter = KubeClientAdapter(client) @Before fun before() { - clearInvocations(genericResourceOperation) + clearInvocations(genericKubernetesResourcesOp) } @Test @@ -107,10 +115,10 @@ class NonCachingSingleResourceOperatorTest { operator.get(namespacedCustomResource) // then verify(client).genericKubernetesResources(argThat { - kind == namespacedCustomResource.kind - && isNamespaceScoped - && group == ApiVersionUtil.trimGroupOrNull(namespacedCustomResource.apiVersion) - && version == ApiVersionUtil.trimVersion(namespacedCustomResource.apiVersion) + kind == namespacedCustomResource.kind + && isNamespaceScoped + && group == ApiVersionUtil.trimGroupOrNull(namespacedCustomResource.apiVersion) + && version == ApiVersionUtil.trimVersion(namespacedCustomResource.apiVersion) }) } @@ -138,7 +146,7 @@ class NonCachingSingleResourceOperatorTest { // when operator.get(namespacedCustomResource) // then - verify(genericResourceOperation).inNamespace(namespacedCustomResource.metadata.namespace) + verify(genericKubernetesResourcesOp).inNamespace(namespacedCustomResource.metadata.namespace) } @Test @@ -153,18 +161,7 @@ class NonCachingSingleResourceOperatorTest { // when operator.get(namespacedCustomResource) // then - verify(genericResourceOperation).inNamespace(client.namespace) - } - - @Test - fun `#get should NOT call client#genericKubernetesResources()#inNamespace() if resource is cluster scoped`() { - // given - val apiResource = clusterScopedApiResource(clusterCustomResource) - val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource)) - // when - operator.get(clusterCustomResource) - // then - verify(genericResourceOperation, never()).inNamespace(any()) + verify(genericKubernetesResourcesOp).inNamespace(client.namespace) } @Test @@ -175,7 +172,7 @@ class NonCachingSingleResourceOperatorTest { // when operator.get(namespacedCustomResource) // then - verify(genericResourceOperation.inNamespace(namespacedCoreResource.metadata.name)) + verify(genericKubernetesResourcesOp.inNamespace(namespacedCoreResource.metadata.name)) .withName(namespacedCustomResource.metadata.name) } @@ -189,73 +186,65 @@ class NonCachingSingleResourceOperatorTest { } @Test - fun `#replace should call #createOrReplace() if namespaced resource has a name`() { + fun `#replace should call #inNamespace for namespaced resource`() { // given - val hasName = PodBuilder(namespacedCoreResource).build() - hasName.metadata.name = "yoda" - hasName.metadata.generateName = null val apiResource = namespacedApiResource(namespacedCoreResource) val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource)) // when - operator.replace(namespacedCustomResource) + operator.replace(namespacedCoreResource) // then - verify(genericResourceOperation - .inNamespace(namespacedCustomResource.metadata.namespace) - .resource(namespacedCustomResource)) - .createOrReplace() + verify(genericKubernetesResourcesOp) + .inNamespace(namespacedCoreResource.metadata.namespace) } @Test - fun `#replace should call #createOrReplace() if clusterscoped resource has a name`() { + fun `#replace should NOT call #inNamespace for clusterScoped resource`() { // given - val hasName = PodBuilder(clusterscopedCoreResource).build() - hasName.metadata.name = "yoda" - hasName.metadata.generateName = null - val apiResource = namespacedApiResource(clusterscopedCoreResource) + val apiResource = clusterScopedApiResource(clusterscopedCoreResource) val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource)) // when - operator.replace(namespacedCustomResource) + operator.replace(clusterscopedCoreResource) // then - verify(genericResourceOperation - .resource(namespacedCustomResource)) - .createOrReplace() + verify(genericKubernetesResourcesOp, never()) + .inNamespace(any()) } @Test - fun `#replace should call #create() if namespaced resource has NO name but has generateName`() { + fun `#replace should call #patch() if resource has a name`() { // given - val hasGeneratedName = PodBuilder(namespacedCoreResource).build() - hasGeneratedName.metadata.name = null - hasGeneratedName.metadata.generateName = "storm trooper clone" - val apiResource = namespacedApiResource(hasGeneratedName) + val hasName = PodBuilder(namespacedCoreResource).build() + hasName.metadata.name = "yoda" + hasName.metadata.generateName = null + val apiResource = namespacedApiResource(namespacedCoreResource) val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource)) // when - operator.replace(hasGeneratedName) + operator.replace(hasName) // then - verify(genericResourceOperation - .inNamespace(namespacedCustomResource.metadata.namespace) - .resource(namespacedCustomResource)) - .create() + verify(resourceOp) + .patch(argThat(ArgumentMatcher { context -> + context.patchType == PatchType.SERVER_SIDE_APPLY + })) } @Test - fun `#replace should call #create() if clusterscoped resource has NO name but has generateName`() { + fun `#replace should call #create() if resource has NO name but has generateName`() { // given - val hasGeneratedName = PodBuilder(clusterscopedCoreResource).build() + val hasGeneratedName = PodBuilder(namespacedCoreResource).build() hasGeneratedName.metadata.name = null hasGeneratedName.metadata.generateName = "storm trooper clone" - val apiResource = namespacedApiResource(hasGeneratedName) - val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource)) + val operator = NonCachingSingleResourceOperator( + clientAdapter, + createAPIResources(namespacedApiResource(hasGeneratedName)) + ) // when operator.replace(hasGeneratedName) // then - verify(genericResourceOperation - .resource(namespacedCustomResource)) + verify(resourceOp) .create() } @Test(expected = ResourceException::class) - fun `#replace should throw if namespaced resource has NO name NOR generateName`() { + fun `#replace should throw if resource has NO name NOR generateName`() { // given val generatedName = PodBuilder(namespacedCoreResource).build() generatedName.metadata.name = null @@ -267,6 +256,18 @@ class NonCachingSingleResourceOperatorTest { // then } + @Test + fun `#replace should return a clone, NOT the resource that it was given`() { + // given + val resource = PodBuilder(namespacedCoreResource).build() + val apiResource = namespacedApiResource(namespacedCustomResource) + val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource)) + // when + val returned = operator.replace(resource) + // then + assertThat(returned).isNotSameAs(resource) + } + @Test fun `#watch should call client#genericKubernetesResource(context) if resource has a name`() { // given @@ -338,33 +339,6 @@ class NonCachingSingleResourceOperatorTest { return client } - private inline fun createMixedOperation(resource: T): MixedOperation> { - val withNameInNamespaceOp: Resource = createWithNameOp(createFromServerOp(resource)) - val resourceOperation: Resource = mock() - val inNamespaceOp: MixedOperation> = mock { - on { withName(any()) } doReturn withNameInNamespaceOp - on { resource(any()) } doReturn resourceOperation - } - val withNameOp: Resource = createWithNameOp(createFromServerOp(resource)) - return mock { - on { inNamespace(any()) } doReturn inNamespaceOp - on { withName(any()) } doReturn withNameOp - on { resource(any()) } doReturn resourceOperation - } - } - - private fun createWithNameOp(fromServerOp: Resource): Resource { - return mock { - on { fromServer() } doReturn fromServerOp - } - } - - private fun createFromServerOp(resource: T): Resource { - return mock { - on { get() } doReturn resource - } - } - private fun createAPIResources(apiResource: APIResource?): APIResources { val apiResources = spy(APIResources(mock())) doReturn(apiResource)