Skip to content

Commit

Permalink
use client#patch instead of #createOrReplace (redhat-developer#561)
Browse files Browse the repository at this point in the history 
Signed-off-by: Andre Dietisheim <adietish@redhat.com>
  • Loading branch information
@adietish
adietish committed Mar 13, 2023
1 parent d884231 commit c727083
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 105 deletions.
37 changes: 29 additions & 8 deletions ...om/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -91,6 +94,22 @@ class NonCachingSingleResourceOperator(
}
}

private fun patch(
genericKubernetesResource: GenericKubernetesResource,
op: NonNamespaceOperation<GenericKubernetesResource, GenericKubernetesResourceList, Resource<GenericKubernetesResource>>
): 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].
Expand All @@ -101,7 +120,7 @@ class NonCachingSingleResourceOperator(
* @return the resource that was created
*/
fun watch(resource: HasMetadata, watcher: Watcher<HasMetadata>): Watch? {
val genericKubernetesResource = toGenericKubernetesResource(resource)
val genericKubernetesResource = toGenericKubernetesResource(resource, false)
if (!hasName(genericKubernetesResource)) {
return null
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -210,4 +231,4 @@ class NonCachingSingleResourceOperator(
watcher.onClose(cause)
}
}
}
}
156 changes: 59 additions & 97 deletions ...edhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperatorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -55,47 +58,52 @@ 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()

private val clusterscopedCoreResource = PodBuilder()
.withApiVersion("v1")
.withKind("GrandJedi")
.withNewMetadata()
// no namespace
.withName("Mace Windu")
// no namespace
.withName("Mace Windu")
.endMetadata()
.build()

/* client.genericKubernetesResources() */
private val genericResourceOperation =
createMixedOperation<GenericKubernetesResource, GenericKubernetesResourceList>(namespacedCustomResource)
/** .withName(name) */
private val withNameOp: Resource<GenericKubernetesResource> = mock()
/** .resource(resource) */
private val resourceOp: Resource<GenericKubernetesResource> = mock()
/** .inNamespace(namespace) */
private val inNamespaceOp =
mock<MixedOperation<GenericKubernetesResource, GenericKubernetesResourceList, Resource<GenericKubernetesResource>>>() {
on { withName(any()) } doReturn withNameOp
on { resource(any()) } doReturn resourceOp
}

/** client.genericKubernetesResources() */
private val genericKubernetesResourcesOp =
mock<MixedOperation<GenericKubernetesResource, GenericKubernetesResourceList, Resource<GenericKubernetesResource>>> {
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
Expand All @@ -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)
})
}

Expand Down Expand Up @@ -138,7 +146,7 @@ class NonCachingSingleResourceOperatorTest {
// when
operator.get(namespacedCustomResource)
// then
verify(genericResourceOperation).inNamespace(namespacedCustomResource.metadata.namespace)
verify(genericKubernetesResourcesOp).inNamespace(namespacedCustomResource.metadata.namespace)
}

@Test
Expand All @@ -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
Expand All @@ -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)
}

Expand All @@ -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<PatchContext> { 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
Expand Down Expand Up @@ -338,33 +327,6 @@ class NonCachingSingleResourceOperatorTest {
return client
}

private inline fun <reified T, L> createMixedOperation(resource: T): MixedOperation<T, L, Resource<T>> {
val withNameInNamespaceOp: Resource<T> = createWithNameOp(createFromServerOp(resource))
val resourceOperation: Resource<T> = mock()
val inNamespaceOp: MixedOperation<T, L, Resource<T>> = mock {
on { withName(any()) } doReturn withNameInNamespaceOp
on { resource(any()) } doReturn resourceOperation
}
val withNameOp: Resource<T> = createWithNameOp(createFromServerOp(resource))
return mock {
on { inNamespace(any()) } doReturn inNamespaceOp
on { withName(any()) } doReturn withNameOp
on { resource(any()) } doReturn resourceOperation
}
}

private fun <T> createWithNameOp(fromServerOp: Resource<T>): Resource<T> {
return mock {
on { fromServer() } doReturn fromServerOp
}
}

private fun <T> createFromServerOp(resource: T): Resource<T> {
return mock {
on { get() } doReturn resource
}
}

private fun createAPIResources(apiResource: APIResource?): APIResources {
val apiResources = spy(APIResources(mock()))
doReturn(apiResource)
Expand Down

0 comments on commit c727083

Please sign in to comment.