Skip to content

Commit 3eb784a

Browse files
adietishadietish
committed
fix: use #create/replace not #patch when resource has managed fields (redhat-developer#755)
Signed-off-by: Andre Dietisheim <[email protected]>
1 parent a0c4512 commit 3eb784a

File tree

8 files changed

+210
-23
lines changed

8 files changed

+210
-23
lines changed

Diff for: ‎src/main/kotlin/com/redhat/devtools/intellij/kubernetes/editor/ClusterResource.kt

+5-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,11 @@ open class ClusterResource protected constructor(
140140
"Unsupported resource kind ${resource.kind} in version ${resource.apiVersion}."
141141
)
142142
}
143-
val updated = context.replace(resource)
143+
val updated = if (exists()) {
144+
context.replace(resource)
145+
} else {
146+
context.create(resource)
147+
}
144148
set(updated)
145149
return updated
146150
} catch (e: KubernetesClientException) {

Diff for: ‎src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/context/ActiveContext.kt

+4
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,10 @@ abstract class ActiveContext<N : HasMetadata, C : KubernetesClient>(
266266
return singleResourceOperator.get(resource)
267267
}
268268

269+
override fun create(resource: HasMetadata): HasMetadata? {
270+
return singleResourceOperator.create(resource)
271+
}
272+
269273
override fun replace(resource: HasMetadata): HasMetadata? {
270274
return singleResourceOperator.replace(resource)
271275
}

Diff for: ‎src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/context/IActiveContext.kt

+11-2
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,21 @@ interface IActiveContext<N: HasMetadata, C: KubernetesClient>: IContext {
142142
fun get(resource: HasMetadata): HasMetadata?
143143

144144
/**
145-
* Replaces the given resource on the cluster if it exists. Creates a new one if it doesn't.
145+
* Creates the given resource on the cluster if it doesn't exist. Throws if it exists already.
146146
*
147-
* @param resource that shall be replaced on the cluster
147+
* @param resource that shall be created on the cluster
148148
*
149149
* @return the resource that was created
150150
*/
151+
fun create(resource: HasMetadata): HasMetadata?
152+
153+
/**
154+
* Replaces the given resource on the cluster if it exists. Throws if it doesn't.
155+
*
156+
* @param resource that shall be replaced on the cluster
157+
*
158+
* @return the resource that was replaced
159+
*/
151160
fun replace(resource: HasMetadata): HasMetadata?
152161

153162
/**

Diff for: ‎src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperator.kt

+25-6
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ package com.redhat.devtools.intellij.kubernetes.model.resource
1313
import com.redhat.devtools.intellij.kubernetes.model.client.ClientAdapter
1414
import com.redhat.devtools.intellij.kubernetes.model.util.ResourceException
1515
import com.redhat.devtools.intellij.kubernetes.model.util.hasGenerateName
16+
import com.redhat.devtools.intellij.kubernetes.model.util.hasManagedFields
1617
import com.redhat.devtools.intellij.kubernetes.model.util.hasName
1718
import com.redhat.devtools.intellij.kubernetes.model.util.runWithoutServerSetProperties
1819
import io.fabric8.kubernetes.api.model.APIResource
@@ -84,26 +85,44 @@ class NonCachingSingleResourceOperator(
8485
val genericKubernetesResource = toGenericKubernetesResource(resource, true)
8586
val op = createOperation(resource)
8687
return if (hasName(genericKubernetesResource)) {
87-
patch(genericKubernetesResource, op)
88+
if (hasManagedFields(genericKubernetesResource)) {
89+
patch(genericKubernetesResource, op, PatchType.JSON_MERGE)
90+
} else {
91+
patch(genericKubernetesResource, op, PatchType.SERVER_SIDE_APPLY)
92+
}
8893
} else if (hasGenerateName(genericKubernetesResource)) {
89-
op.resource(genericKubernetesResource)
90-
.create()
94+
op.resource(genericKubernetesResource).create()
9195
} else {
9296
throw ResourceException("Could not replace ${resource.kind ?: "resource"}: has neither name nor generateName.")
9397
}
9498
}
9599

100+
fun create(resource: HasMetadata): HasMetadata? {
101+
// force clone, patch changes the given resource
102+
val genericKubernetesResource = toGenericKubernetesResource(resource, true)
103+
val op = createOperation(resource)
104+
return if (hasName(genericKubernetesResource)
105+
&& !hasManagedFields(genericKubernetesResource)
106+
) {
107+
patch(genericKubernetesResource, op, PatchType.SERVER_SIDE_APPLY)
108+
} else {
109+
op.resource(genericKubernetesResource)
110+
.create()
111+
}
112+
}
113+
96114
private fun patch(
97115
genericKubernetesResource: GenericKubernetesResource,
98-
op: NonNamespaceOperation<GenericKubernetesResource, GenericKubernetesResourceList, Resource<GenericKubernetesResource>>
116+
op: NonNamespaceOperation<GenericKubernetesResource, GenericKubernetesResourceList, Resource<GenericKubernetesResource>>,
117+
patchType: PatchType
99118
): HasMetadata? {
100119
return runWithoutServerSetProperties(genericKubernetesResource) {
101120
op
102121
.resource(genericKubernetesResource)
103122
.patch(
104123
PatchContext.Builder()
105-
.withForce(true)
106-
.withPatchType(PatchType.SERVER_SIDE_APPLY)
124+
//.withForce(true)
125+
.withPatchType(patchType)
107126
.build()
108127
)
109128
}

Diff for: ‎src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/util/ResourceUtils.kt

+4
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ fun hasDeletionTimestamp(resource: HasMetadata?): Boolean {
172172
return null != resource?.metadata?.deletionTimestamp
173173
}
174174

175+
fun hasManagedFields(resource: HasMetadata?): Boolean {
176+
return true == resource?.metadata?.managedFields?.isNotEmpty()
177+
}
178+
175179
fun setWillBeDeleted(resource: HasMetadata) {
176180
setDeletionTimestamp(MARKER_WILL_BE_DELETED, resource)
177181
}

Diff for: ‎src/test/kotlin/com/redhat/devtools/intellij/kubernetes/editor/ClusterResourceTest.kt

+13-2
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,21 @@ class ClusterResourceTest {
125125
}
126126

127127
@Test
128-
fun `#push should call operator#replace`() {
128+
fun `#push should call operator#create if resource does NOT exist`() {
129129
// given
130130
whenever(context.get(any()))
131-
.doReturn(null)
131+
.thenReturn(null)
132+
// when
133+
cluster.push(endorResourceOnCluster)
134+
// then
135+
verify(context).create(endorResourceOnCluster)
136+
}
137+
138+
@Test
139+
fun `#push should call operator#replace if resource exists`() {
140+
// given
141+
whenever(context.get(any()))
142+
.thenReturn(endorResourceOnCluster)
132143
// when
133144
cluster.push(endorResourceOnCluster)
134145
// then

Diff for: ‎src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/resource/NonCachingSingleResourceOperatorTest.kt

+103-12
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import io.fabric8.kubernetes.api.model.APIResource
2828
import io.fabric8.kubernetes.api.model.GenericKubernetesResource
2929
import io.fabric8.kubernetes.api.model.GenericKubernetesResourceList
3030
import io.fabric8.kubernetes.api.model.HasMetadata
31+
import io.fabric8.kubernetes.api.model.ManagedFieldsEntry
3132
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder
3233
import io.fabric8.kubernetes.api.model.PodBuilder
3334
import io.fabric8.kubernetes.client.KubernetesClientException
@@ -185,6 +186,63 @@ class NonCachingSingleResourceOperatorTest {
185186
// then
186187
}
187188

189+
@Test
190+
fun `#create should call #patch(SERVER_SIDE_APPLY) if resource has a name and NO managed fields`() {
191+
// given
192+
val metadata = ObjectMetaBuilder().build().apply {
193+
managedFields = null
194+
}
195+
val hasName = PodBuilder(namespacedCoreResource)
196+
.withMetadata(metadata)
197+
.build()
198+
hasName.metadata.name = "yoda"
199+
hasName.metadata.generateName = null
200+
val apiResource = namespacedApiResource(namespacedCoreResource)
201+
val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource))
202+
// when
203+
operator.create(hasName)
204+
// then
205+
verify(resourceOp)
206+
.patch(argThat(ArgumentMatcher<PatchContext> { context ->
207+
context.patchType == PatchType.SERVER_SIDE_APPLY
208+
}))
209+
}
210+
211+
@Test
212+
fun `#create should call #create if resource has no name`() {
213+
// given
214+
val hasNoName = PodBuilder(namespacedCoreResource)
215+
.withNewMetadata()
216+
.withManagedFields(ManagedFieldsEntry())
217+
.endMetadata()
218+
.build()
219+
val apiResource = namespacedApiResource(namespacedCoreResource)
220+
val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource))
221+
// when
222+
operator.create(hasNoName)
223+
// then
224+
verify(resourceOp)
225+
.create()
226+
}
227+
228+
@Test
229+
fun `#create should call #create if resource has a name but managed fields`() {
230+
// given
231+
val hasNameAndManagedFields = PodBuilder(namespacedCoreResource)
232+
.withNewMetadata()
233+
.withManagedFields(ManagedFieldsEntry())
234+
.withName("obiwan")
235+
.endMetadata()
236+
.build()
237+
val apiResource = namespacedApiResource(namespacedCoreResource)
238+
val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource))
239+
// when
240+
operator.create(hasNameAndManagedFields)
241+
// then
242+
verify(resourceOp)
243+
.create()
244+
}
245+
188246
@Test
189247
fun `#replace should call #inNamespace for namespaced resource`() {
190248
// given
@@ -210,11 +268,40 @@ class NonCachingSingleResourceOperatorTest {
210268
}
211269

212270
@Test
213-
fun `#replace should call #patch() if resource has a name`() {
271+
fun `#replace should call #patch(JSON_MERGE) if resource has a name and managed fields`() {
214272
// given
215-
val hasName = PodBuilder(namespacedCoreResource).build()
216-
hasName.metadata.name = "yoda"
217-
hasName.metadata.generateName = null
273+
val metadata = ObjectMetaBuilder()
274+
.withManagedFields(ManagedFieldsEntry())
275+
.build().apply {
276+
name = "yoda"
277+
generateName = null
278+
}
279+
val hasName = PodBuilder(namespacedCoreResource)
280+
.withMetadata(metadata)
281+
.build()
282+
val apiResource = namespacedApiResource(namespacedCoreResource)
283+
val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource))
284+
// when
285+
operator.replace(hasName)
286+
// then
287+
verify(resourceOp)
288+
.patch(argThat(ArgumentMatcher<PatchContext> { context ->
289+
context.patchType == PatchType.JSON_MERGE
290+
}))
291+
}
292+
293+
@Test
294+
fun `#replace should call #patch(SERVER_SIDE_APPLY) if resource has a name and NO managed fields`() {
295+
// given
296+
val metadata = ObjectMetaBuilder()
297+
.build().apply {
298+
name = "yoda"
299+
generateName = null
300+
managedFields = null
301+
}
302+
val hasName = PodBuilder(namespacedCoreResource)
303+
.withMetadata(metadata)
304+
.build()
218305
val apiResource = namespacedApiResource(namespacedCoreResource)
219306
val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource))
220307
// when
@@ -229,9 +316,10 @@ class NonCachingSingleResourceOperatorTest {
229316
@Test
230317
fun `#replace should call #create() if resource has NO name but has generateName`() {
231318
// given
232-
val hasGeneratedName = PodBuilder(namespacedCoreResource).build()
233-
hasGeneratedName.metadata.name = null
234-
hasGeneratedName.metadata.generateName = "storm trooper clone"
319+
val hasGeneratedName = PodBuilder(namespacedCoreResource).build().apply {
320+
metadata.name = null
321+
metadata.generateName = "storm trooper clone"
322+
}
235323
val operator = NonCachingSingleResourceOperator(
236324
clientAdapter,
237325
createAPIResources(namespacedApiResource(hasGeneratedName))
@@ -246,9 +334,10 @@ class NonCachingSingleResourceOperatorTest {
246334
@Test(expected = ResourceException::class)
247335
fun `#replace should throw if resource has NO name NOR generateName`() {
248336
// given
249-
val generatedName = PodBuilder(namespacedCoreResource).build()
250-
generatedName.metadata.name = null
251-
generatedName.metadata.generateName = null
337+
val generatedName = PodBuilder(namespacedCoreResource).build().apply {
338+
metadata.name = null
339+
metadata.generateName = null
340+
}
252341
val apiResource = namespacedApiResource(namespacedCustomResource)
253342
val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource))
254343
// when
@@ -306,8 +395,10 @@ class NonCachingSingleResourceOperatorTest {
306395
@Test
307396
fun `#watch should return NULL if resource has NO name`() {
308397
// given
309-
val unnamed = PodBuilder(namespacedCoreResource).build()
310-
unnamed.metadata.name = null
398+
val unnamed = PodBuilder(namespacedCoreResource).build().apply {
399+
metadata.name = null
400+
}
401+
311402
val apiResource = namespacedApiResource(unnamed)
312403
val operator = NonCachingSingleResourceOperator(clientAdapter, createAPIResources(apiResource))
313404
// when

Diff for: ‎src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/util/ResourceUtilsTest.kt

+45
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ package com.redhat.devtools.intellij.kubernetes.model.util
1212

1313
import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.resource
1414
import io.fabric8.kubernetes.api.model.HasMetadata
15+
import io.fabric8.kubernetes.api.model.ManagedFieldsEntry
1516
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder
1617
import io.fabric8.kubernetes.api.model.Pod
1718
import io.fabric8.kubernetes.api.model.PodBuilder
@@ -525,4 +526,48 @@ class ResourceUtilsTest {
525526
assertThat(resource2.metadata.resourceVersion).isEqualTo("21")
526527
assertThat(resource2.metadata.uid).isEqualTo("obiwan")
527528
}
529+
530+
@Test
531+
fun `#hasManagedField should return true if resource has managed fields `() {
532+
// given
533+
val meta = ObjectMetaBuilder()
534+
.withManagedFields(ManagedFieldsEntry())
535+
.build()
536+
val neo = PodBuilder()
537+
.withMetadata(meta)
538+
.build()
539+
// when
540+
val hasManagedFields = hasManagedFields(neo)
541+
// then
542+
assertThat(hasManagedFields).isTrue()
543+
}
544+
545+
@Test
546+
fun `#hasManagedField should return false if resource has empty list of managed fields `() {
547+
// given
548+
val meta = ObjectMetaBuilder()
549+
.build()
550+
meta.managedFields = emptyList()
551+
val neo = PodBuilder()
552+
.withMetadata(meta)
553+
.build()
554+
// when
555+
val hasManagedFields = hasManagedFields(neo)
556+
// then
557+
assertThat(hasManagedFields).isFalse()
558+
}
559+
560+
@Test
561+
fun `#hasManagedField should return false if resource has no managed fields `() {
562+
// given
563+
val meta = ObjectMetaBuilder().build()
564+
val neo = PodBuilder()
565+
.withMetadata(meta)
566+
.build()
567+
// when
568+
val hasManagedFields = hasManagedFields(neo)
569+
// then
570+
assertThat(hasManagedFields).isFalse()
571+
}
572+
528573
}

0 commit comments

Comments
 (0)