Skip to content

fix for issues, when we are having different default namespace then custom resource namespace #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 13, 2020
Merged

fix for issues, when we are having different default namespace then custom resource namespace #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions operator-framework/src/main/java/com/github/containersolutions/operator/Operator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ private <R extends CustomResource> void registerController(ResourceController<R>
Class<? extends CustomResourceList<R>> list = getCustomResourceListClass(controller);
Class<? extends CustomResourceDoneable<R>> doneable = getCustomResourceDonebaleClass(controller);
MixedOperation client = k8sClient.customResources(crd, resClass, list, doneable);

EventDispatcher eventDispatcher = new EventDispatcher(controller, (CustomResourceOperationsImpl) client,
getDefaultFinalizer(controller));
EventDispatcher eventDispatcher = new EventDispatcher(controller,
getDefaultFinalizer(controller), new EventDispatcher.CustomResourceReplaceFacade(client));
EventScheduler eventScheduler = new EventScheduler(eventDispatcher, retry);
registerWatches(controller, client, resClass, watchAllNamespaces, targetNamespaces, eventScheduler);
}
Expand Down
33 changes: 25 additions & 8 deletions ...work/src/main/java/com/github/containersolutions/operator/processing/EventDispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import com.github.containersolutions.operator.api.ResourceController;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.Watcher;
import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationsImpl;
import io.fabric8.kubernetes.client.dsl.MixedOperation;
import io.fabric8.kubernetes.client.dsl.Resource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -18,15 +19,14 @@ public class EventDispatcher {
private final static Logger log = LoggerFactory.getLogger(EventDispatcher.class);

private final ResourceController controller;
private final CustomResourceOperationsImpl resourceOperation;
private final String resourceDefaultFinalizer;
private final CustomResourceReplaceFacade customResourceReplaceFacade;

public EventDispatcher(ResourceController controller,
CustomResourceOperationsImpl resourceOperation,
String defaultFinalizer
) {
String defaultFinalizer,
CustomResourceReplaceFacade customResourceReplaceFacade) {
this.controller = controller;
this.resourceOperation = resourceOperation;
this.customResourceReplaceFacade = customResourceReplaceFacade;
this.resourceDefaultFinalizer = defaultFinalizer;
}

Expand Down Expand Up @@ -77,12 +77,12 @@ private boolean hasDefaultFinalizer(CustomResource resource) {
private void removeDefaultFinalizer(CustomResource resource) {
resource.getMetadata().getFinalizers().remove(resourceDefaultFinalizer);
log.debug("Removed finalizer. Trying to replace resource {}, version: {}", resource.getMetadata().getName(), resource.getMetadata().getResourceVersion());
resourceOperation.lockResourceVersion(resource.getMetadata().getResourceVersion()).replace(resource);
customResourceReplaceFacade.replaceWithLock(resource);
}

private void replace(CustomResource resource) {
log.debug("Trying to replace resource {}, version: {}", resource.getMetadata().getName(), resource.getMetadata().getResourceVersion());
resourceOperation.lockResourceVersion(resource.getMetadata().getResourceVersion()).replace(resource);
customResourceReplaceFacade.replaceWithLock(resource);
}

private void addFinalizerIfNotPresent(CustomResource resource) {
Expand All @@ -98,4 +98,21 @@ private void addFinalizerIfNotPresent(CustomResource resource) {
private boolean markedForDeletion(CustomResource resource) {
return resource.getMetadata().getDeletionTimestamp() != null && !resource.getMetadata().getDeletionTimestamp().isEmpty();
}

// created to support unit testing
public static class CustomResourceReplaceFacade {

private final MixedOperation<?, ?, ?, Resource<CustomResource, ?>> resourceOperation;

public CustomResourceReplaceFacade(MixedOperation<?, ?, ?, Resource<CustomResource, ?>> resourceOperation) {
this.resourceOperation = resourceOperation;
}

public CustomResource replaceWithLock(CustomResource resource) {
return resourceOperation.inNamespace(resource.getMetadata().getNamespace())
.withName(resource.getMetadata().getName())
.lockResourceVersion(resource.getMetadata().getResourceVersion())
.replace(resource);
}
}
}
25 changes: 10 additions & 15 deletions ...r-framework/src/test/java/com/github/containersolutions/operator/EventDispatcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@
import com.github.containersolutions.operator.sample.TestCustomResource;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.CustomResourceDoneable;
import io.fabric8.kubernetes.client.CustomResourceList;
import io.fabric8.kubernetes.client.Watcher;
import io.fabric8.kubernetes.client.dsl.Replaceable;
import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationsImpl;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentMatchers;
Expand All @@ -27,19 +23,19 @@ class EventDispatcherTest {
private CustomResource testCustomResource;
private EventDispatcher eventDispatcher;
private ResourceController<CustomResource> resourceController = mock(ResourceController.class);
private CustomResourceOperationsImpl<CustomResource, CustomResourceList<CustomResource>,
CustomResourceDoneable<CustomResource>> resourceOperation = mock(CustomResourceOperationsImpl.class);
private EventDispatcher.CustomResourceReplaceFacade customResourceReplaceFacade = mock(EventDispatcher.CustomResourceReplaceFacade.class);


@BeforeEach
void setup() {
eventDispatcher = new EventDispatcher(resourceController, resourceOperation,
Controller.DEFAULT_FINALIZER);
eventDispatcher = new EventDispatcher(resourceController,
Controller.DEFAULT_FINALIZER, customResourceReplaceFacade);

testCustomResource = getResource();

when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.of(testCustomResource));
when(resourceController.deleteResource(eq(testCustomResource))).thenReturn(true);
when(resourceOperation.lockResourceVersion(any())).thenReturn(mock(Replaceable.class));
when(customResourceReplaceFacade.replaceWithLock(any())).thenReturn(null);
}

@Test
Expand Down Expand Up @@ -92,7 +88,7 @@ void removesDefaultFinalizerOnDelete() {
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);

assertEquals(0, testCustomResource.getMetadata().getFinalizers().size());
verify(resourceOperation, times(1)).lockResourceVersion(any());
verify(customResourceReplaceFacade, times(1)).replaceWithLock(any());
}

@Test
Expand All @@ -104,7 +100,7 @@ void doesNotRemovesTheFinalizerIfTheDeleteMethodRemovesFalse() {
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);

assertEquals(1, testCustomResource.getMetadata().getFinalizers().size());
verify(resourceOperation, never()).lockResourceVersion(any());
verify(customResourceReplaceFacade, never()).replaceWithLock(any());
}

@Test
Expand All @@ -113,8 +109,7 @@ void doesNotUpdateTheResourceIfEmptyOptionalReturned() {
when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.empty());

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

verify(resourceOperation, never()).lockResourceVersion(any());
verify(customResourceReplaceFacade, never()).replaceWithLock(any());
}

@Test
Expand All @@ -124,7 +119,7 @@ void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() {
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);

assertEquals(1, testCustomResource.getMetadata().getFinalizers().size());
verify(resourceOperation, times(1)).lockResourceVersion(any());
verify(customResourceReplaceFacade, times(1)).replaceWithLock(any());
}

@Test
Expand All @@ -135,7 +130,7 @@ void doesNotAddFinalizerIfOptionalIsReturnedButMarkedForDeletion() {
eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);

assertEquals(0, testCustomResource.getMetadata().getFinalizers().size());
verify(resourceOperation, never()).lockResourceVersion(any());
verify(customResourceReplaceFacade, never()).replaceWithLock(any());
}

private void markForDeletion(CustomResource customResource) {
Expand Down