Skip to content

Commit 11e5232

Browse files
csviricsviri
committed
wip
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
1 parent 6b3cc1d commit 11e5232

File tree

13 files changed

+110
-88
lines changed

13 files changed

+110
-88
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ default <R> Stream<R> getSecondaryResourcesAsStream(Class<R> expectedType) {
5858

5959
KubernetesClient getClient();
6060

61-
KubernetesClientFacade<P> getClientFacade();
61+
ResourceOperations<P> resourceOperations();
6262

6363
/** ExecutorService initialized by framework for workflows. Used for workflow standalone mode. */
6464
ExecutorService getWorkflowExecutorService();

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class DefaultContext<P extends HasMetadata> implements Context<P> {
4646
private final boolean primaryResourceDeleted;
4747
private final boolean primaryResourceFinalStateUnknown;
4848
private final Map<DependentResource<?, P>, Object> desiredStates = new ConcurrentHashMap<>();
49-
private final KubernetesClientFacade<P> clientFacade;
49+
private final ResourceOperations<P> resourceOperations;
5050

5151
public DefaultContext(
5252
RetryInfo retryInfo,
@@ -62,7 +62,7 @@ public DefaultContext(
6262
this.primaryResourceFinalStateUnknown = primaryResourceFinalStateUnknown;
6363
this.defaultManagedDependentResourceContext =
6464
new DefaultManagedWorkflowAndDependentResourceContext<>(controller, primaryResource, this);
65-
this.clientFacade = new KubernetesClientFacade<>(this);
65+
this.resourceOperations = new ResourceOperations<>(this);
6666
}
6767

6868
@Override
@@ -127,8 +127,8 @@ public KubernetesClient getClient() {
127127
}
128128

129129
@Override
130-
public KubernetesClientFacade<P> getClientFacade() {
131-
return null;
130+
public ResourceOperations<P> resourceOperations() {
131+
return resourceOperations;
132132
}
133133

134134
@Override

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/KubernetesClientFacade.java renamed to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
116
package io.javaoperatorsdk.operator.api.reconciler;
217

318
import java.lang.reflect.InvocationTargetException;
@@ -20,15 +35,15 @@
2035
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID;
2136
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion;
2237

23-
public class KubernetesClientFacade<P extends HasMetadata> {
38+
public class ResourceOperations<P extends HasMetadata> {
2439

2540
public static final int DEFAULT_MAX_RETRY = 10;
2641

27-
private static final Logger log = LoggerFactory.getLogger(KubernetesClientFacade.class);
42+
private static final Logger log = LoggerFactory.getLogger(ResourceOperations.class);
2843

2944
private final Context<P> context;
3045

31-
public KubernetesClientFacade(Context<P> context) {
46+
public ResourceOperations(Context<P> context) {
3247
this.context = context;
3348
}
3449

@@ -301,7 +316,10 @@ public <R extends HasMetadata> R jsonMergePatchPrimary(R resource) {
301316
* @see #jsonMergePatchPrimaryStatus(HasMetadata)
302317
*/
303318
public <R extends HasMetadata> R jsonMergePatchPrimaryStatus(R resource) {
304-
return jsonMergePatchPrimaryStatus(resource);
319+
return resourcePatch(
320+
resource,
321+
r -> context.getClient().resource(r).patchStatus(),
322+
context.eventSourceRetriever().getControllerEventSource());
305323
}
306324

307325
/**
@@ -401,8 +419,8 @@ public P removeFinalizer() {
401419
}
402420

403421
/**
404-
* Removes the target finalizer from target resource. Uses JSON Patch and handles retries, see
405-
* {@link PrimaryUpdateAndCacheUtils#conflictRetryingPatch(KubernetesClient, HasMetadata,
422+
* Removes the target finalizer from the primary resource. Uses JSON Patch and handles retries,
423+
* see {@link PrimaryUpdateAndCacheUtils#conflictRetryingPatch(KubernetesClient, HasMetadata,
406424
* UnaryOperator, Predicate)} for details. It does not try to remove finalizer if finalizer is not
407425
* present on the resource.
408426
*
@@ -430,7 +448,7 @@ public P removeFinalizer(String finalizerName) {
430448
/**
431449
* Patches the resource using JSON Patch. In case the server responds with conflict (HTTP 409) or
432450
* unprocessable content (HTTP 422) it retries the operation up to the maximum number defined in
433-
* {@link KubernetesClientFacade#DEFAULT_MAX_RETRY}.
451+
* {@link ResourceOperations#DEFAULT_MAX_RETRY}.
434452
*
435453
* @param resourceChangesOperator changes to be done on the resource before update
436454
* @param preCondition condition to check if the patch operation still needs to be performed or

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,14 @@ public ReconciliationDispatcher(Controller<P> controller) {
7272
public PostExecutionControl<P> handleExecution(ExecutionScope<P> executionScope) {
7373
validateExecutionScope(executionScope);
7474
try {
75-
return handleDispatch(executionScope);
75+
return handleDispatch(executionScope, null);
7676
} catch (Exception e) {
7777
return PostExecutionControl.exceptionDuringExecution(e);
7878
}
7979
}
8080

81-
private PostExecutionControl<P> handleDispatch(ExecutionScope<P> executionScope)
81+
// visible for testing
82+
PostExecutionControl<P> handleDispatch(ExecutionScope<P> executionScope, Context<P> context)
8283
throws Exception {
8384
P originalResource = executionScope.getResource();
8485
var resourceForExecution = cloneResource(originalResource);
@@ -97,13 +98,15 @@ && shouldNotDispatchToCleanupWhenMarkedForDeletion(originalResource)) {
9798
originalResource.getMetadata().getFinalizers());
9899
return PostExecutionControl.defaultDispatch();
99100
}
100-
Context<P> context =
101-
new DefaultContext<>(
102-
executionScope.getRetryInfo(),
103-
controller,
104-
resourceForExecution,
105-
executionScope.isDeleteEvent(),
106-
executionScope.isDeleteFinalStateUnknown());
101+
context =
102+
context == null
103+
? new DefaultContext<>(
104+
executionScope.getRetryInfo(),
105+
controller,
106+
resourceForExecution,
107+
executionScope.isDeleteEvent(),
108+
executionScope.isDeleteFinalStateUnknown())
109+
: context;
107110

108111
// checking the cleaner for all-event-mode
109112
if (!triggerOnAllEvents() && markedForDeletion) {
@@ -136,9 +139,9 @@ private PostExecutionControl<P> handleReconcile(
136139
*/
137140
P updatedResource;
138141
if (useSSA) {
139-
updatedResource = context.getClientFacade().addFinalizerWithSSA();
142+
updatedResource = context.resourceOperations().addFinalizerWithSSA();
140143
} else {
141-
updatedResource = context.getClientFacade().addFinalizer();
144+
updatedResource = context.resourceOperations().addFinalizer();
142145
}
143146
return PostExecutionControl.onlyFinalizerAdded(updatedResource)
144147
.withReSchedule(BaseControl.INSTANT_RESCHEDULE);
@@ -320,7 +323,7 @@ private PostExecutionControl<P> handleCleanup(
320323
// cleanup is finished, nothing left to be done
321324
final var finalizerName = configuration().getFinalizerName();
322325
if (deleteControl.isRemoveFinalizer() && resourceForExecution.hasFinalizer(finalizerName)) {
323-
P customResource = context.getClientFacade().removeFinalizer();
326+
P customResource = context.resourceOperations().removeFinalizer();
324327
return PostExecutionControl.customResourceFinalizerRemoved(customResource);
325328
}
326329
}
@@ -386,9 +389,9 @@ public R patchResource(Context<R> context, R resource, R originalResource) {
386389
resource.getMetadata().getResourceVersion());
387390
}
388391
if (useSSA) {
389-
return context.getClientFacade().serverSideApplyPrimary(resource);
392+
return context.resourceOperations().serverSideApplyPrimary(resource);
390393
} else {
391-
return context.getClientFacade().jsonPatchPrimary(originalResource, r -> resource);
394+
return context.resourceOperations().jsonPatchPrimary(originalResource, r -> resource);
392395
}
393396
}
394397

@@ -398,7 +401,7 @@ public R patchStatus(Context<R> context, R resource, R originalResource) {
398401
var managedFields = resource.getMetadata().getManagedFields();
399402
try {
400403
resource.getMetadata().setManagedFields(null);
401-
return context.getClientFacade().serverSideApplyPrimaryStatus(resource);
404+
return context.resourceOperations().serverSideApplyPrimaryStatus(resource);
402405
} finally {
403406
resource.getMetadata().setManagedFields(managedFields);
404407
}
@@ -416,7 +419,7 @@ private R editStatus(Context<R> context, R resource, R originalResource) {
416419
clonedOriginal.getMetadata().setResourceVersion(null);
417420
resource.getMetadata().setResourceVersion(null);
418421
return context
419-
.getClientFacade()
422+
.resourceOperations()
420423
.jsonPatchPrimaryStatus(
421424
clonedOriginal,
422425
r -> {

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java‎

Lines changed: 46 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.junit.jupiter.api.Test;
2727
import org.mockito.ArgumentCaptor;
2828
import org.mockito.ArgumentMatchers;
29-
import org.mockito.MockedStatic;
3029

3130
import io.fabric8.kubernetes.api.model.HasMetadata;
3231
import io.fabric8.kubernetes.api.model.ObjectMeta;
@@ -45,8 +44,8 @@
4544
import io.javaoperatorsdk.operator.api.reconciler.DefaultContext;
4645
import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
4746
import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl;
48-
import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils;
4947
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
48+
import io.javaoperatorsdk.operator.api.reconciler.ResourceOperations;
5049
import io.javaoperatorsdk.operator.api.reconciler.RetryInfo;
5150
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
5251
import io.javaoperatorsdk.operator.processing.Controller;
@@ -72,6 +71,7 @@ class ReconciliationDispatcherTest {
7271
private final CustomResourceFacade<TestCustomResource> customResourceFacade =
7372
mock(ReconciliationDispatcher.CustomResourceFacade.class);
7473
private static ConfigurationService configurationService;
74+
private ResourceOperations mockResourceOperations;
7575

7676
@BeforeEach
7777
void setup() {
@@ -151,27 +151,25 @@ public boolean useFinalizer() {
151151
}
152152

153153
@Test
154-
void addFinalizerOnNewResource() {
155-
try (MockedStatic<ReconcileUtils> mockedReconcileUtils = mockStatic(ReconcileUtils.class)) {
156-
assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER));
157-
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
158-
verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any());
159-
mockedReconcileUtils.verify(() -> ReconcileUtils.addFinalizerWithSSA(any()), times(1));
160-
}
154+
void addFinalizerOnNewResource() throws Exception {
155+
assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER));
156+
reconciliationDispatcher.handleDispatch(
157+
executionScopeWithCREvent(testCustomResource), createTestContext());
158+
verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any());
159+
verify(mockResourceOperations, times(1)).addFinalizerWithSSA();
161160
}
162161

163162
@Test
164-
void addFinalizerOnNewResourceWithoutSSA() {
165-
try (MockedStatic<ReconcileUtils> mockedReconcileUtils = mockStatic(ReconcileUtils.class)) {
166-
initConfigService(false, false);
167-
final ReconciliationDispatcher<TestCustomResource> dispatcher =
168-
init(testCustomResource, reconciler, null, customResourceFacade, true);
163+
void addFinalizerOnNewResourceWithoutSSA() throws Exception {
164+
initConfigService(false, false);
165+
final ReconciliationDispatcher<TestCustomResource> dispatcher =
166+
init(testCustomResource, reconciler, null, customResourceFacade, true);
167+
assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER));
169168

170-
assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER));
171-
dispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
172-
verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any());
173-
mockedReconcileUtils.verify(() -> ReconcileUtils.addFinalizer(any()), times(1));
174-
}
169+
dispatcher.handleDispatch(executionScopeWithCREvent(testCustomResource), createTestContext());
170+
171+
verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any());
172+
verify(mockResourceOperations, times(1)).addFinalizer();
175173
}
176174

177175
@Test
@@ -227,17 +225,16 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() {
227225
}
228226

229227
@Test
230-
void removesDefaultFinalizerOnDeleteIfSet() {
231-
try (MockedStatic<ReconcileUtils> mockedReconcileUtils = mockStatic(ReconcileUtils.class)) {
232-
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
233-
markForDeletion(testCustomResource);
228+
void removesDefaultFinalizerOnDeleteIfSet() throws Exception {
229+
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
230+
markForDeletion(testCustomResource);
234231

235-
var postExecControl =
236-
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
232+
var postExecControl =
233+
reconciliationDispatcher.handleDispatch(
234+
executionScopeWithCREvent(testCustomResource), createTestContext());
237235

238-
assertThat(postExecControl.isFinalizerRemoved()).isTrue();
239-
mockedReconcileUtils.verify(() -> ReconcileUtils.removeFinalizer(any()), times(1));
240-
}
236+
assertThat(postExecControl.isFinalizerRemoved()).isTrue();
237+
verify(mockResourceOperations, times(1)).removeFinalizer();
241238
}
242239

243240
@Test
@@ -295,20 +292,21 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() {
295292
}
296293

297294
@Test
298-
void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() {
299-
try (MockedStatic<ReconcileUtils> mockedReconcileUtils = mockStatic(ReconcileUtils.class)) {
300-
removeFinalizers(testCustomResource);
301-
reconciler.reconcile = (r, c) -> UpdateControl.noUpdate();
302-
mockedReconcileUtils
303-
.when(() -> ReconcileUtils.addFinalizerWithSSA(any()))
304-
.thenReturn(testCustomResource);
305-
var postExecControl =
306-
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
295+
void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() throws Exception {
307296

308-
mockedReconcileUtils.verify(() -> ReconcileUtils.addFinalizerWithSSA(any()), times(1));
309-
assertThat(postExecControl.updateIsStatusPatch()).isFalse();
310-
assertThat(postExecControl.getUpdatedCustomResource()).isPresent();
311-
}
297+
removeFinalizers(testCustomResource);
298+
reconciler.reconcile = (r, c) -> UpdateControl.noUpdate();
299+
var context = createTestContext();
300+
when(mockResourceOperations.addFinalizerWithSSA()).thenReturn(testCustomResource);
301+
302+
var postExecControl =
303+
reconciliationDispatcher.handleDispatch(
304+
executionScopeWithCREvent(testCustomResource), context);
305+
306+
verify(mockResourceOperations, times(1)).addFinalizerWithSSA();
307+
308+
assertThat(postExecControl.updateIsStatusPatch()).isFalse();
309+
assertThat(postExecControl.getUpdatedCustomResource()).isPresent();
312310
}
313311

314312
@Test
@@ -646,6 +644,13 @@ void reconcilerContextUsesTheSameInstanceOfResourceAsParam() {
646644
.isNotSameAs(testCustomResource);
647645
}
648646

647+
private Context<TestCustomResource> createTestContext() {
648+
var mockContext = mock(Context.class);
649+
mockResourceOperations = mock(ResourceOperations.class);
650+
when(mockContext.resourceOperations()).thenReturn(mockResourceOperations);
651+
return mockContext;
652+
}
653+
649654
private ObservedGenCustomResource createObservedGenCustomResource() {
650655
ObservedGenCustomResource observedGenCustomResource = new ObservedGenCustomResource();
651656
observedGenCustomResource.setMetadata(new ObjectMeta());

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/changenamespace/ChangeNamespaceTestReconciler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public UpdateControl<ChangeNamespaceTestCustomResource> reconcile(
5353
ChangeNamespaceTestCustomResource primary,
5454
Context<ChangeNamespaceTestCustomResource> context) {
5555

56-
ReconcileUtils.serverSideApply(context, configMap(primary));
56+
context.resourceOperations().serverSideApply(configMap(primary));
5757

5858
if (primary.getStatus() == null) {
5959
primary.setStatus(new ChangeNamespaceTestCustomResourceStatus());

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/generickubernetesresourcehandling/GenericKubernetesResourceHandlingReconciler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public UpdateControl<GenericKubernetesResourceHandlingCustomResource> reconcile(
4040
GenericKubernetesResourceHandlingCustomResource primary,
4141
Context<GenericKubernetesResourceHandlingCustomResource> context) {
4242

43-
ReconcileUtils.serverSideApply(context, desiredConfigMap(primary, context));
43+
context.resourceOperations().serverSideApply(desiredConfigMap(primary, context));
4444

4545
return UpdateControl.noUpdate();
4646
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/multiplesecondaryeventsource/MultipleSecondaryEventSourceReconciler.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import io.javaoperatorsdk.operator.api.reconciler.Context;
2727
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
2828
import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
29-
import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils;
3029
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
3130
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
3231
import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -54,8 +53,8 @@ public UpdateControl<MultipleSecondaryEventSourceCustomResource> reconcile(
5453
Context<MultipleSecondaryEventSourceCustomResource> context) {
5554
numberOfExecutions.addAndGet(1);
5655

57-
ReconcileUtils.serverSideApply(context, configMap(getName1(resource), resource));
58-
ReconcileUtils.serverSideApply(context, configMap(getName2(resource), resource));
56+
context.resourceOperations().serverSideApply(configMap(getName1(resource), resource));
57+
context.resourceOperations().serverSideApply(configMap(getName2(resource), resource));
5958

6059
if (numberOfExecutions.get() >= 3) {
6160
if (context.getSecondaryResources(ConfigMap.class).size() != 2) {

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/TestReconciler.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public UpdateControl<TestCustomResource> reconcile(
6969
if (existingConfigMap != null) {
7070
existingConfigMap.setData(configMapData(resource));
7171
log.info("Updating config map");
72-
ReconcileUtils.serverSideApply(context, existingConfigMap);
72+
context.resourceOperations().serverSideApply(existingConfigMap);
7373
} else {
7474
Map<String, String> labels = new HashMap<>();
7575
labels.put("managedBy", TestReconciler.class.getSimpleName());
@@ -84,7 +84,7 @@ public UpdateControl<TestCustomResource> reconcile(
8484
.withData(configMapData(resource))
8585
.build();
8686
log.info("Creating config map");
87-
ReconcileUtils.serverSideApply(context, newConfigMap);
87+
context.resourceOperations().serverSideApply(newConfigMap);
8888
}
8989
if (updateStatus) {
9090
var statusUpdateResource = new TestCustomResource();

0 commit comments

Comments
 (0)