Skip to content

Commit 204d346

Browse files
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 204d346

File tree

7 files changed

+107
-11
lines changed

7 files changed

+107
-11
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/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)