Skip to content

Commit

Permalink
use client#patch instead of #createOrReplace (#561) (#580)
Browse files Browse the repository at this point in the history 
Signed-off-by: Andre Dietisheim <adietish@redhat.com>
  • Loading branch information
@adietish
adietish authored Mar 14, 2023
1 parent c6df668 commit 67cb78f
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 46 deletions.
42 changes: 33 additions & 9 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,11 +80,11 @@ 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 @@ -90,6 +93,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 @@ -100,7 +119,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 @@ -148,13 +167,18 @@ class NonCachingSingleResourceOperator(

/**
* Returns a [GenericKubernetesResource] for a given [HasMetadata].
* The given resource is returned as if it is a [GenericKubernetesResource].
* If the given resource is a [GenericKubernetesResource] then the resource is returned as is unless
* [clone] is `true`. In this case a clone is returned instead.
* If the given resource is not a [GenericKubernetesResource] then an equivalent [GenericKubernetesResource]
* is created using serialization.
*
* @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 = false): GenericKubernetesResource {
return if (resource is GenericKubernetesResource
&& !clone) {
resource
} else {
val yaml = Serialization.asYaml(resource)
Expand Down Expand Up @@ -209,4 +233,4 @@ class NonCachingSingleResourceOperator(
watcher.onClose(cause)
}
}
}
}
90 changes: 53 additions & 37 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,29 +58,21 @@ 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()

Expand Down Expand Up @@ -120,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 @@ -191,46 +186,55 @@ 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(hasName)
operator.replace(namespacedCoreResource)
// then
verify(resourceOp)
.createOrReplace()
verify(genericKubernetesResourcesOp)
.inNamespace(namespacedCoreResource.metadata.namespace)
}

@Test
fun `#replace should call #create() if namespaced resource has NO name but has generateName`() {
fun `#replace should NOT call #inNamespace for clusterScoped resource`() {
// given
val hasGeneratedName = PodBuilder(namespacedCoreResource).build()
hasGeneratedName.metadata.name = null
hasGeneratedName.metadata.generateName = "storm trooper clone"
val operator = NonCachingSingleResourceOperator(
clientAdapter,
createAPIResources(namespacedApiResource(hasGeneratedName))
)
val apiResource = clusterScopedApiResource(clusterscopedCoreResource)
val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource))
// when
operator.replace(hasGeneratedName)
operator.replace(clusterscopedCoreResource)
// then
verify(genericKubernetesResourcesOp, never())
.inNamespace(any())
}

@Test
fun `#replace should call #patch() if resource has a name`() {
// 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(hasName)
// then
verify(resourceOp)
.create()
.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 operator = NonCachingSingleResourceOperator(
clientAdapter,
createAPIResources(clusterScopedApiResource(hasGeneratedName))
createAPIResources(namespacedApiResource(hasGeneratedName))
)
// when
operator.replace(hasGeneratedName)
Expand All @@ -240,7 +244,7 @@ class NonCachingSingleResourceOperatorTest {
}

@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 All @@ -252,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
Expand Down

0 comments on commit 67cb78f

Please sign in to comment.