Skip to content

Commit 6a445c0

Browse files
csviricsviri
committed
removed context object from controller api
1 parent 94232ca commit 6a445c0

File tree

15 files changed

+91
-198
lines changed

15 files changed

+91
-198
lines changed

‎operator-framework/src/main/java/com/github/containersolutions/operator/Operator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ private <R extends CustomResource> void registerController(ResourceController<R>
5757
MixedOperation client = k8sClient.customResources(crd.get(), resClass, list, doneable);
5858

5959
EventDispatcher<R> eventDispatcher =
60-
new EventDispatcher<>(controller, (CustomResourceOperationsImpl) client, client, k8sClient,
60+
new EventDispatcher<>(controller, (CustomResourceOperationsImpl) client,
6161
ControllerUtils.getDefaultFinalizer(controller));
6262

6363
eventScheduler = new EventScheduler(eventDispatcher);

‎operator-framework/src/main/java/com/github/containersolutions/operator/api/Context.java‎

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎operator-framework/src/main/java/com/github/containersolutions/operator/api/ResourceController.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,20 @@ public interface ResourceController<R extends CustomResource> {
1212
* unless the return value is false - note that this is almost never the case.
1313
*
1414
* @param resource
15-
* @param context
1615
* @return true - so the finalizer is automatically removed after the call.
1716
* false if you don't want to remove the finalizer. Note that this is ALMOST NEVER the case.
1817
*/
19-
boolean deleteResource(R resource, Context<R> context);
18+
boolean deleteResource(R resource);
2019

2120
/**
2221
* The implementation of this operation is required to be idempotent.
2322
*
2423
* @return The resource is updated in api server if the return value is present
25-
* within Optional. This the common use cases. However in cases, for example the operator is restarted,
26-
* and we don't want to have an update call to k8s api to be made unnecessarily, by returning an empty Optional
27-
* this update can be skipped.
28-
* <b>However we will always call an update if there is no finalizer on object and its not marked for deletion.</b>
24+
* within Optional. This the common use cases. However in cases, for example the operator is restarted,
25+
* and we don't want to have an update call to k8s api to be made unnecessarily, by returning an empty Optional
26+
* this update can be skipped.
27+
* <b>However we will always call an update if there is no finalizer on object and its not marked for deletion.</b>
2928
*/
30-
Optional<R> createOrUpdateResource(R resource, Context<R> context);
29+
Optional<R> createOrUpdateResource(R resource);
3130

3231
}

‎operator-framework/src/main/java/com/github/containersolutions/operator/api/ResourceControllerAdapter.java‎

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎operator-framework/src/main/java/com/github/containersolutions/operator/processing/EventDispatcher.java‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package com.github.containersolutions.operator.processing;
22

3-
import com.github.containersolutions.operator.api.Context;
43
import com.github.containersolutions.operator.api.ResourceController;
5-
import io.fabric8.kubernetes.client.*;
6-
import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation;
7-
import io.fabric8.kubernetes.client.dsl.Resource;
4+
import io.fabric8.kubernetes.client.CustomResource;
5+
import io.fabric8.kubernetes.client.CustomResourceDoneable;
6+
import io.fabric8.kubernetes.client.CustomResourceList;
7+
import io.fabric8.kubernetes.client.Watcher;
88
import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationsImpl;
99
import org.slf4j.Logger;
1010
import org.slf4j.LoggerFactory;
@@ -22,22 +22,14 @@ public class EventDispatcher<R extends CustomResource> {
2222
private final ResourceController<R> controller;
2323
private final CustomResourceOperationsImpl<R, CustomResourceList<R>, CustomResourceDoneable<R>> resourceOperation;
2424
private final String resourceDefaultFinalizer;
25-
private final NonNamespaceOperation<R, CustomResourceList<R>, CustomResourceDoneable<R>,
26-
Resource<R, CustomResourceDoneable<R>>> resourceClient;
27-
private final KubernetesClient k8sClient;
2825

2926
public EventDispatcher(ResourceController<R> controller,
3027
CustomResourceOperationsImpl<R, CustomResourceList<R>, CustomResourceDoneable<R>> resourceOperation,
31-
NonNamespaceOperation<R, CustomResourceList<R>, CustomResourceDoneable<R>,
32-
Resource<R, CustomResourceDoneable<R>>> resourceClient, KubernetesClient k8sClient,
3328
String defaultFinalizer
34-
3529
) {
3630
this.controller = controller;
3731
this.resourceOperation = resourceOperation;
38-
this.resourceClient = resourceClient;
3932
this.resourceDefaultFinalizer = defaultFinalizer;
40-
this.k8sClient = k8sClient;
4133
}
4234

4335
public void handleEvent(Watcher.Action action, R resource) {
@@ -49,13 +41,13 @@ public void handleEvent(Watcher.Action action, R resource) {
4941
// finalizer into the resource before marked for delete. So for now we will call delete every time, since delete
5042
// operation should be idempotent too, and this way we cover the corner case.
5143
if (markedForDeletion(resource)) {
52-
boolean removeFinalizer = controller.deleteResource(resource, new Context(k8sClient, resourceClient));
44+
boolean removeFinalizer = controller.deleteResource(resource);
5345
if (removeFinalizer && hasDefaultFinalizer(resource)) {
5446
log.debug("Removing finalizer on {}: {}", resource.getMetadata().getName(), resource.getMetadata());
5547
removeDefaultFinalizer(resource);
5648
}
5749
} else {
58-
Optional<R> updateResult = controller.createOrUpdateResource(resource, new Context<>(k8sClient, resourceClient));
50+
Optional<R> updateResult = controller.createOrUpdateResource(resource);
5951
if (updateResult.isPresent()) {
6052
log.debug("Updating resource: {} with version: {}", resource.getMetadata().getName(),
6153
resource.getMetadata().getResourceVersion());

‎operator-framework/src/test/java/com/github/containersolutions/operator/ControllerUtilsTest.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ class ControllerUtilsTest {
1616

1717
@Test
1818
public void returnsValuesFromControllerAnnotationFinalizer() {
19-
assertEquals(DEFAULT_FINALIZER, ControllerUtils.getDefaultFinalizer(new TestCustomResourceController()));
20-
assertEquals(TEST_GROUP + "/" + DEFAULT_VERSION, ControllerUtils.getApiVersion(new TestCustomResourceController()));
21-
assertEquals(DEFAULT_VERSION, ControllerUtils.getVersion(new TestCustomResourceController()));
22-
assertEquals(TestCustomResourceController.KIND_NAME, ControllerUtils.getKind(new TestCustomResourceController()));
23-
assertEquals(TestCustomResource.class, ControllerUtils.getCustomResourceClass(new TestCustomResourceController()));
24-
assertEquals(TestCustomResourceDoneable.class, ControllerUtils.getCustomResourceDonebaleClass(new TestCustomResourceController()));
25-
assertEquals(TestCustomResourceList.class, ControllerUtils.getCustomResourceListClass(new TestCustomResourceController()));
26-
assertEquals(CRD_NAME, ControllerUtils.getCrdName(new TestCustomResourceController()).get());
19+
assertEquals(DEFAULT_FINALIZER, ControllerUtils.getDefaultFinalizer(new TestCustomResourceController(null)));
20+
assertEquals(TEST_GROUP + "/" + DEFAULT_VERSION, ControllerUtils.getApiVersion(new TestCustomResourceController(null)));
21+
assertEquals(DEFAULT_VERSION, ControllerUtils.getVersion(new TestCustomResourceController(null)));
22+
assertEquals(TestCustomResourceController.KIND_NAME, ControllerUtils.getKind(new TestCustomResourceController(null)));
23+
assertEquals(TestCustomResource.class, ControllerUtils.getCustomResourceClass(new TestCustomResourceController(null)));
24+
assertEquals(TestCustomResourceDoneable.class, ControllerUtils.getCustomResourceDonebaleClass(new TestCustomResourceController(null)));
25+
assertEquals(TestCustomResourceList.class, ControllerUtils.getCustomResourceListClass(new TestCustomResourceController(null)));
26+
assertEquals(CRD_NAME, ControllerUtils.getCrdName(new TestCustomResourceController(null)).get());
2727
}
2828

2929
}

‎operator-framework/src/test/java/com/github/containersolutions/operator/EventConsumerTest.java‎

Lines changed: 0 additions & 38 deletions
This file was deleted.

‎operator-framework/src/test/java/com/github/containersolutions/operator/EventDispatcherTest.java‎

Lines changed: 16 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
import com.github.containersolutions.operator.processing.EventDispatcher;
77
import com.github.containersolutions.operator.sample.TestCustomResource;
88
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
9-
import io.fabric8.kubernetes.client.*;
10-
import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation;
9+
import io.fabric8.kubernetes.client.CustomResource;
10+
import io.fabric8.kubernetes.client.CustomResourceDoneable;
11+
import io.fabric8.kubernetes.client.CustomResourceList;
12+
import io.fabric8.kubernetes.client.Watcher;
1113
import io.fabric8.kubernetes.client.dsl.Replaceable;
12-
import io.fabric8.kubernetes.client.dsl.Resource;
1314
import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationsImpl;
14-
import io.fabric8.kubernetes.client.dsl.internal.RawCustomResourceOperationsImpl;
1515
import org.junit.jupiter.api.BeforeEach;
1616
import org.junit.jupiter.api.Test;
1717
import org.mockito.ArgumentMatchers;
@@ -27,49 +27,39 @@ class EventDispatcherTest {
2727
private CustomResource testCustomResource;
2828
private EventDispatcher<CustomResource> eventDispatcher;
2929
private ResourceController<CustomResource> resourceController = mock(ResourceController.class);
30-
private NonNamespaceOperation<CustomResource, CustomResourceList<CustomResource>,
31-
CustomResourceDoneable<CustomResource>,
32-
Resource<CustomResource, CustomResourceDoneable<CustomResource>>>
33-
operation = mock(NonNamespaceOperation.class);
34-
private KubernetesClient k8sClient = mock(KubernetesClient.class);
3530
private CustomResourceOperationsImpl<CustomResource, CustomResourceList<CustomResource>,
3631
CustomResourceDoneable<CustomResource>> resourceOperation = mock(CustomResourceOperationsImpl.class);
3732

38-
private RawCustomResourceOperationsImpl rawResourceOperations = mock(RawCustomResourceOperationsImpl.class);
3933
@BeforeEach
4034
void setup() {
41-
eventDispatcher = new EventDispatcher(resourceController, resourceOperation, operation, k8sClient,
35+
eventDispatcher = new EventDispatcher(resourceController, resourceOperation,
4236
Controller.DEFAULT_FINALIZER);
4337

4438
testCustomResource = getResource();
4539

46-
when(resourceController.createOrUpdateResource(eq(testCustomResource), any())).thenReturn(Optional.of(testCustomResource));
47-
when(resourceController.deleteResource(eq(testCustomResource), any())).thenReturn(true);
40+
when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.of(testCustomResource));
41+
when(resourceController.deleteResource(eq(testCustomResource))).thenReturn(true);
4842
when(resourceOperation.lockResourceVersion(any())).thenReturn(mock(Replaceable.class));
49-
50-
// K8s client mocking
51-
when(k8sClient.customResource(any())).thenReturn(rawResourceOperations);
52-
when(rawResourceOperations.get(any(), any())).thenReturn(getRawResource());
5343
}
5444

5545
@Test
5646
void callCreateOrUpdateOnNewResource() {
5747
eventDispatcher.handleEvent(Watcher.Action.ADDED, testCustomResource);
58-
verify(resourceController, times(1)).createOrUpdateResource(ArgumentMatchers.eq(testCustomResource), any());
48+
verify(resourceController, times(1)).createOrUpdateResource(ArgumentMatchers.eq(testCustomResource));
5949
}
6050

6151
@Test
6252
void callCreateOrUpdateOnModifiedResource() {
6353
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
64-
verify(resourceController, times(1)).createOrUpdateResource(ArgumentMatchers.eq(testCustomResource), any());
54+
verify(resourceController, times(1)).createOrUpdateResource(ArgumentMatchers.eq(testCustomResource));
6555
}
6656

6757
@Test
6858
void adsDefaultFinalizerOnCreateIfNotThere() {
6959
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
7060
verify(resourceController, times(1))
7161
.createOrUpdateResource(argThat(testCustomResource ->
72-
testCustomResource.getMetadata().getFinalizers().contains(Controller.DEFAULT_FINALIZER)), any());
62+
testCustomResource.getMetadata().getFinalizers().contains(Controller.DEFAULT_FINALIZER)));
7363
}
7464

7565
@Test
@@ -79,7 +69,7 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() {
7969

8070
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
8171

82-
verify(resourceController, times(1)).deleteResource(eq(testCustomResource), any());
72+
verify(resourceController, times(1)).deleteResource(eq(testCustomResource));
8373
}
8474

8575
/**
@@ -91,7 +81,7 @@ void callDeleteOnControllerIfMarkedForDeletionButThereIsNoDefaultFinalizer() {
9181

9282
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
9383

94-
verify(resourceController).deleteResource(eq(testCustomResource), any());
84+
verify(resourceController).deleteResource(eq(testCustomResource));
9585
}
9686

9787
@Test
@@ -107,7 +97,7 @@ void removesDefaultFinalizerOnDelete() {
10797

10898
@Test
10999
void doesNotRemovesTheFinalizerIfTheDeleteMethodRemovesFalse() {
110-
when(resourceController.deleteResource(eq(testCustomResource), any())).thenReturn(false);
100+
when(resourceController.deleteResource(eq(testCustomResource))).thenReturn(false);
111101
markForDeletion(testCustomResource);
112102
testCustomResource.getMetadata().getFinalizers().add(Controller.DEFAULT_FINALIZER);
113103

@@ -120,7 +110,7 @@ void doesNotRemovesTheFinalizerIfTheDeleteMethodRemovesFalse() {
120110
@Test
121111
void doesNotUpdateTheResourceIfEmptyOptionalReturned() {
122112
testCustomResource.getMetadata().getFinalizers().add(Controller.DEFAULT_FINALIZER);
123-
when(resourceController.createOrUpdateResource(eq(testCustomResource), any())).thenReturn(Optional.empty());
113+
when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.empty());
124114

125115
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
126116

@@ -129,7 +119,7 @@ void doesNotUpdateTheResourceIfEmptyOptionalReturned() {
129119

130120
@Test
131121
void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() {
132-
when(resourceController.createOrUpdateResource(eq(testCustomResource), any())).thenReturn(Optional.empty());
122+
when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.empty());
133123

134124
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
135125

@@ -140,26 +130,14 @@ void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() {
140130
@Test
141131
void doesNotAddFinalizerIfOptionalIsReturnedButMarkedForDeletion() {
142132
markForDeletion(testCustomResource);
143-
when(resourceController.createOrUpdateResource(eq(testCustomResource), any())).thenReturn(Optional.empty());
133+
when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.empty());
144134

145135
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
146136

147137
assertEquals(0, testCustomResource.getMetadata().getFinalizers().size());
148138
verify(resourceOperation, never()).lockResourceVersion(any());
149139
}
150140

151-
/**
152-
* When receiving a DELETE event the resource is already gone and we did the processing already on a MODIFIED
153-
* event received when then finalizer is added. The correct action on a DELETE event is a noop.
154-
*/
155-
@Test
156-
void doesntDoAnythingOnDeleteEvent() {
157-
eventDispatcher.handleEvent(Watcher.Action.DELETED, testCustomResource);
158-
159-
verify(k8sClient, never()).customResource(any());
160-
}
161-
162-
163141
private void markForDeletion(CustomResource customResource) {
164142
customResource.getMetadata().setDeletionTimestamp("2019-8-10");
165143
}

‎operator-framework/src/test/java/com/github/containersolutions/operator/IntegrationTestSupport.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public void initialize() {
4444
.withMetadata(new ObjectMetaBuilder().withName(TEST_NAMESPACE).build()).build());
4545
}
4646
operator = new Operator(k8sClient);
47-
operator.registerController(new TestCustomResourceController(), TEST_NAMESPACE);
47+
operator.registerController(new TestCustomResourceController(k8sClient), TEST_NAMESPACE);
4848
}
4949

5050
public void cleanup() {

0 commit comments

Comments
 (0)