From e0f3009b6301e21e25b58224d0b1f770ef522e55 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 19 Jul 2022 14:17:57 +0200 Subject: [PATCH 01/31] feat: leader election --- .../api/config/LeaderElectionConfiguration.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java new file mode 100644 index 0000000000..1c0ebec869 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java @@ -0,0 +1,13 @@ +package io.javaoperatorsdk.operator.api.config; + +public class LeaderElectionConfiguration { + + private String leaderElectionNamespaces; + private String leaderElectionID; + + // todo discuss + private boolean syncEventSources; + + // todo leader election with lease vs for life, other options: + // see: https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/manager#Options +} From 5a19f8696db419c61a011c4943940a3ac54c966d Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 21 Jul 2022 16:11:09 +0200 Subject: [PATCH 02/31] refactor: event processor to controller --- .../operator/ControllerManager.java | 79 +++++++++++++++++++ .../io/javaoperatorsdk/operator/Operator.java | 61 -------------- .../api/config/ConfigurationService.java | 6 ++ .../config/ConfigurationServiceOverrider.java | 12 +++ .../config/LeaderElectionConfiguration.java | 61 ++++++++++++-- .../LeaderElectionConfigurationBuilder.java | 50 ++++++++++++ .../operator/processing/Controller.java | 26 +++++- .../processing/event/EventProcessor.java | 4 +- .../processing/event/EventSourceManager.java | 48 +++++------ .../event/ReconciliationDispatcher.java | 76 +++++++++--------- .../operator/ControllerManagerTest.java | 7 +- .../event/EventSourceManagerTest.java | 2 +- 12 files changed, 290 insertions(+), 142 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java new file mode 100644 index 0000000000..ef62f4cf07 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java @@ -0,0 +1,79 @@ +package io.javaoperatorsdk.operator; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.javaoperatorsdk.operator.processing.Controller; +import io.javaoperatorsdk.operator.processing.LifecycleAware; + +/** + * Not confuse with controller manager form go operators. The high level aggregate is Operator in + * JOSDK. + */ +class ControllerManager implements LifecycleAware { + + private static final Logger log = LoggerFactory.getLogger(ControllerManager.class); + + private final Map controllers = new HashMap<>(); + private boolean started = false; + + public synchronized void shouldStart() { + if (started) { + return; + } + if (controllers.isEmpty()) { + throw new OperatorException("No Controller exists. Exiting!"); + } + } + + public synchronized void start() { + controllers().parallelStream().forEach(Controller::start); + started = true; + } + + public synchronized void stop() { + controllers().parallelStream().forEach(closeable -> { + log.debug("closing {}", closeable); + closeable.stop(); + }); + + started = false; + } + + @SuppressWarnings("unchecked") + synchronized void add(Controller controller) { + final var configuration = controller.getConfiguration(); + final var resourceTypeName = ReconcilerUtils + .getResourceTypeNameWithVersion(configuration.getResourceClass()); + final var existing = controllers.get(resourceTypeName); + if (existing != null) { + throw new OperatorException("Cannot register controller '" + configuration.getName() + + "': another controller named '" + existing.getConfiguration().getName() + + "' is already registered for resource '" + resourceTypeName + "'"); + } + controllers.put(resourceTypeName, controller); + if (started) { + controller.start(); + } + } + + synchronized Optional get(String name) { + return controllers().stream() + .filter(c -> name.equals(c.getConfiguration().getName())) + .findFirst(); + } + + synchronized Collection controllers() { + return controllers.values(); + } + + synchronized int size() { + return controllers.size(); + } +} + diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index a17e86d8cc..f403e8b05e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -1,9 +1,6 @@ package io.javaoperatorsdk.operator; -import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; -import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Consumer; @@ -202,62 +199,4 @@ public int getRegisteredControllersNumber() { return controllers.size(); } - static class ControllerManager implements LifecycleAware { - private final Map controllers = new HashMap<>(); - private boolean started = false; - - public synchronized void shouldStart() { - if (started) { - return; - } - if (controllers.isEmpty()) { - throw new OperatorException("No Controller exists. Exiting!"); - } - } - - public synchronized void start() { - controllers().parallelStream().forEach(Controller::start); - started = true; - } - - public synchronized void stop() { - controllers().parallelStream().forEach(closeable -> { - log.debug("closing {}", closeable); - closeable.stop(); - }); - - started = false; - } - - @SuppressWarnings("unchecked") - synchronized void add(Controller controller) { - final var configuration = controller.getConfiguration(); - final var resourceTypeName = ReconcilerUtils - .getResourceTypeNameWithVersion(configuration.getResourceClass()); - final var existing = controllers.get(resourceTypeName); - if (existing != null) { - throw new OperatorException("Cannot register controller '" + configuration.getName() - + "': another controller named '" + existing.getConfiguration().getName() - + "' is already registered for resource '" + resourceTypeName + "'"); - } - controllers.put(resourceTypeName, controller); - if (started) { - controller.start(); - } - } - - synchronized Optional get(String name) { - return controllers().stream() - .filter(c -> name.equals(c.getConfiguration().getName())) - .findFirst(); - } - - synchronized Collection controllers() { - return controllers.values(); - } - - synchronized int size() { - return controllers.size(); - } - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index a425bda5bd..f0d1455dd7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.api.config; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -132,4 +133,9 @@ default ObjectMapper getObjectMapper() { default DependentResourceFactory dependentResourceFactory() { return new DependentResourceFactory() {}; } + + default Optional getLeaderElectionConfiguration() { + return Optional.empty(); + } + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index f8833231c2..451cc7f509 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.api.config; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.function.Consumer; @@ -21,6 +22,7 @@ public class ConfigurationServiceOverrider { private boolean closeClientOnStop; private ObjectMapper objectMapper; private ExecutorService executorService = null; + private LeaderElectionConfiguration leaderElectionConfiguration; ConfigurationServiceOverrider(ConfigurationService original) { this.original = original; @@ -80,6 +82,11 @@ public ConfigurationServiceOverrider withObjectMapper(ObjectMapper objectMapper) return this; } + public LeaderElectionConfiguration withLeaderElectionConfiguration( + LeaderElectionConfiguration leaderElectionConfiguration) { + return leaderElectionConfiguration; + } + public ConfigurationService build() { return new BaseConfigurationService(original.getVersion(), cloner) { @Override @@ -130,6 +137,11 @@ public ExecutorService getExecutorService() { public ObjectMapper getObjectMapper() { return objectMapper; } + + @Override + public Optional getLeaderElectionConfiguration() { + return Optional.ofNullable(leaderElectionConfiguration); + } }; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java index 1c0ebec869..57cd409e70 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java @@ -1,13 +1,64 @@ package io.javaoperatorsdk.operator.api.config; +import java.time.Duration; + +// todo discuss leader election with lease vs for life, other options: +// see: https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/manager#Options public class LeaderElectionConfiguration { - private String leaderElectionNamespaces; - private String leaderElectionID; + public static final Duration LEASE_DURATION_DEFAULT_VALUE = Duration.ofSeconds(15); + public static final Duration RENEW_DEADLINE_DEFAULT_VALUE = Duration.ofSeconds(10); + public static final Duration RETRY_PERIOD_DEFAULT_VALUE = Duration.ofSeconds(2); // todo discuss - private boolean syncEventSources; + // private boolean syncEventSources; + + private final String leaseName; + private final String leaseNamespace; + + private final Duration leaseDuration; + private final Duration renewDeadline; + private final Duration retryPeriod; + + public LeaderElectionConfiguration(String leaseName, String leaseNamespace) { + this( + leaseName, + leaseNamespace, + LEASE_DURATION_DEFAULT_VALUE, + RENEW_DEADLINE_DEFAULT_VALUE, + RETRY_PERIOD_DEFAULT_VALUE); + } + + public LeaderElectionConfiguration( + String leaseName, + String leaseNamespace, + Duration leaseDuration, + Duration renewDeadline, + Duration retryPeriod) { + this.leaseName = leaseName; + this.leaseNamespace = leaseNamespace; + this.leaseDuration = leaseDuration; + this.renewDeadline = renewDeadline; + this.retryPeriod = retryPeriod; + } + + public String getLeaseNamespace() { + return leaseNamespace; + } + + public String getLeaseName() { + return leaseName; + } + + public Duration getLeaseDuration() { + return leaseDuration; + } + + public Duration getRenewDeadline() { + return renewDeadline; + } - // todo leader election with lease vs for life, other options: - // see: https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/manager#Options + public Duration getRetryPeriod() { + return retryPeriod; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java new file mode 100644 index 0000000000..c279135a35 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java @@ -0,0 +1,50 @@ +package io.javaoperatorsdk.operator.api.config; + +import java.time.Duration; + +import static io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration.*; + +public final class LeaderElectionConfigurationBuilder { + + private String leaseName; + private String leaseNamespace; + private Duration leaseDuration = LEASE_DURATION_DEFAULT_VALUE; + private Duration renewDeadline = RENEW_DEADLINE_DEFAULT_VALUE; + private Duration retryPeriod = RETRY_PERIOD_DEFAULT_VALUE; + + private LeaderElectionConfigurationBuilder() {} + + public static LeaderElectionConfigurationBuilder aLeaderElectionConfiguration() { + return new LeaderElectionConfigurationBuilder(); + } + + public LeaderElectionConfigurationBuilder withLeaseName(String leaseName) { + this.leaseName = leaseName; + return this; + } + + public LeaderElectionConfigurationBuilder withLeaseNamespace(String leaseNamespace) { + this.leaseNamespace = leaseNamespace; + return this; + } + + public LeaderElectionConfigurationBuilder withLeaseDuration(Duration leaseDuration) { + this.leaseDuration = leaseDuration; + return this; + } + + public LeaderElectionConfigurationBuilder withRenewDeadline(Duration renewDeadline) { + this.renewDeadline = renewDeadline; + return this; + } + + public LeaderElectionConfigurationBuilder withRetryPeriod(Duration retryPeriod) { + this.retryPeriod = retryPeriod; + return this; + } + + public LeaderElectionConfiguration build() { + return new LeaderElectionConfiguration(leaseName, leaseNamespace, leaseDuration, renewDeadline, + retryPeriod); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 66439ab8b8..c7f6efb377 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -36,6 +36,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext; import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflow; import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; +import io.javaoperatorsdk.operator.processing.event.EventProcessor; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -44,7 +45,8 @@ @SuppressWarnings({"unchecked", "rawtypes"}) @Ignore public class Controller

- implements Reconciler

, Cleaner

, LifecycleAware, RegisteredController

{ + implements Reconciler

, Cleaner

, LifecycleAware, + RegisteredController

{ private static final Logger log = LoggerFactory.getLogger(Controller.class); @@ -58,6 +60,8 @@ public class Controller

private final ManagedWorkflow

managedWorkflow; private final GroupVersionKind associatedGVK; + private final EventProcessor

eventProcessor; + public Controller(Reconciler

reconciler, ControllerConfiguration

configuration, @@ -75,6 +79,8 @@ public Controller(Reconciler

reconciler, managedWorkflow = ManagedWorkflow.workflowFor(kubernetesClient, configuration.getDependentResources()); eventSourceManager = new EventSourceManager<>(this); + eventProcessor = new EventProcessor<>(eventSourceManager); + eventSourceManager.postProcessDefaultEventSourcesAfterProcessorInitializer(); } @Override @@ -284,6 +290,7 @@ public void start() throws OperatorException { initAndRegisterEventSources(context); eventSourceManager.start(); + eventProcessor.start(); log.info("'{}' controller started, pending event sources initialization", controllerName); } catch (MissingCRDException e) { stop(); @@ -311,7 +318,17 @@ public void changeNamespaces(Set namespaces) { || namespaces.contains(WATCH_CURRENT_NAMESPACE)) { throw new OperatorException("Unexpected value in target namespaces: " + namespaces); } + eventProcessor.stop(); eventSourceManager.changeNamespaces(namespaces); + eventProcessor.start(); + } + + public void startEventProcessing() { + eventProcessor.start(); + } + + public void stopEventProcessing() { + eventProcessor.stop(); } private void throwMissingCRDException(String crdName, String specVersion, String controllerName) { @@ -350,6 +367,9 @@ public void stop() { if (eventSourceManager != null) { eventSourceManager.stop(); } + if (eventProcessor != null) { + eventProcessor.stop(); + } } public boolean useFinalizer() { @@ -359,4 +379,8 @@ public boolean useFinalizer() { public GroupVersionKind getAssociatedGroupVersionKind() { return associatedGVK; } + + public EventProcessor

getEventProcessor() { + return eventProcessor; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java index 226a656abc..e46878f927 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java @@ -31,7 +31,7 @@ import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; -class EventProcessor implements EventHandler, LifecycleAware { +public class EventProcessor implements EventHandler, LifecycleAware { private static final Logger log = LoggerFactory.getLogger(EventProcessor.class); private static final long MINIMAL_RATE_LIMIT_RESCHEDULE_DURATION = 50; @@ -48,7 +48,7 @@ class EventProcessor implements EventHandler, LifecycleAw private final ResourceStateManager resourceStateManager = new ResourceStateManager(); private final Map metricsMetadata; - EventProcessor(EventSourceManager eventSourceManager) { + public EventProcessor(EventSourceManager eventSourceManager) { this( eventSourceManager.getControllerResourceEventSource(), ExecutorServiceManager.instance().executorService(), diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index ade840ff4b..b5db5f8eee 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -23,31 +23,30 @@ import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction; import io.javaoperatorsdk.operator.processing.event.source.timer.TimerEventSource; -public class EventSourceManager implements LifecycleAware { +public class EventSourceManager

implements LifecycleAware { private static final Logger log = LoggerFactory.getLogger(EventSourceManager.class); - private final EventSources eventSources; - private final EventProcessor eventProcessor; - private final Controller controller; + private final EventSources

eventSources; + private final Controller

controller; - public EventSourceManager(Controller controller) { + public EventSourceManager(Controller

controller) { this(controller, new EventSources<>()); } - EventSourceManager(Controller controller, EventSources eventSources) { + EventSourceManager(Controller

controller, EventSources

eventSources) { this.eventSources = eventSources; this.controller = controller; // controller event source needs to be available before we create the event processor eventSources.initControllerEventSource(controller); - this.eventProcessor = new EventProcessor<>(this); - postProcessDefaultEventSources(); + + postProcessDefaultEventSourcesAfterProcessorInitializer(); } - private void postProcessDefaultEventSources() { - eventSources.controllerResourceEventSource().setEventHandler(eventProcessor); - eventSources.retryEventSource().setEventHandler(eventProcessor); + public void postProcessDefaultEventSourcesAfterProcessorInitializer() { + eventSources.controllerResourceEventSource().setEventHandler(controller.getEventProcessor()); + eventSources.retryEventSource().setEventHandler(controller.getEventProcessor()); } /** @@ -64,7 +63,6 @@ private void postProcessDefaultEventSources() { public synchronized void start() { startEventSource(eventSources.namedControllerResourceEventSource()); eventSources.additionalNamedEventSources().parallel().forEach(this::startEventSource); - eventProcessor.start(); } @Override @@ -72,7 +70,7 @@ public synchronized void stop() { stopEventSource(eventSources.namedControllerResourceEventSource()); eventSources.additionalNamedEventSources().parallel().forEach(this::stopEventSource); eventSources.clear(); - eventProcessor.stop(); + } @SuppressWarnings("rawtypes") @@ -122,7 +120,7 @@ public final synchronized void registerEventSource(String name, EventSource even name = EventSourceInitializer.generateNameFor(eventSource); } eventSources.add(name, eventSource); - eventSource.setEventHandler(eventProcessor); + eventSource.setEventHandler(controller.getEventProcessor()); } catch (IllegalStateException | MissingCRDException e) { throw e; // leave untouched } catch (Exception e) { @@ -132,10 +130,10 @@ public final synchronized void registerEventSource(String name, EventSource even } @SuppressWarnings("unchecked") - public void broadcastOnResourceEvent(ResourceAction action, R resource, R oldResource) { + public void broadcastOnResourceEvent(ResourceAction action, P resource, P oldResource) { eventSources.additionalNamedEventSources().forEach(eventSource -> { if (eventSource instanceof ResourceEventAware) { - var lifecycleAwareES = ((ResourceEventAware) eventSource); + var lifecycleAwareES = ((ResourceEventAware

) eventSource); switch (action) { case ADDED: lifecycleAwareES.onResourceCreated(resource); @@ -152,7 +150,6 @@ public void broadcastOnResourceEvent(ResourceAction action, R resource, R oldRes } public void changeNamespaces(Set namespaces) { - eventProcessor.stop(); eventSources.controllerResourceEventSource() .changeNamespaces(namespaces); eventSources @@ -162,11 +159,6 @@ public void changeNamespaces(Set namespaces) { .filter(NamespaceChangeable::allowsNamespaceChanges) .parallel() .forEach(ies -> ies.changeNamespaces(namespaces)); - eventProcessor.start(); - } - - EventHandler getEventHandler() { - return eventProcessor; } public Set getRegisteredEventSources() { @@ -175,30 +167,30 @@ public Set getRegisteredEventSources() { .collect(Collectors.toCollection(LinkedHashSet::new)); } - public ControllerResourceEventSource getControllerResourceEventSource() { + public ControllerResourceEventSource

getControllerResourceEventSource() { return eventSources.controllerResourceEventSource(); } - ResourceEventSource getResourceEventSourceFor( + ResourceEventSource getResourceEventSourceFor( Class dependentType) { return getResourceEventSourceFor(dependentType, null); } - public List> getEventSourcesFor(Class dependentType) { + public List> getEventSourcesFor(Class dependentType) { return eventSources.getEventSources(dependentType); } - public ResourceEventSource getResourceEventSourceFor( + public ResourceEventSource getResourceEventSourceFor( Class dependentType, String qualifier) { Objects.requireNonNull(dependentType, "dependentType is Mandatory"); return eventSources.get(dependentType, qualifier); } - TimerEventSource retryEventSource() { + TimerEventSource

retryEventSource() { return eventSources.retryEventSource(); } - Controller getController() { + Controller

getController() { return controller; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index 1520f21134..bb19e6b505 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -30,26 +30,26 @@ /** * Handles calls and results of a Reconciler and finalizer related logic */ -class ReconciliationDispatcher { +class ReconciliationDispatcher

{ public static final int MAX_FINALIZER_REMOVAL_RETRY = 10; private static final Logger log = LoggerFactory.getLogger(ReconciliationDispatcher.class); - private final Controller controller; - private final CustomResourceFacade customResourceFacade; + private final Controller

controller; + private final CustomResourceFacade

customResourceFacade; - ReconciliationDispatcher(Controller controller, - CustomResourceFacade customResourceFacade) { + ReconciliationDispatcher(Controller

controller, + CustomResourceFacade

customResourceFacade) { this.controller = controller; this.customResourceFacade = customResourceFacade; } - public ReconciliationDispatcher(Controller controller) { + public ReconciliationDispatcher(Controller

controller) { this(controller, new CustomResourceFacade<>(controller.getCRClient())); } - public PostExecutionControl handleExecution(ExecutionScope executionScope) { + public PostExecutionControl

handleExecution(ExecutionScope

executionScope) { try { return handleDispatch(executionScope); } catch (Exception e) { @@ -58,9 +58,9 @@ public PostExecutionControl handleExecution(ExecutionScope executionScope) } } - private PostExecutionControl handleDispatch(ExecutionScope executionScope) + private PostExecutionControl

handleDispatch(ExecutionScope

executionScope) throws Exception { - R originalResource = executionScope.getResource(); + P originalResource = executionScope.getResource(); var resourceForExecution = cloneResource(originalResource); log.debug("Handling dispatch for resource {}", getName(originalResource)); @@ -73,7 +73,7 @@ private PostExecutionControl handleDispatch(ExecutionScope executionScope) return PostExecutionControl.defaultDispatch(); } - Context context = + Context

context = new DefaultContext<>(executionScope.getRetryInfo(), controller, originalResource); if (markedForDeletion) { return handleCleanup(resourceForExecution, context); @@ -82,7 +82,7 @@ private PostExecutionControl handleDispatch(ExecutionScope executionScope) } } - private boolean shouldNotDispatchToCleanupWhenMarkedForDeletion(R resource) { + private boolean shouldNotDispatchToCleanupWhenMarkedForDeletion(P resource) { var alreadyRemovedFinalizer = controller.useFinalizer() && !resource.hasFinalizer(configuration().getFinalizerName()); if (alreadyRemovedFinalizer) { @@ -92,9 +92,9 @@ private boolean shouldNotDispatchToCleanupWhenMarkedForDeletion(R resource) { return !controller.useFinalizer() || alreadyRemovedFinalizer; } - private PostExecutionControl handleReconcile( - ExecutionScope executionScope, R resourceForExecution, R originalResource, - Context context) throws Exception { + private PostExecutionControl

handleReconcile( + ExecutionScope

executionScope, P resourceForExecution, P originalResource, + Context

context) throws Exception { if (controller.useFinalizer() && !originalResource.hasFinalizer(configuration().getFinalizerName())) { /* @@ -114,21 +114,21 @@ private PostExecutionControl handleReconcile( } } - private R cloneResource(R resource) { + private P cloneResource(P resource) { final var cloner = ConfigurationServiceProvider.instance().getResourceCloner(); return cloner.clone(resource); } - private PostExecutionControl reconcileExecution(ExecutionScope executionScope, - R resourceForExecution, R originalResource, Context context) throws Exception { + private PostExecutionControl

reconcileExecution(ExecutionScope

executionScope, + P resourceForExecution, P originalResource, Context

context) throws Exception { log.debug( "Reconciling resource {} with version: {} with execution scope: {}", getName(resourceForExecution), getVersion(resourceForExecution), executionScope); - UpdateControl updateControl = controller.reconcile(resourceForExecution, context); - R updatedCustomResource = null; + UpdateControl

updateControl = controller.reconcile(resourceForExecution, context); + P updatedCustomResource = null; if (updateControl.isUpdateResourceAndStatus()) { updatedCustomResource = updateCustomResource(updateControl.getResource()); @@ -160,8 +160,8 @@ && shouldUpdateObservedGenerationAutomatically(resourceForExecution)) { } @SuppressWarnings("unchecked") - private PostExecutionControl handleErrorStatusHandler(R resource, R originalResource, - Context context, + private PostExecutionControl

handleErrorStatusHandler(P resource, P originalResource, + Context

context, Exception e) throws Exception { if (isErrorStatusHandlerPresent()) { try { @@ -176,11 +176,11 @@ public boolean isLastAttempt() { return controller.getConfiguration().getRetry() == null; } }); - ((DefaultContext) context).setRetryInfo(retryInfo); - var errorStatusUpdateControl = ((ErrorStatusHandler) controller.getReconciler()) + ((DefaultContext

) context).setRetryInfo(retryInfo); + var errorStatusUpdateControl = ((ErrorStatusHandler

) controller.getReconciler()) .updateErrorStatus(resource, context, e); - R updatedResource = null; + P updatedResource = null; if (errorStatusUpdateControl.getResource().isPresent()) { updatedResource = errorStatusUpdateControl.isPatch() ? customResourceFacade .patchStatus(errorStatusUpdateControl.getResource().orElseThrow(), originalResource) @@ -207,7 +207,7 @@ private boolean isErrorStatusHandlerPresent() { return controller.getReconciler() instanceof ErrorStatusHandler; } - private R updateStatusGenerationAware(R resource, R originalResource, boolean patch) { + private P updateStatusGenerationAware(P resource, P originalResource, boolean patch) { updateStatusObservedGenerationIfRequired(resource); if (patch) { return customResourceFacade.patchStatus(resource, originalResource); @@ -217,7 +217,7 @@ private R updateStatusGenerationAware(R resource, R originalResource, boolean pa } @SuppressWarnings("rawtypes") - private boolean shouldUpdateObservedGenerationAutomatically(R resource) { + private boolean shouldUpdateObservedGenerationAutomatically(P resource) { if (configuration().isGenerationAware() && resource instanceof CustomResource) { var customResource = (CustomResource) resource; var status = customResource.getStatus(); @@ -232,7 +232,7 @@ private boolean shouldUpdateObservedGenerationAutomatically(R resource) { } @SuppressWarnings("rawtypes") - private void updateStatusObservedGenerationIfRequired(R resource) { + private void updateStatusObservedGenerationIfRequired(P resource) { if (configuration().isGenerationAware() && resource instanceof CustomResource) { var customResource = (CustomResource) resource; var status = customResource.getStatus(); @@ -244,9 +244,9 @@ private void updateStatusObservedGenerationIfRequired(R resource) { } } - private PostExecutionControl createPostExecutionControl(R updatedCustomResource, - UpdateControl updateControl) { - PostExecutionControl postExecutionControl; + private PostExecutionControl

createPostExecutionControl(P updatedCustomResource, + UpdateControl

updateControl) { + PostExecutionControl

postExecutionControl; if (updatedCustomResource != null) { if (updateControl.isUpdateStatus() && updateControl.isPatch()) { postExecutionControl = @@ -262,7 +262,7 @@ private PostExecutionControl createPostExecutionControl(R updatedCustomResour } private void updatePostExecutionControlWithReschedule( - PostExecutionControl postExecutionControl, + PostExecutionControl

postExecutionControl, BaseControl baseControl) { baseControl.getScheduleDelay().ifPresentOrElse(postExecutionControl::withReSchedule, () -> controller.getConfiguration().maxReconciliationInterval() @@ -270,7 +270,7 @@ private void updatePostExecutionControlWithReschedule( } - private PostExecutionControl handleCleanup(R resource, Context context) { + private PostExecutionControl

handleCleanup(P resource, Context

context) { log.debug( "Executing delete for resource: {} with version: {}", getName(resource), @@ -283,7 +283,7 @@ private PostExecutionControl handleCleanup(R resource, Context context) { // cleanup is finished, nothing left to done final var finalizerName = configuration().getFinalizerName(); if (deleteControl.isRemoveFinalizer() && resource.hasFinalizer(finalizerName)) { - R customResource = removeFinalizer(resource, finalizerName); + P customResource = removeFinalizer(resource, finalizerName); return PostExecutionControl.customResourceFinalizerRemoved(customResource); } } @@ -293,29 +293,29 @@ private PostExecutionControl handleCleanup(R resource, Context context) { getVersion(resource), deleteControl, useFinalizer); - PostExecutionControl postExecutionControl = PostExecutionControl.defaultDispatch(); + PostExecutionControl

postExecutionControl = PostExecutionControl.defaultDispatch(); updatePostExecutionControlWithReschedule(postExecutionControl, deleteControl); return postExecutionControl; } - private R updateCustomResourceWithFinalizer(R resource) { + private P updateCustomResourceWithFinalizer(P resource) { log.debug( "Adding finalizer for resource: {} version: {}", getUID(resource), getVersion(resource)); resource.addFinalizer(configuration().getFinalizerName()); return customResourceFacade.updateResource(resource); } - private R updateCustomResource(R resource) { + private P updateCustomResource(P resource) { log.debug("Updating resource: {} with version: {}", getUID(resource), getVersion(resource)); log.trace("Resource before update: {}", resource); return customResourceFacade.updateResource(resource); } - ControllerConfiguration configuration() { + ControllerConfiguration

configuration() { return controller.getConfiguration(); } - public R removeFinalizer(R resource, String finalizer) { + public P removeFinalizer(P resource, String finalizer) { if (log.isDebugEnabled()) { log.debug("Removing finalizer on resource: {}", ResourceID.fromResource(resource)); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java index b328ace132..d788f61e4a 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java @@ -4,15 +4,10 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.Operator.ControllerManager; import io.javaoperatorsdk.operator.api.config.DefaultControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.Controller; -import io.javaoperatorsdk.operator.sample.simple.DuplicateCRController; -import io.javaoperatorsdk.operator.sample.simple.TestCustomReconciler; -import io.javaoperatorsdk.operator.sample.simple.TestCustomReconcilerOtherV1; -import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; -import io.javaoperatorsdk.operator.sample.simple.TestCustomResourceOtherV1; +import io.javaoperatorsdk.operator.sample.simple.*; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index 779fe032f9..2782ed52d1 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -39,7 +39,7 @@ public void registersEventSource() { Set registeredSources = eventSourceManager.getRegisteredEventSources(); assertThat(registeredSources).contains(eventSource); - verify(eventSource, times(1)).setEventHandler(eventSourceManager.getEventHandler()); + verify(eventSource, times(1)).setEventHandler(any()); } @Test From 9837cccf980f83295a9f6afb2c992307b6ca4513 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 21 Jul 2022 17:17:14 +0200 Subject: [PATCH 03/31] wip --- .../operator/LeaderElectionManager.java | 6 +++ .../io/javaoperatorsdk/operator/Operator.java | 40 +++++++++++++++---- 2 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java new file mode 100644 index 0000000000..3712d2b7b6 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -0,0 +1,6 @@ +package io.javaoperatorsdk.operator; + +public class LeaderElectionManager { + + public LeaderElectionManager() {} +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index f403e8b05e..b28808dacb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -11,13 +11,14 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.NamespacedKubernetesClient; import io.fabric8.kubernetes.client.Version; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; -import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; +import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectionConfig; +import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElector; +import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectorBuilder; +import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.LeaseLock; +import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.Lock; +import io.javaoperatorsdk.operator.api.config.*; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.LifecycleAware; @@ -27,6 +28,7 @@ public class Operator implements LifecycleAware { private static final Logger log = LoggerFactory.getLogger(Operator.class); private final KubernetesClient kubernetesClient; private final ControllerManager controllers = new ControllerManager(); + private LeaderElector leaderElector; public Operator() { this(new DefaultKubernetesClient(), ConfigurationServiceProvider.instance()); @@ -49,8 +51,9 @@ public Operator(Consumer overrider) { } public Operator(KubernetesClient client, Consumer overrider) { - this(client); + this.kubernetesClient = client; ConfigurationServiceProvider.overrideCurrent(overrider); + } /** @@ -81,6 +84,7 @@ public KubernetesClient getKubernetesClient() { */ public void start() { try { + controllers.shouldStart(); final var version = ConfigurationServiceProvider.instance().getVersion(); @@ -199,4 +203,26 @@ public int getRegisteredControllersNumber() { return controllers.size(); } + private void initLeaderElector(KubernetesClient client) { + + var leaderElectionConfig = + ConfigurationServiceProvider.instance().getLeaderElectionConfiguration(); + if (leaderElectionConfig.isEmpty()) { + return; + } + var conf = leaderElectionConfig.get(); + + // todo discuss openshift client not a NamespacedKubernetesClient? + // name of the pod + // todo configurable + String identity = System.getenv("HOSTNAME"); + + Lock lock = new LeaseLock(conf.getLeaseNamespace(), conf.getLeaseName(), identity); + // todo check release on cancel + leaderElector = new LeaderElectorBuilder<>((NamespacedKubernetesClient) client) + .withConfig(new LeaderElectionConfig(lock, conf.getLeaseDuration(), conf.getRenewDeadline(), + conf.getRetryPeriod(), null, true, conf.getLeaseName())) + .build(); + } + } From f035fb36328815c270dc7b49cdefc093a4e7ae74 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 22 Jul 2022 13:18:15 +0200 Subject: [PATCH 04/31] wip --- .../operator/ControllerManager.java | 4 +- .../operator/LeaderElectionManager.java | 62 ++++++++++++++++++- .../io/javaoperatorsdk/operator/Operator.java | 52 +++++----------- .../config/LeaderElectionConfiguration.java | 21 ++++++- 4 files changed, 95 insertions(+), 44 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java index ef62f4cf07..ca2deb27c1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java @@ -12,8 +12,8 @@ import io.javaoperatorsdk.operator.processing.LifecycleAware; /** - * Not confuse with controller manager form go operators. The high level aggregate is Operator in - * JOSDK. + * Not confuse with controller manager form go operators. The highest level aggregate is + * {@link Operator} in JOSDK. */ class ControllerManager implements LifecycleAware { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index 3712d2b7b6..3d7cebf71d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -1,6 +1,66 @@ package io.javaoperatorsdk.operator; +import java.util.UUID; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.NamespacedKubernetesClient; +import io.fabric8.kubernetes.client.extended.leaderelection.LeaderCallbacks; +import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectionConfig; +import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElector; +import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectorBuilder; +import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.LeaseLock; +import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.Lock; +import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; + public class LeaderElectionManager { - public LeaderElectionManager() {} + private static final Logger log = LoggerFactory.getLogger(LeaderElectionManager.class); + + private LeaderElector leaderElector = null; + private ControllerManager controllerManager; + + public LeaderElectionManager(ControllerManager controllerManager) { + this.controllerManager = controllerManager; + } + + public void init(LeaderElectionConfiguration config, KubernetesClient client) { + Lock lock = new LeaseLock(config.getLeaseNamespace(), config.getLeaseName(), identity(config)); + // the releaseOnCancel does not seem to be used anywhere + leaderElector = new LeaderElectorBuilder<>((NamespacedKubernetesClient) client) + .withConfig( + new LeaderElectionConfig(lock, config.getLeaseDuration(), config.getRenewDeadline(), + config.getRetryPeriod(), leaderCallbacks(), true, config.getLeaseName())) + .build(); + } + + + public boolean isLeaderElectionOn() { + return leaderElector != null; + } + + private LeaderCallbacks leaderCallbacks() { + return new LeaderCallbacks(this::startLeading, this::stopLeading, leader -> { + log.info("New leader with identity: {}", leader); + }); + } + + private void startLeading() { + + } + + private void stopLeading() { + + } + + private String identity(LeaderElectionConfiguration config) { + String identity = config.getIdentity().orElse(System.getenv("HOSTNAME")); + if (identity == null || identity.isBlank()) { + identity = UUID.randomUUID().toString(); + } + return identity; + } + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index b28808dacb..826320a212 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -11,13 +11,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; -import io.fabric8.kubernetes.client.NamespacedKubernetesClient; import io.fabric8.kubernetes.client.Version; -import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectionConfig; -import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElector; -import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectorBuilder; -import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.LeaseLock; -import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.Lock; import io.javaoperatorsdk.operator.api.config.*; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.Controller; @@ -27,8 +21,9 @@ public class Operator implements LifecycleAware { private static final Logger log = LoggerFactory.getLogger(Operator.class); private final KubernetesClient kubernetesClient; - private final ControllerManager controllers = new ControllerManager(); - private LeaderElector leaderElector; + private final ControllerManager controllerManager = new ControllerManager(); + private final LeaderElectionManager leaderElectionManager = + new LeaderElectionManager(controllerManager); public Operator() { this(new DefaultKubernetesClient(), ConfigurationServiceProvider.instance()); @@ -53,7 +48,8 @@ public Operator(Consumer overrider) { public Operator(KubernetesClient client, Consumer overrider) { this.kubernetesClient = client; ConfigurationServiceProvider.overrideCurrent(overrider); - + ConfigurationServiceProvider.instance().getLeaderElectionConfiguration() + .ifPresent(c -> leaderElectionManager.init(c, kubernetesClient)); } /** @@ -66,6 +62,8 @@ public Operator(KubernetesClient client, Consumer public Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService) { this.kubernetesClient = kubernetesClient; ConfigurationServiceProvider.set(configurationService); + configurationService.getLeaderElectionConfiguration() + .ifPresent(c -> leaderElectionManager.init(c, kubernetesClient)); } /** Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. */ @@ -85,7 +83,7 @@ public KubernetesClient getKubernetesClient() { public void start() { try { - controllers.shouldStart(); + controllerManager.shouldStart(); final var version = ConfigurationServiceProvider.instance().getVersion(); log.info( @@ -98,7 +96,7 @@ public void start() { log.info("Client version: {}", clientVersion); ExecutorServiceManager.init(); - controllers.start(); + controllerManager.start(); } catch (Exception e) { log.error("Error starting operator", e); stop(); @@ -112,7 +110,7 @@ public void stop() throws OperatorException { log.info( "Operator SDK {} is shutting down...", configurationService.getVersion().getSdkVersion()); - controllers.stop(); + controllerManager.stop(); ExecutorServiceManager.stop(); if (configurationService.closeClientOnStop()) { @@ -162,7 +160,7 @@ public

RegisteredController

register(Reconciler

re final var controller = new Controller<>(reconciler, configuration, kubernetesClient); - controllers.add(controller); + controllerManager.add(controller); final var watchedNS = configuration.watchAllNamespaces() ? "[all namespaces]" : configuration.getEffectiveNamespaces(); @@ -192,37 +190,15 @@ public

RegisteredController

register(Reconciler

re } public Optional getRegisteredController(String name) { - return controllers.get(name).map(RegisteredController.class::cast); + return controllerManager.get(name).map(RegisteredController.class::cast); } public Set getRegisteredControllers() { - return new HashSet<>(controllers.controllers()); + return new HashSet<>(controllerManager.controllers()); } public int getRegisteredControllersNumber() { - return controllers.size(); - } - - private void initLeaderElector(KubernetesClient client) { - - var leaderElectionConfig = - ConfigurationServiceProvider.instance().getLeaderElectionConfiguration(); - if (leaderElectionConfig.isEmpty()) { - return; - } - var conf = leaderElectionConfig.get(); - - // todo discuss openshift client not a NamespacedKubernetesClient? - // name of the pod - // todo configurable - String identity = System.getenv("HOSTNAME"); - - Lock lock = new LeaseLock(conf.getLeaseNamespace(), conf.getLeaseName(), identity); - // todo check release on cancel - leaderElector = new LeaderElectorBuilder<>((NamespacedKubernetesClient) client) - .withConfig(new LeaderElectionConfig(lock, conf.getLeaseDuration(), conf.getRenewDeadline(), - conf.getRetryPeriod(), null, true, conf.getLeaseName())) - .build(); + return controllerManager.size(); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java index 57cd409e70..de82f53150 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java @@ -1,9 +1,8 @@ package io.javaoperatorsdk.operator.api.config; import java.time.Duration; +import java.util.Optional; -// todo discuss leader election with lease vs for life, other options: -// see: https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/manager#Options public class LeaderElectionConfiguration { public static final Duration LEASE_DURATION_DEFAULT_VALUE = Duration.ofSeconds(15); @@ -15,6 +14,7 @@ public class LeaderElectionConfiguration { private final String leaseName; private final String leaseNamespace; + private final String identity; private final Duration leaseDuration; private final Duration renewDeadline; @@ -26,7 +26,7 @@ public LeaderElectionConfiguration(String leaseName, String leaseNamespace) { leaseNamespace, LEASE_DURATION_DEFAULT_VALUE, RENEW_DEADLINE_DEFAULT_VALUE, - RETRY_PERIOD_DEFAULT_VALUE); + RETRY_PERIOD_DEFAULT_VALUE, null); } public LeaderElectionConfiguration( @@ -35,11 +35,22 @@ public LeaderElectionConfiguration( Duration leaseDuration, Duration renewDeadline, Duration retryPeriod) { + this(leaseName, leaseNamespace, leaseDuration, renewDeadline, retryPeriod, null); + } + + public LeaderElectionConfiguration( + String leaseName, + String leaseNamespace, + Duration leaseDuration, + Duration renewDeadline, + Duration retryPeriod, + String identity) { this.leaseName = leaseName; this.leaseNamespace = leaseNamespace; this.leaseDuration = leaseDuration; this.renewDeadline = renewDeadline; this.retryPeriod = retryPeriod; + this.identity = identity; } public String getLeaseNamespace() { @@ -61,4 +72,8 @@ public Duration getRenewDeadline() { public Duration getRetryPeriod() { return retryPeriod; } + + public Optional getIdentity() { + return Optional.ofNullable(identity); + } } From 3f3923c425223eb2453188edb5ee12481ec439e8 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 25 Jul 2022 15:05:05 +0200 Subject: [PATCH 05/31] migrate to v6 --- .../operator/LeaderElectionManager.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index 3d7cebf71d..6240f344b9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -6,20 +6,20 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.client.KubernetesClient; -import io.fabric8.kubernetes.client.NamespacedKubernetesClient; import io.fabric8.kubernetes.client.extended.leaderelection.LeaderCallbacks; import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectionConfig; import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElector; import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectorBuilder; import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.LeaseLock; import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.Lock; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; public class LeaderElectionManager { private static final Logger log = LoggerFactory.getLogger(LeaderElectionManager.class); - private LeaderElector leaderElector = null; + private LeaderElector leaderElector = null; private ControllerManager controllerManager; public LeaderElectionManager(ControllerManager controllerManager) { @@ -28,8 +28,10 @@ public LeaderElectionManager(ControllerManager controllerManager) { public void init(LeaderElectionConfiguration config, KubernetesClient client) { Lock lock = new LeaseLock(config.getLeaseNamespace(), config.getLeaseName(), identity(config)); - // the releaseOnCancel does not seem to be used anywhere - leaderElector = new LeaderElectorBuilder<>((NamespacedKubernetesClient) client) + // todo releaseOnCancel + // todo use this executor service? + leaderElector = new LeaderElectorBuilder(client, + ConfigurationServiceProvider.instance().getExecutorService()) .withConfig( new LeaderElectionConfig(lock, config.getLeaseDuration(), config.getRenewDeadline(), config.getRetryPeriod(), leaderCallbacks(), true, config.getLeaseName())) From dc3f2a98d5de8c2499f0318ae65584d6787b768e Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 1 Aug 2022 15:10:30 +0200 Subject: [PATCH 06/31] leader election impl --- .../operator/ControllerManager.java | 18 ++++++----- .../operator/LeaderElectionManager.java | 31 ++++++++++++++----- .../io/javaoperatorsdk/operator/Operator.java | 21 ++++++++----- .../operator/processing/Controller.java | 25 +++++++-------- .../operator/processing/ControllerTest.java | 4 +-- .../event/ReconciliationDispatcherTest.java | 2 +- 6 files changed, 63 insertions(+), 38 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java index ca2deb27c1..dcede9a1ff 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java @@ -9,16 +9,16 @@ import org.slf4j.LoggerFactory; import io.javaoperatorsdk.operator.processing.Controller; -import io.javaoperatorsdk.operator.processing.LifecycleAware; /** * Not confuse with controller manager form go operators. The highest level aggregate is * {@link Operator} in JOSDK. */ -class ControllerManager implements LifecycleAware { +class ControllerManager { private static final Logger log = LoggerFactory.getLogger(ControllerManager.class); + @SuppressWarnings("rawtypes") private final Map controllers = new HashMap<>(); private boolean started = false; @@ -31,8 +31,8 @@ public synchronized void shouldStart() { } } - public synchronized void start() { - controllers().parallelStream().forEach(Controller::start); + public synchronized void start(boolean startEventProcessor) { + controllers().parallelStream().forEach(c -> c.start(startEventProcessor)); started = true; } @@ -41,10 +41,13 @@ public synchronized void stop() { log.debug("closing {}", closeable); closeable.stop(); }); - started = false; } + public synchronized void startEventProcessing() { + controllers().parallelStream().forEach(Controller::startEventProcessing); + } + @SuppressWarnings("unchecked") synchronized void add(Controller controller) { final var configuration = controller.getConfiguration(); @@ -57,17 +60,16 @@ synchronized void add(Controller controller) { + "' is already registered for resource '" + resourceTypeName + "'"); } controllers.put(resourceTypeName, controller); - if (started) { - controller.start(); - } } + @SuppressWarnings("rawtypes") synchronized Optional get(String name) { return controllers().stream() .filter(c -> name.equals(c.getConfiguration().getName())) .findFirst(); } + @SuppressWarnings("rawtypes") synchronized Collection controllers() { return controllers.values(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index 6240f344b9..d35f3d676f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -1,6 +1,7 @@ package io.javaoperatorsdk.operator; import java.util.UUID; +import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -20,16 +21,18 @@ public class LeaderElectionManager { private static final Logger log = LoggerFactory.getLogger(LeaderElectionManager.class); private LeaderElector leaderElector = null; - private ControllerManager controllerManager; + private final ControllerManager controllerManager; + private String identity; + private CompletableFuture leaderElectionFuture; public LeaderElectionManager(ControllerManager controllerManager) { this.controllerManager = controllerManager; } public void init(LeaderElectionConfiguration config, KubernetesClient client) { - Lock lock = new LeaseLock(config.getLeaseNamespace(), config.getLeaseName(), identity(config)); - // todo releaseOnCancel - // todo use this executor service? + this.identity = identity(config); + Lock lock = new LeaseLock(config.getLeaseNamespace(), config.getLeaseName(), identity); + // releaseOnCancel is not used in the underlying implementation leaderElector = new LeaderElectorBuilder(client, ConfigurationServiceProvider.instance().getExecutorService()) .withConfig( @@ -38,7 +41,6 @@ public void init(LeaderElectionConfiguration config, KubernetesClient client) { .build(); } - public boolean isLeaderElectionOn() { return leaderElector != null; } @@ -50,11 +52,15 @@ private LeaderCallbacks leaderCallbacks() { } private void startLeading() { - + controllerManager.startEventProcessing(); } private void stopLeading() { - + log.info("Stopped leading for identity: {}. Exiting.", identity); + // When leader stops leading the process ends immediately to prevent multiple reconciliations + // running parallel. + // Note that some reconciliations might run a very long time. + System.exit(1); } private String identity(LeaderElectionConfiguration config) { @@ -65,4 +71,15 @@ private String identity(LeaderElectionConfiguration config) { return identity; } + public void start() { + if (isLeaderElectionOn()) { + leaderElectionFuture = leaderElector.start(); + } + } + + public void stop() { + if (leaderElectionFuture != null) { + leaderElectionFuture.cancel(false); + } + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 826320a212..00952f9d5a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -11,6 +11,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.fabric8.kubernetes.client.Version; import io.javaoperatorsdk.operator.api.config.*; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -24,9 +25,10 @@ public class Operator implements LifecycleAware { private final ControllerManager controllerManager = new ControllerManager(); private final LeaderElectionManager leaderElectionManager = new LeaderElectionManager(controllerManager); + private volatile boolean started = false; public Operator() { - this(new DefaultKubernetesClient(), ConfigurationServiceProvider.instance()); + this(new KubernetesClientBuilder().build(), ConfigurationServiceProvider.instance()); } public Operator(KubernetesClient kubernetesClient) { @@ -42,7 +44,7 @@ public Operator(ConfigurationService configurationService) { } public Operator(Consumer overrider) { - this(new DefaultKubernetesClient(), overrider); + this(new KubernetesClientBuilder().build(), overrider); } public Operator(KubernetesClient client, Consumer overrider) { @@ -82,9 +84,11 @@ public KubernetesClient getKubernetesClient() { */ public void start() { try { - + if (started) { + return; + } + started = true; controllerManager.shouldStart(); - final var version = ConfigurationServiceProvider.instance().getVersion(); log.info( "Operator SDK {} (commit: {}) built on {} starting...", @@ -94,9 +98,9 @@ public void start() { final var clientVersion = Version.clientVersion(); log.info("Client version: {}", clientVersion); - ExecutorServiceManager.init(); - controllerManager.start(); + controllerManager.start(!leaderElectionManager.isLeaderElectionOn()); + leaderElectionManager.start(); } catch (Exception e) { log.error("Error starting operator", e); stop(); @@ -111,8 +115,8 @@ public void stop() throws OperatorException { "Operator SDK {} is shutting down...", configurationService.getVersion().getSdkVersion()); controllerManager.stop(); - ExecutorServiceManager.stop(); + leaderElectionManager.stop(); if (configurationService.closeClientOnStop()) { kubernetesClient.close(); } @@ -148,6 +152,9 @@ public

RegisteredController

register(Reconciler

re public

RegisteredController

register(Reconciler

reconciler, ControllerConfiguration

configuration) throws OperatorException { + if (started) { + throw new OperatorException("Operator already started. Register all the controllers before."); + } if (configuration == null) { throw new OperatorException( diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index c7f6efb377..3e4bf1fdb3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -45,7 +45,7 @@ @SuppressWarnings({"unchecked", "rawtypes"}) @Ignore public class Controller

- implements Reconciler

, Cleaner

, LifecycleAware, + implements Reconciler

, Cleaner

, RegisteredController

{ private static final Logger log = LoggerFactory.getLogger(Controller.class); @@ -62,7 +62,6 @@ public class Controller

private final GroupVersionKind associatedGVK; private final EventProcessor

eventProcessor; - public Controller(Reconciler

reconciler, ControllerConfiguration

configuration, KubernetesClient kubernetesClient) { @@ -272,7 +271,7 @@ public MixedOperation, Resource

> getCRClient() { * * @throws OperatorException if a problem occurred during the registration process */ - public void start() throws OperatorException { + public synchronized void start(boolean startEventProcessor) throws OperatorException { final Class

resClass = configuration.getResourceClass(); final String controllerName = configuration.getName(); final var crdName = configuration.getResourceTypeName(); @@ -290,7 +289,9 @@ public void start() throws OperatorException { initAndRegisterEventSources(context); eventSourceManager.start(); - eventProcessor.start(); + if (startEventProcessor) { + eventProcessor.start(); + } log.info("'{}' controller started, pending event sources initialization", controllerName); } catch (MissingCRDException e) { stop(); @@ -298,6 +299,7 @@ public void start() throws OperatorException { } } + private void validateCRDWithLocalModelIfRequired(Class

resClass, String controllerName, String crdName, String specVersion) { final CustomResourceDefinition crd; @@ -323,14 +325,11 @@ public void changeNamespaces(Set namespaces) { eventProcessor.start(); } - public void startEventProcessing() { + public synchronized void startEventProcessing() { + log.info("Started event processing for controller: {}", configuration.getName()); eventProcessor.start(); } - public void stopEventProcessing() { - eventProcessor.stop(); - } - private void throwMissingCRDException(String crdName, String specVersion, String controllerName) { throw new MissingCRDException( crdName, @@ -363,13 +362,13 @@ public EventSourceManager

getEventSourceManager() { return eventSourceManager; } - public void stop() { - if (eventSourceManager != null) { - eventSourceManager.stop(); - } + public synchronized void stop() { if (eventProcessor != null) { eventProcessor.stop(); } + if (eventSourceManager != null) { + eventSourceManager.stop(); + } } public boolean useFinalizer() { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 67e9112b27..95504f504b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -24,7 +24,7 @@ void crdShouldNotBeCheckedForNativeResources() { final var configuration = MockControllerConfiguration.forResource(Secret.class); final var controller = new Controller(reconciler, configuration, client); - controller.start(); + controller.start(true); verify(client, never()).apiextensions(); } @@ -36,7 +36,7 @@ void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { try { ConfigurationServiceProvider.overrideCurrent(o -> o.checkingCRDAndValidateLocalModel(false)); final var controller = new Controller(reconciler, configuration, client); - controller.start(); + controller.start(true); verify(client, never()).apiextensions(); } finally { ConfigurationServiceProvider.reset(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index 89dafa9fc4..dfa01783f9 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -121,7 +121,7 @@ public boolean useFinalizer() { return useFinalizer; } }; - controller.start(); + controller.start(true); return new ReconciliationDispatcher<>(controller, customResourceFacade); } From 866f06a5550512da8416b2eb8e2e8f8b2db5ccb1 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 1 Aug 2022 16:31:15 +0200 Subject: [PATCH 07/31] leader election IT --- .../operator/LeaderElectionIT.java | 12 ++++++++ .../LeaderElectionTestCustomResource.java | 16 ++++++++++ ...eaderElectionTestCustomResourceStatus.java | 21 ++++++++++++++ .../LeaderElectionTestReconciler.java | 29 +++++++++++++++++++ 4 files changed, 78 insertions(+) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java new file mode 100644 index 0000000000..5445424db8 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java @@ -0,0 +1,12 @@ +package io.javaoperatorsdk.operator; + +import org.junit.jupiter.api.Test; + +class LeaderElectionIT { + + @Test + void leaderElection() { + + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java new file mode 100644 index 0000000000..cc83c7e617 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java @@ -0,0 +1,16 @@ +package io.javaoperatorsdk.operator.sample.leaderelection; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Kind; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("le") +public class LeaderElectionTestCustomResource + extends CustomResource + implements Namespaced { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java new file mode 100644 index 0000000000..e7667efdf2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java @@ -0,0 +1,21 @@ +package io.javaoperatorsdk.operator.sample.leaderelection; + +import java.util.ArrayList; +import java.util.List; + +public class LeaderElectionTestCustomResourceStatus { + + private List reconciledBy; + + public List getReconciledBy() { + if (reconciledBy == null) { + reconciledBy = new ArrayList<>(); + } + return reconciledBy; + } + + public LeaderElectionTestCustomResourceStatus setReconciledBy(List reconciledBy) { + this.reconciledBy = reconciledBy; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java new file mode 100644 index 0000000000..d2d276f6ab --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java @@ -0,0 +1,29 @@ +package io.javaoperatorsdk.operator.sample.leaderelection; + +import io.javaoperatorsdk.operator.api.reconciler.*; + +import java.time.Duration; + +@ControllerConfiguration() +public class LeaderElectionTestReconciler + implements Reconciler { + + private String reconcilerName; + + public LeaderElectionTestReconciler(String reconcilerName) { + this.reconcilerName = reconcilerName; + } + + @Override + public UpdateControl reconcile( + LeaderElectionTestCustomResource resource, Context context) { + + if (resource.getStatus() == null) { + resource.setStatus(new LeaderElectionTestCustomResourceStatus()); + } + + resource.getStatus().getReconciledBy().add(reconcilerName); + return UpdateControl.patchStatus(resource).rescheduleAfter(Duration.ofSeconds(200)); + } + +} From 4edd392e14b59c8566509e8c3097abef852a3eeb Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 1 Aug 2022 17:08:09 +0200 Subject: [PATCH 08/31] format --- .../operator/LeaderElectionIT.java | 6 +++--- .../LeaderElectionTestCustomResource.java | 1 - ...eaderElectionTestCustomResourceStatus.java | 20 +++++++++---------- .../LeaderElectionTestReconciler.java | 7 ++++--- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java index 5445424db8..1b6520f7e0 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java @@ -4,9 +4,9 @@ class LeaderElectionIT { - @Test - void leaderElection() { + @Test + void leaderElection() { - } + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java index cc83c7e617..a9c6f504a2 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java @@ -3,7 +3,6 @@ import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.model.annotation.Group; -import io.fabric8.kubernetes.model.annotation.Kind; import io.fabric8.kubernetes.model.annotation.ShortNames; import io.fabric8.kubernetes.model.annotation.Version; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java index e7667efdf2..1ac6dca767 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java @@ -5,17 +5,17 @@ public class LeaderElectionTestCustomResourceStatus { - private List reconciledBy; + private List reconciledBy; - public List getReconciledBy() { - if (reconciledBy == null) { - reconciledBy = new ArrayList<>(); - } - return reconciledBy; + public List getReconciledBy() { + if (reconciledBy == null) { + reconciledBy = new ArrayList<>(); } + return reconciledBy; + } - public LeaderElectionTestCustomResourceStatus setReconciledBy(List reconciledBy) { - this.reconciledBy = reconciledBy; - return this; - } + public LeaderElectionTestCustomResourceStatus setReconciledBy(List reconciledBy) { + this.reconciledBy = reconciledBy; + return this; + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java index d2d276f6ab..ff4674666b 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java @@ -1,9 +1,9 @@ package io.javaoperatorsdk.operator.sample.leaderelection; -import io.javaoperatorsdk.operator.api.reconciler.*; - import java.time.Duration; +import io.javaoperatorsdk.operator.api.reconciler.*; + @ControllerConfiguration() public class LeaderElectionTestReconciler implements Reconciler { @@ -16,7 +16,8 @@ public LeaderElectionTestReconciler(String reconcilerName) { @Override public UpdateControl reconcile( - LeaderElectionTestCustomResource resource, Context context) { + LeaderElectionTestCustomResource resource, + Context context) { if (resource.getStatus() == null) { resource.setStatus(new LeaderElectionTestCustomResourceStatus()); From 53cd42f9622ac4a571be28fa4768aae171890cf1 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 3 Aug 2022 13:11:03 +0200 Subject: [PATCH 09/31] e2e test --- .github/workflows/e2e-test.yml | 1 + .../operator/LeaderElectionManager.java | 8 +- .../config/ConfigurationServiceOverrider.java | 1 + .../LeaderElectionConfigurationBuilder.java | 10 +- .../operator/LeaderElectionIT.java | 12 -- .../LeaderElectionTestReconciler.java | 30 --- sample-operators/leader-election/README.md | 6 + .../k8s/operator-instance-2.yaml | 19 ++ .../leader-election/k8s/operator.yaml | 65 +++++++ sample-operators/leader-election/pom.xml | 82 +++++++++ .../operator/sample/LeaderElectionTest.java | 6 +- .../sample/LeaderElectionTestOperator.java | 37 ++++ .../sample/LeaderElectionTestReconciler.java | 35 ++++ .../sample/LeaderElectionTestStatus.java | 6 +- .../src/main/resources/log4j2.xml | 13 ++ .../operator/sample/LeaderElectionE2E.java | 173 ++++++++++++++++++ .../src/test/resources/log4j2.xml | 13 ++ sample-operators/pom.xml | 1 + 18 files changed, 464 insertions(+), 54 deletions(-) delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java create mode 100644 sample-operators/leader-election/README.md create mode 100644 sample-operators/leader-election/k8s/operator-instance-2.yaml create mode 100644 sample-operators/leader-election/k8s/operator.yaml create mode 100644 sample-operators/leader-election/pom.xml rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java => sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTest.java (67%) create mode 100644 sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java create mode 100644 sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java => sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestStatus.java (59%) create mode 100644 sample-operators/leader-election/src/main/resources/log4j2.xml create mode 100644 sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java create mode 100644 sample-operators/leader-election/src/test/resources/log4j2.xml diff --git a/.github/workflows/e2e-test.yml b/.github/workflows/e2e-test.yml index df96f28d90..1007878ea9 100644 --- a/.github/workflows/e2e-test.yml +++ b/.github/workflows/e2e-test.yml @@ -21,6 +21,7 @@ jobs: - "sample-operators/mysql-schema" - "sample-operators/tomcat-operator" - "sample-operators/webpage" + - "sample-operators/leader-election" runs-on: ubuntu-latest steps: - name: Checkout diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index d35f3d676f..f425f228f3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -64,11 +64,11 @@ private void stopLeading() { } private String identity(LeaderElectionConfiguration config) { - String identity = config.getIdentity().orElse(System.getenv("HOSTNAME")); - if (identity == null || identity.isBlank()) { - identity = UUID.randomUUID().toString(); + String id = config.getIdentity().orElse(System.getenv("HOSTNAME")); + if (id == null || id.isBlank()) { + id = UUID.randomUUID().toString(); } - return identity; + return id; } public void start() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index 451cc7f509..c9d2fab302 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -84,6 +84,7 @@ public ConfigurationServiceOverrider withObjectMapper(ObjectMapper objectMapper) public LeaderElectionConfiguration withLeaderElectionConfiguration( LeaderElectionConfiguration leaderElectionConfiguration) { + this.leaderElectionConfiguration = leaderElectionConfiguration; return leaderElectionConfiguration; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java index c279135a35..979d9cb50c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java @@ -11,8 +11,9 @@ public final class LeaderElectionConfigurationBuilder { private Duration leaseDuration = LEASE_DURATION_DEFAULT_VALUE; private Duration renewDeadline = RENEW_DEADLINE_DEFAULT_VALUE; private Duration retryPeriod = RETRY_PERIOD_DEFAULT_VALUE; + private String identity; - private LeaderElectionConfigurationBuilder() {} + public LeaderElectionConfigurationBuilder() {} public static LeaderElectionConfigurationBuilder aLeaderElectionConfiguration() { return new LeaderElectionConfigurationBuilder(); @@ -43,8 +44,13 @@ public LeaderElectionConfigurationBuilder withRetryPeriod(Duration retryPeriod) return this; } + public LeaderElectionConfigurationBuilder withIdentity(String identity) { + this.identity = identity; + return this; + } + public LeaderElectionConfiguration build() { return new LeaderElectionConfiguration(leaseName, leaseNamespace, leaseDuration, renewDeadline, - retryPeriod); + retryPeriod, identity); } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java deleted file mode 100644 index 1b6520f7e0..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionIT.java +++ /dev/null @@ -1,12 +0,0 @@ -package io.javaoperatorsdk.operator; - -import org.junit.jupiter.api.Test; - -class LeaderElectionIT { - - @Test - void leaderElection() { - - } - -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java deleted file mode 100644 index ff4674666b..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestReconciler.java +++ /dev/null @@ -1,30 +0,0 @@ -package io.javaoperatorsdk.operator.sample.leaderelection; - -import java.time.Duration; - -import io.javaoperatorsdk.operator.api.reconciler.*; - -@ControllerConfiguration() -public class LeaderElectionTestReconciler - implements Reconciler { - - private String reconcilerName; - - public LeaderElectionTestReconciler(String reconcilerName) { - this.reconcilerName = reconcilerName; - } - - @Override - public UpdateControl reconcile( - LeaderElectionTestCustomResource resource, - Context context) { - - if (resource.getStatus() == null) { - resource.setStatus(new LeaderElectionTestCustomResourceStatus()); - } - - resource.getStatus().getReconciledBy().add(reconcilerName); - return UpdateControl.patchStatus(resource).rescheduleAfter(Duration.ofSeconds(200)); - } - -} diff --git a/sample-operators/leader-election/README.md b/sample-operators/leader-election/README.md new file mode 100644 index 0000000000..2a9a369a45 --- /dev/null +++ b/sample-operators/leader-election/README.md @@ -0,0 +1,6 @@ +# Leader Election E2E Test + +The purpose of this module is to e2e test leader election feature. + +The deployment is using directly pods in order to better control some aspects in test. +In real life this would be a Deployment. \ No newline at end of file diff --git a/sample-operators/leader-election/k8s/operator-instance-2.yaml b/sample-operators/leader-election/k8s/operator-instance-2.yaml new file mode 100644 index 0000000000..abde7eb56a --- /dev/null +++ b/sample-operators/leader-election/k8s/operator-instance-2.yaml @@ -0,0 +1,19 @@ +apiVersion: v1 +kind: Pod +metadata: + name: leader-election-operator-2 +spec: + serviceAccountName: leader-election-operator + containers: + - name: operator + image: leader-election-operator + imagePullPolicy: Never + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name diff --git a/sample-operators/leader-election/k8s/operator.yaml b/sample-operators/leader-election/k8s/operator.yaml new file mode 100644 index 0000000000..fd75ba9cfc --- /dev/null +++ b/sample-operators/leader-election/k8s/operator.yaml @@ -0,0 +1,65 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: leader-election-operator + +--- +apiVersion: v1 +kind: Pod +metadata: + name: leader-election-operator-1 +spec: + serviceAccountName: leader-election-operator + containers: + - name: operator + image: leader-election-operator + imagePullPolicy: Never + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: operator-admin +subjects: + - kind: ServiceAccount + name: leader-election-operator +roleRef: + kind: ClusterRole + name: leader-election-operator + apiGroup: "" + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: leader-election-operator +rules: + - apiGroups: + - "apiextensions.k8s.io" + resources: + - customresourcedefinitions + verbs: + - '*' + - apiGroups: + - "sample.javaoperatorsdk" + resources: + - leaderelectiontests + - leaderelectiontests/status + verbs: + - '*' + - apiGroups: + - "coordination.k8s.io" + resources: + - "leases" + verbs: + - '*' + diff --git a/sample-operators/leader-election/pom.xml b/sample-operators/leader-election/pom.xml new file mode 100644 index 0000000000..8eb84a01ea --- /dev/null +++ b/sample-operators/leader-election/pom.xml @@ -0,0 +1,82 @@ + + + 4.0.0 + + + io.javaoperatorsdk + sample-operators + 3.1.1-SNAPSHOT + + + sample-leader-election + Operator SDK - Samples - Leader Election + An E2E test for leader election + jar + + + 11 + 11 + 3.2.1 + + + + + io.javaoperatorsdk + operator-framework + + + org.apache.logging.log4j + log4j-slf4j-impl + + + org.takes + takes + 1.21.1 + + + io.fabric8 + crd-generator-apt + provided + + + io.fabric8 + crd-generator-api + + + org.awaitility + awaitility + compile + + + io.javaoperatorsdk + operator-framework-junit-5 + ${project.version} + test + + + + + + com.google.cloud.tools + jib-maven-plugin + ${jib-maven-plugin.version} + + + gcr.io/distroless/java:11 + + + leader-election-operator + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.10.0 + + + + + \ No newline at end of file diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTest.java similarity index 67% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java rename to sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTest.java index a9c6f504a2..3832df9158 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResource.java +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTest.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample.leaderelection; +package io.javaoperatorsdk.operator.sample; import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; @@ -9,7 +9,7 @@ @Group("sample.javaoperatorsdk") @Version("v1") @ShortNames("le") -public class LeaderElectionTestCustomResource - extends CustomResource +public class LeaderElectionTest + extends CustomResource implements Namespaced { } diff --git a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java new file mode 100644 index 0000000000..fc25ad7d16 --- /dev/null +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java @@ -0,0 +1,37 @@ +package io.javaoperatorsdk.operator.sample; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.client.ConfigBuilder; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; +import io.javaoperatorsdk.operator.Operator; +import io.javaoperatorsdk.operator.api.config.LeaderElectionConfigurationBuilder; + +public class LeaderElectionTestOperator { + + private static final Logger log = LoggerFactory.getLogger(LeaderElectionTestOperator.class); + + public static void main(String[] args) { + System.out.println("Starting..."); + String identity = System.getenv("POD_NAME"); + String namespace = System.getenv("POD_NAMESPACE"); + + log.info("Starting operator with identity: {}", identity); + + var client = new KubernetesClientBuilder().withConfig(new ConfigBuilder() + .withNamespace(namespace) + .build()).build(); + + Operator operator = new Operator(client, + c -> c.withLeaderElectionConfiguration(new LeaderElectionConfigurationBuilder() + .withLeaseName("leader-election-test") + .withLeaseNamespace(namespace) + .withIdentity(identity) + .build())); + operator.register(new LeaderElectionTestReconciler(identity)); + operator.installShutdownHook(); + operator.start(); + } + +} diff --git a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java new file mode 100644 index 0000000000..e53fd5f652 --- /dev/null +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java @@ -0,0 +1,35 @@ +package io.javaoperatorsdk.operator.sample; + +import java.time.Duration; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; + +@ControllerConfiguration() +public class LeaderElectionTestReconciler + implements Reconciler { + + + private final String reconcilerName; + + public LeaderElectionTestReconciler(String reconcilerName) { + this.reconcilerName = reconcilerName; + } + + @Override + public UpdateControl reconcile( + LeaderElectionTest resource, + Context context) { + + if (resource.getStatus() == null) { + resource.setStatus(new LeaderElectionTestStatus()); + } + + resource.getStatus().getReconciledBy().add(reconcilerName); + + return UpdateControl.patchStatus(resource).rescheduleAfter(Duration.ofSeconds(1)); + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestStatus.java similarity index 59% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java rename to sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestStatus.java index 1ac6dca767..5a9e66e270 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelection/LeaderElectionTestCustomResourceStatus.java +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestStatus.java @@ -1,9 +1,9 @@ -package io.javaoperatorsdk.operator.sample.leaderelection; +package io.javaoperatorsdk.operator.sample; import java.util.ArrayList; import java.util.List; -public class LeaderElectionTestCustomResourceStatus { +public class LeaderElectionTestStatus { private List reconciledBy; @@ -14,7 +14,7 @@ public List getReconciledBy() { return reconciledBy; } - public LeaderElectionTestCustomResourceStatus setReconciledBy(List reconciledBy) { + public LeaderElectionTestStatus setReconciledBy(List reconciledBy) { this.reconciledBy = reconciledBy; return this; } diff --git a/sample-operators/leader-election/src/main/resources/log4j2.xml b/sample-operators/leader-election/src/main/resources/log4j2.xml new file mode 100644 index 0000000000..bd0d711c2d --- /dev/null +++ b/sample-operators/leader-election/src/main/resources/log4j2.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java new file mode 100644 index 0000000000..66ec8e49c8 --- /dev/null +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -0,0 +1,173 @@ +package io.javaoperatorsdk.operator.sample; + +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.time.Duration; +import java.util.List; +import java.util.Locale; +import java.util.OptionalInt; +import java.util.UUID; +import java.util.stream.IntStream; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.NamespaceBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding; +import io.fabric8.kubernetes.client.ConfigBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; + +import static io.javaoperatorsdk.operator.junit.AbstractOperatorExtension.CRD_READY_WAIT; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class LeaderElectionE2E { + + private static final Logger log = LoggerFactory.getLogger(LeaderElectionE2E.class); + public static final int POD_STARTUP_TIMEOUT = 20; + + public static final String TEST_RESOURCE_NAME = "test1"; + public static final int MINIMAL_SECONDS_FOR_RENEWAL = 3; + public static final int MAX_WAIT_SECONDS = 10; + + private static final String OPERATOR_1_POD_NAME = "leader-election-operator-1"; + private static final String OPERATOR_2_POD_NAME = "leader-election-operator-2"; + + private String namespace; + private KubernetesClient client; + + @Test + void otherInstancesTakesOverWhenSteppingDown() { + log.info("Deploying operator"); + deployOperatorsInOrder(); + log.info("Applying custom resource"); + applyCustomResource(); + + log.info("Awaiting custom resource reconciliations"); + await().pollDelay(Duration.ofSeconds(MINIMAL_SECONDS_FOR_RENEWAL)) + .atMost(Duration.ofSeconds(MAX_WAIT_SECONDS)) + .untilAsserted(() -> { + var actualStatus = client.resources(LeaderElectionTest.class) + .inNamespace(namespace).withName(TEST_RESOURCE_NAME).get().getStatus(); + + assertThat(actualStatus).isNotNull(); + assertThat(actualStatus.getReconciledBy()).hasSizeGreaterThan(2); + }); + + client.pods().inNamespace(namespace).withName(OPERATOR_1_POD_NAME).delete(); + + var actualListSize = client.resources(LeaderElectionTest.class) + .inNamespace(namespace).withName(TEST_RESOURCE_NAME).get().getStatus().getReconciledBy() + .size(); + + await().pollDelay(Duration.ofSeconds(MINIMAL_SECONDS_FOR_RENEWAL)) + .atMost(Duration.ofSeconds(240)) + .untilAsserted(() -> { + var actualStatus = client.resources(LeaderElectionTest.class) + .inNamespace(namespace).withName(TEST_RESOURCE_NAME).get().getStatus(); + + assertThat(actualStatus).isNotNull(); + assertThat(actualStatus.getReconciledBy()).hasSizeGreaterThan(actualListSize + 2); + }); + + assertReconciliations(client.resources(LeaderElectionTest.class).inNamespace(namespace) + .withName(TEST_RESOURCE_NAME).get().getStatus().getReconciledBy()); + } + + private void assertReconciliations(List reconciledBy) { + log.info("Reconciled by content: {}", reconciledBy); + OptionalInt firstO2StatusIndex = IntStream.range(0, reconciledBy.size()) + .filter(i -> reconciledBy.get(i).equals(OPERATOR_2_POD_NAME)) + .findFirst(); + assertThat(firstO2StatusIndex).isPresent(); + assertThat(reconciledBy.subList(0, firstO2StatusIndex.getAsInt() - 1)) + .allMatch(s -> s.equals(OPERATOR_1_POD_NAME)); + assertThat(reconciledBy.subList(firstO2StatusIndex.getAsInt(), reconciledBy.size())) + .allMatch(s -> s.equals(OPERATOR_2_POD_NAME)); + } + + private void applyCustomResource() { + var res = new LeaderElectionTest(); + res.setMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .withNamespace(namespace) + .build()); + client.resource(res).create(); + } + + @BeforeEach + void setup() { + namespace = "leader-election-it-" + UUID.randomUUID(); + client = new KubernetesClientBuilder().withConfig(new ConfigBuilder() + .withNamespace(namespace) + .build()).build(); + applyCRD(); + client.namespaces().resource(new NamespaceBuilder().withNewMetadata().withName(namespace) + .endMetadata().build()).create(); + } + + @AfterEach + void tearDown() { + client.namespaces().resource(new NamespaceBuilder().withNewMetadata().withName(namespace) + .endMetadata().build()).delete(); + await() + .atMost(Duration.ofSeconds(15)) + .untilAsserted(() -> assertThat(client.namespaces().withName(namespace).get()).isNull()); + } + + private void deployOperatorsInOrder() { + applyResources("k8s/operator.yaml"); + await().atMost(Duration.ofSeconds(POD_STARTUP_TIMEOUT)).untilAsserted(() -> { + var pod = client.pods().inNamespace(namespace).withName(OPERATOR_1_POD_NAME).get(); + assertThat(pod.getStatus().getContainerStatuses().get(0).getReady()).isTrue(); + }); + + applyResources("k8s/operator-instance-2.yaml"); + await().atMost(Duration.ofSeconds(POD_STARTUP_TIMEOUT)).untilAsserted(() -> { + var pod = client.pods().inNamespace(namespace).withName(OPERATOR_2_POD_NAME).get(); + assertThat(pod.getStatus().getContainerStatuses().get(0).getReady()).isTrue(); + }); + } + + void applyCRD() { + String path = + "./target/classes/META-INF/fabric8/leaderelectiontests.sample.javaoperatorsdk-v1.yml"; + try (InputStream is = new FileInputStream(path)) { + final var crd = client.load(is); + crd.createOrReplace(); + Thread.sleep(CRD_READY_WAIT); + log.debug("Applied CRD with name: {}", crd.get().get(0).getMetadata().getName()); + } catch (InterruptedException | IOException e) { + throw new RuntimeException(e); + } + } + + void applyResources(String path) { + try { + List resources = client.load(new FileInputStream(path)).get(); + resources.forEach(hm -> { + hm.getMetadata().setNamespace(namespace); + if (hm.getKind().toLowerCase(Locale.ROOT).equals("clusterrolebinding")) { + var crb = (ClusterRoleBinding) hm; + for (var subject : crb.getSubjects()) { + subject.setNamespace(namespace); + } + } + }); + client.resourceList(resources) + .inNamespace(namespace) + .createOrReplace(); + + } catch (FileNotFoundException e) { + throw new RuntimeException(e); + } + } +} diff --git a/sample-operators/leader-election/src/test/resources/log4j2.xml b/sample-operators/leader-election/src/test/resources/log4j2.xml new file mode 100644 index 0000000000..2b7fdd3479 --- /dev/null +++ b/sample-operators/leader-election/src/test/resources/log4j2.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/sample-operators/pom.xml b/sample-operators/pom.xml index f8772d62da..4c533716b5 100644 --- a/sample-operators/pom.xml +++ b/sample-operators/pom.xml @@ -22,5 +22,6 @@ tomcat-operator webpage mysql-schema + leader-election \ No newline at end of file From 5a1cdff6d842729d27bc73d09d32dc4aea0904ba Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 3 Aug 2022 13:30:36 +0200 Subject: [PATCH 10/31] e2e improvements --- .../leader-election/k8s/operator.yaml | 4 ++-- ... => LeaderElectionTestCustomResource.java} | 2 +- .../sample/LeaderElectionTestReconciler.java | 9 ++++----- .../operator/sample/LeaderElectionE2E.java | 19 ++++++++++--------- 4 files changed, 17 insertions(+), 17 deletions(-) rename sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/{LeaderElectionTest.java => LeaderElectionTestCustomResource.java} (91%) diff --git a/sample-operators/leader-election/k8s/operator.yaml b/sample-operators/leader-election/k8s/operator.yaml index fd75ba9cfc..00ed3e7273 100644 --- a/sample-operators/leader-election/k8s/operator.yaml +++ b/sample-operators/leader-election/k8s/operator.yaml @@ -52,8 +52,8 @@ rules: - apiGroups: - "sample.javaoperatorsdk" resources: - - leaderelectiontests - - leaderelectiontests/status + - leaderelectiontestcustomresources + - leaderelectiontestcustomresources/status verbs: - '*' - apiGroups: diff --git a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTest.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestCustomResource.java similarity index 91% rename from sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTest.java rename to sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestCustomResource.java index 3832df9158..b440be4d18 100644 --- a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTest.java +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestCustomResource.java @@ -9,7 +9,7 @@ @Group("sample.javaoperatorsdk") @Version("v1") @ShortNames("le") -public class LeaderElectionTest +public class LeaderElectionTestCustomResource extends CustomResource implements Namespaced { } diff --git a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java index e53fd5f652..f4681c677a 100644 --- a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java @@ -9,8 +9,7 @@ @ControllerConfiguration() public class LeaderElectionTestReconciler - implements Reconciler { - + implements Reconciler { private final String reconcilerName; @@ -19,9 +18,9 @@ public LeaderElectionTestReconciler(String reconcilerName) { } @Override - public UpdateControl reconcile( - LeaderElectionTest resource, - Context context) { + public UpdateControl reconcile( + LeaderElectionTestCustomResource resource, + Context context) { if (resource.getStatus() == null) { resource.setStatus(new LeaderElectionTestStatus()); diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 66ec8e49c8..7119a5629e 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -36,7 +36,7 @@ class LeaderElectionE2E { public static final String TEST_RESOURCE_NAME = "test1"; public static final int MINIMAL_SECONDS_FOR_RENEWAL = 3; - public static final int MAX_WAIT_SECONDS = 10; + public static final int MAX_WAIT_SECONDS = 30; private static final String OPERATOR_1_POD_NAME = "leader-election-operator-1"; private static final String OPERATOR_2_POD_NAME = "leader-election-operator-2"; @@ -55,7 +55,7 @@ void otherInstancesTakesOverWhenSteppingDown() { await().pollDelay(Duration.ofSeconds(MINIMAL_SECONDS_FOR_RENEWAL)) .atMost(Duration.ofSeconds(MAX_WAIT_SECONDS)) .untilAsserted(() -> { - var actualStatus = client.resources(LeaderElectionTest.class) + var actualStatus = client.resources(LeaderElectionTestCustomResource.class) .inNamespace(namespace).withName(TEST_RESOURCE_NAME).get().getStatus(); assertThat(actualStatus).isNotNull(); @@ -64,22 +64,23 @@ void otherInstancesTakesOverWhenSteppingDown() { client.pods().inNamespace(namespace).withName(OPERATOR_1_POD_NAME).delete(); - var actualListSize = client.resources(LeaderElectionTest.class) + var actualListSize = client.resources(LeaderElectionTestCustomResource.class) .inNamespace(namespace).withName(TEST_RESOURCE_NAME).get().getStatus().getReconciledBy() .size(); await().pollDelay(Duration.ofSeconds(MINIMAL_SECONDS_FOR_RENEWAL)) .atMost(Duration.ofSeconds(240)) .untilAsserted(() -> { - var actualStatus = client.resources(LeaderElectionTest.class) + var actualStatus = client.resources(LeaderElectionTestCustomResource.class) .inNamespace(namespace).withName(TEST_RESOURCE_NAME).get().getStatus(); assertThat(actualStatus).isNotNull(); assertThat(actualStatus.getReconciledBy()).hasSizeGreaterThan(actualListSize + 2); }); - assertReconciliations(client.resources(LeaderElectionTest.class).inNamespace(namespace) - .withName(TEST_RESOURCE_NAME).get().getStatus().getReconciledBy()); + assertReconciliations( + client.resources(LeaderElectionTestCustomResource.class).inNamespace(namespace) + .withName(TEST_RESOURCE_NAME).get().getStatus().getReconciledBy()); } private void assertReconciliations(List reconciledBy) { @@ -95,7 +96,7 @@ private void assertReconciliations(List reconciledBy) { } private void applyCustomResource() { - var res = new LeaderElectionTest(); + var res = new LeaderElectionTestCustomResource(); res.setMetadata(new ObjectMetaBuilder() .withName(TEST_RESOURCE_NAME) .withNamespace(namespace) @@ -119,7 +120,7 @@ void tearDown() { client.namespaces().resource(new NamespaceBuilder().withNewMetadata().withName(namespace) .endMetadata().build()).delete(); await() - .atMost(Duration.ofSeconds(15)) + .atMost(Duration.ofSeconds(60)) .untilAsserted(() -> assertThat(client.namespaces().withName(namespace).get()).isNull()); } @@ -139,7 +140,7 @@ private void deployOperatorsInOrder() { void applyCRD() { String path = - "./target/classes/META-INF/fabric8/leaderelectiontests.sample.javaoperatorsdk-v1.yml"; + "./target/classes/META-INF/fabric8/leaderelectiontestcustomresources.sample.javaoperatorsdk-v1.yml"; try (InputStream is = new FileInputStream(path)) { final var crd = client.load(is); crd.createOrReplace(); From bb4eb9d26433c1af847f77ae9d75f1d7af96f70c Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 3 Aug 2022 13:43:40 +0200 Subject: [PATCH 11/31] pod startup timeout --- .../io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 7119a5629e..70abe13271 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -32,7 +32,7 @@ class LeaderElectionE2E { private static final Logger log = LoggerFactory.getLogger(LeaderElectionE2E.class); - public static final int POD_STARTUP_TIMEOUT = 20; + public static final int POD_STARTUP_TIMEOUT = 60; public static final String TEST_RESOURCE_NAME = "test1"; public static final int MINIMAL_SECONDS_FOR_RENEWAL = 3; From f0fddf3e4962552a66d7407a517546438c7ffe85 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 3 Aug 2022 13:48:25 +0200 Subject: [PATCH 12/31] pod --- .../io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 70abe13271..3d80e09022 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -128,6 +128,8 @@ private void deployOperatorsInOrder() { applyResources("k8s/operator.yaml"); await().atMost(Duration.ofSeconds(POD_STARTUP_TIMEOUT)).untilAsserted(() -> { var pod = client.pods().inNamespace(namespace).withName(OPERATOR_1_POD_NAME).get(); + log.info("Operator 1: {}",pod); + assertThat(pod.getStatus().getContainerStatuses().get(0).getReady()).isTrue(); }); From f81be31cd9f454ba97b7de61460a90a5d404e56e Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 3 Aug 2022 13:56:40 +0200 Subject: [PATCH 13/31] image pull policy --- sample-operators/leader-election/k8s/operator-instance-2.yaml | 2 +- sample-operators/leader-election/k8s/operator.yaml | 2 +- .../io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sample-operators/leader-election/k8s/operator-instance-2.yaml b/sample-operators/leader-election/k8s/operator-instance-2.yaml index abde7eb56a..5e5211500c 100644 --- a/sample-operators/leader-election/k8s/operator-instance-2.yaml +++ b/sample-operators/leader-election/k8s/operator-instance-2.yaml @@ -7,7 +7,7 @@ spec: containers: - name: operator image: leader-election-operator - imagePullPolicy: Never + imagePullPolicy: IfNotPresent env: - name: POD_NAMESPACE valueFrom: diff --git a/sample-operators/leader-election/k8s/operator.yaml b/sample-operators/leader-election/k8s/operator.yaml index 00ed3e7273..501c13ecec 100644 --- a/sample-operators/leader-election/k8s/operator.yaml +++ b/sample-operators/leader-election/k8s/operator.yaml @@ -13,7 +13,7 @@ spec: containers: - name: operator image: leader-election-operator - imagePullPolicy: Never + imagePullPolicy: IfNotPresent env: - name: POD_NAMESPACE valueFrom: diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 3d80e09022..2034c89c39 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -128,8 +128,8 @@ private void deployOperatorsInOrder() { applyResources("k8s/operator.yaml"); await().atMost(Duration.ofSeconds(POD_STARTUP_TIMEOUT)).untilAsserted(() -> { var pod = client.pods().inNamespace(namespace).withName(OPERATOR_1_POD_NAME).get(); - log.info("Operator 1: {}",pod); - + log.info("Operator 1: {}", pod); + assertThat(pod.getStatus().getContainerStatuses().get(0).getReady()).isTrue(); }); From 382cfd15a094fc33362c0b0007ce328450d90a2a Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 3 Aug 2022 14:10:06 +0200 Subject: [PATCH 14/31] disable test for local mode --- .../leader-election/k8s/operator-instance-2.yaml | 2 +- sample-operators/leader-election/k8s/operator.yaml | 2 +- .../javaoperatorsdk/operator/sample/LeaderElectionE2E.java | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sample-operators/leader-election/k8s/operator-instance-2.yaml b/sample-operators/leader-election/k8s/operator-instance-2.yaml index 5e5211500c..abde7eb56a 100644 --- a/sample-operators/leader-election/k8s/operator-instance-2.yaml +++ b/sample-operators/leader-election/k8s/operator-instance-2.yaml @@ -7,7 +7,7 @@ spec: containers: - name: operator image: leader-election-operator - imagePullPolicy: IfNotPresent + imagePullPolicy: Never env: - name: POD_NAMESPACE valueFrom: diff --git a/sample-operators/leader-election/k8s/operator.yaml b/sample-operators/leader-election/k8s/operator.yaml index 501c13ecec..00ed3e7273 100644 --- a/sample-operators/leader-election/k8s/operator.yaml +++ b/sample-operators/leader-election/k8s/operator.yaml @@ -13,7 +13,7 @@ spec: containers: - name: operator image: leader-election-operator - imagePullPolicy: IfNotPresent + imagePullPolicy: Never env: - name: POD_NAMESPACE valueFrom: diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 2034c89c39..243ccd8277 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -14,6 +14,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfSystemProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,6 +46,8 @@ class LeaderElectionE2E { private KubernetesClient client; @Test + // not for local mode by design + @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") void otherInstancesTakesOverWhenSteppingDown() { log.info("Deploying operator"); deployOperatorsInOrder(); @@ -128,8 +131,6 @@ private void deployOperatorsInOrder() { applyResources("k8s/operator.yaml"); await().atMost(Duration.ofSeconds(POD_STARTUP_TIMEOUT)).untilAsserted(() -> { var pod = client.pods().inNamespace(namespace).withName(OPERATOR_1_POD_NAME).get(); - log.info("Operator 1: {}", pod); - assertThat(pod.getStatus().getContainerStatuses().get(0).getReady()).isTrue(); }); From c2d2211717475130b505f2faab162615bb7c3289 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 3 Aug 2022 16:08:54 +0200 Subject: [PATCH 15/31] docs --- docs/documentation/features.md | 12 ++++++++++++ .../java/io/javaoperatorsdk/operator/Operator.java | 2 ++ .../api/config/LeaderElectionConfiguration.java | 3 --- .../operator/sample/LeaderElectionTestOperator.java | 1 - .../leader-election/src/main/resources/log4j2.xml | 2 +- sample-operators/pom.xml | 2 +- 6 files changed, 16 insertions(+), 6 deletions(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 727349ddd3..aa4efe4f94 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -683,6 +683,18 @@ See also the [integration test](https://github.com/java-operator-sdk/java-operator-sdk/blob/ec37025a15046d8f409c77616110024bf32c3416/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java) for this feature. +## Leader Election + +In general operators are deployed with single instance running, or a single active instance running. That means +if multiple instances are deployed only one of the should process the events. This is achieved by configuring leader +election. Leader election will make sure only one instance of the operator is processing events, other instances will +start event sources, where those will populate the caches - this is beneficial for the case the instance becomes the +leader it will start reconcile immediately, and don't have to wait until the event sources are started and +caches are populated. + +See sample configuration in the [E2E test](https://github.com/java-operator-sdk/java-operator-sdk/blob/144947d89323f1c65de6e86bd8b9a6a8ffe714ff/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java#L26-L30) +. + ## Monitoring with Micrometer ## Automatic Generation of CRDs diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 00952f9d5a..abbf58571e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -99,6 +99,8 @@ public void start() { final var clientVersion = Version.clientVersion(); log.info("Client version: {}", clientVersion); ExecutorServiceManager.init(); + // first start the controller manager before leader election, + // the leader election would start subsequently the processor if on controllerManager.start(!leaderElectionManager.isLeaderElectionOn()); leaderElectionManager.start(); } catch (Exception e) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java index de82f53150..67764be655 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java @@ -9,9 +9,6 @@ public class LeaderElectionConfiguration { public static final Duration RENEW_DEADLINE_DEFAULT_VALUE = Duration.ofSeconds(10); public static final Duration RETRY_PERIOD_DEFAULT_VALUE = Duration.ofSeconds(2); - // todo discuss - // private boolean syncEventSources; - private final String leaseName; private final String leaseNamespace; private final String identity; diff --git a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java index fc25ad7d16..32dd9ebefd 100644 --- a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java @@ -13,7 +13,6 @@ public class LeaderElectionTestOperator { private static final Logger log = LoggerFactory.getLogger(LeaderElectionTestOperator.class); public static void main(String[] args) { - System.out.println("Starting..."); String identity = System.getenv("POD_NAME"); String namespace = System.getenv("POD_NAMESPACE"); diff --git a/sample-operators/leader-election/src/main/resources/log4j2.xml b/sample-operators/leader-election/src/main/resources/log4j2.xml index bd0d711c2d..0ec69bf713 100644 --- a/sample-operators/leader-election/src/main/resources/log4j2.xml +++ b/sample-operators/leader-election/src/main/resources/log4j2.xml @@ -10,4 +10,4 @@ - \ No newline at end of file + diff --git a/sample-operators/pom.xml b/sample-operators/pom.xml index 4c533716b5..82b0e8ba00 100644 --- a/sample-operators/pom.xml +++ b/sample-operators/pom.xml @@ -24,4 +24,4 @@ mysql-schema leader-election - \ No newline at end of file + From d29f51bf57ac4cbe9abb1e9dac81ebe240927361 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 5 Aug 2022 13:02:28 +0200 Subject: [PATCH 16/31] fixes on rebase --- sample-operators/leader-election/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sample-operators/leader-election/pom.xml b/sample-operators/leader-election/pom.xml index 8eb84a01ea..8ae0e3e4ba 100644 --- a/sample-operators/leader-election/pom.xml +++ b/sample-operators/leader-election/pom.xml @@ -7,7 +7,7 @@ io.javaoperatorsdk sample-operators - 3.1.1-SNAPSHOT + 3.2.0-SNAPSHOT sample-leader-election From 1e4f64c97651f76320cf6dca64e2b7a88b6c4b35 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 22 Aug 2022 18:21:19 +0200 Subject: [PATCH 17/31] docs: improve wording --- docs/documentation/features.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index aa4efe4f94..06fa0698c0 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -685,12 +685,13 @@ for this feature. ## Leader Election -In general operators are deployed with single instance running, or a single active instance running. That means -if multiple instances are deployed only one of the should process the events. This is achieved by configuring leader -election. Leader election will make sure only one instance of the operator is processing events, other instances will -start event sources, where those will populate the caches - this is beneficial for the case the instance becomes the -leader it will start reconcile immediately, and don't have to wait until the event sources are started and -caches are populated. +Operators are generally deployed with a single running or active instance. However, it is +possible to deploy multiple instances in such a way that only one, called the "leader", processes the +events. This is achieved via a mechanism called "leader election". While all the instances are +running, and even start their event sources to populate the caches, only the leader will process +the events. This means that should the leader change for any reason, for example because it +crashed, the other instances are already warmed up and ready to pick up where the previous +leader left off should one of them become elected leader. See sample configuration in the [E2E test](https://github.com/java-operator-sdk/java-operator-sdk/blob/144947d89323f1c65de6e86bd8b9a6a8ffe714ff/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java#L26-L30) . From 6361b36c906fbf30371625e2ee906f6d537a1247 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 23 Aug 2022 11:22:21 +0200 Subject: [PATCH 18/31] fix: match other methods' usage pattern --- .../operator/api/config/ConfigurationServiceOverrider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index c9d2fab302..20dca09f1e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -82,10 +82,10 @@ public ConfigurationServiceOverrider withObjectMapper(ObjectMapper objectMapper) return this; } - public LeaderElectionConfiguration withLeaderElectionConfiguration( + public ConfigurationServiceOverrider withLeaderElectionConfiguration( LeaderElectionConfiguration leaderElectionConfiguration) { this.leaderElectionConfiguration = leaderElectionConfiguration; - return leaderElectionConfiguration; + return this; } public ConfigurationService build() { From 440bf0f207e6281a3123b73a4df8dc5964d7aaa8 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 23 Aug 2022 12:32:12 +0200 Subject: [PATCH 19/31] refactor: make sure all constructors cascade --- .../io/javaoperatorsdk/operator/Operator.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index abbf58571e..14f273b7b4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -9,7 +9,6 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.fabric8.kubernetes.client.Version; @@ -28,7 +27,7 @@ public class Operator implements LifecycleAware { private volatile boolean started = false; public Operator() { - this(new KubernetesClientBuilder().build(), ConfigurationServiceProvider.instance()); + this((KubernetesClient) null); } public Operator(KubernetesClient kubernetesClient) { @@ -40,18 +39,16 @@ public Operator(KubernetesClient kubernetesClient) { */ @Deprecated public Operator(ConfigurationService configurationService) { - this(new DefaultKubernetesClient(), configurationService); + this(null, configurationService); } public Operator(Consumer overrider) { - this(new KubernetesClientBuilder().build(), overrider); + this(null, overrider); } public Operator(KubernetesClient client, Consumer overrider) { - this.kubernetesClient = client; + this(client); ConfigurationServiceProvider.overrideCurrent(overrider); - ConfigurationServiceProvider.instance().getLeaderElectionConfiguration() - .ifPresent(c -> leaderElectionManager.init(c, kubernetesClient)); } /** @@ -62,10 +59,11 @@ public Operator(KubernetesClient client, Consumer * @param configurationService provides configuration */ public Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService) { - this.kubernetesClient = kubernetesClient; + this.kubernetesClient = + kubernetesClient != null ? kubernetesClient : new KubernetesClientBuilder().build(); ConfigurationServiceProvider.set(configurationService); configurationService.getLeaderElectionConfiguration() - .ifPresent(c -> leaderElectionManager.init(c, kubernetesClient)); + .ifPresent(c -> leaderElectionManager.init(c, this.kubernetesClient)); } /** Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. */ From 0cd2a90abe5cdf6a5f382d0510ac1c9b43814234 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 23 Aug 2022 12:35:52 +0200 Subject: [PATCH 20/31] docs: improve wording --- .../java/io/javaoperatorsdk/operator/ControllerManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java index dcede9a1ff..44aa430ec2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java @@ -11,8 +11,8 @@ import io.javaoperatorsdk.operator.processing.Controller; /** - * Not confuse with controller manager form go operators. The highest level aggregate is - * {@link Operator} in JOSDK. + * Not to be confused with the controller manager concept from Go's controller-runtime project. In + * JOSDK, the equivalent concept is {@link Operator}. */ class ControllerManager { @@ -48,7 +48,7 @@ public synchronized void startEventProcessing() { controllers().parallelStream().forEach(Controller::startEventProcessing); } - @SuppressWarnings("unchecked") + @SuppressWarnings({"unchecked", "rawtypes"}) synchronized void add(Controller controller) { final var configuration = controller.getConfiguration(); final var resourceTypeName = ReconcilerUtils From 93a260a7a2ee0d53b1289b70101b168b75969c70 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 23 Aug 2022 12:51:20 +0200 Subject: [PATCH 21/31] refactor: move identity generation to configuration --- .../operator/LeaderElectionManager.java | 26 ++++++------------- .../config/LeaderElectionConfiguration.java | 21 ++++++++++++--- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index f425f228f3..eb5d540d7c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -1,6 +1,5 @@ package io.javaoperatorsdk.operator; -import java.util.UUID; import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; @@ -16,7 +15,7 @@ import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; -public class LeaderElectionManager { +class LeaderElectionManager { private static final Logger log = LoggerFactory.getLogger(LeaderElectionManager.class); @@ -30,15 +29,15 @@ public LeaderElectionManager(ControllerManager controllerManager) { } public void init(LeaderElectionConfiguration config, KubernetesClient client) { - this.identity = identity(config); + this.identity = config.getIdentity(); Lock lock = new LeaseLock(config.getLeaseNamespace(), config.getLeaseName(), identity); // releaseOnCancel is not used in the underlying implementation leaderElector = new LeaderElectorBuilder(client, ConfigurationServiceProvider.instance().getExecutorService()) - .withConfig( - new LeaderElectionConfig(lock, config.getLeaseDuration(), config.getRenewDeadline(), - config.getRetryPeriod(), leaderCallbacks(), true, config.getLeaseName())) - .build(); + .withConfig( + new LeaderElectionConfig(lock, config.getLeaseDuration(), config.getRenewDeadline(), + config.getRetryPeriod(), leaderCallbacks(), true, config.getLeaseName())) + .build(); } public boolean isLeaderElectionOn() { @@ -46,9 +45,8 @@ public boolean isLeaderElectionOn() { } private LeaderCallbacks leaderCallbacks() { - return new LeaderCallbacks(this::startLeading, this::stopLeading, leader -> { - log.info("New leader with identity: {}", leader); - }); + return new LeaderCallbacks(this::startLeading, this::stopLeading, + leader -> log.info("New leader with identity: {}", leader)); } private void startLeading() { @@ -63,14 +61,6 @@ private void stopLeading() { System.exit(1); } - private String identity(LeaderElectionConfiguration config) { - String id = config.getIdentity().orElse(System.getenv("HOSTNAME")); - if (id == null || id.isBlank()) { - id = UUID.randomUUID().toString(); - } - return id; - } - public void start() { if (isLeaderElectionOn()) { leaderElectionFuture = leaderElector.start(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java index 67764be655..15691f28bf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java @@ -1,7 +1,9 @@ package io.javaoperatorsdk.operator.api.config; import java.time.Duration; -import java.util.Optional; +import java.util.UUID; + +import io.fabric8.zjsonpatch.internal.guava.Strings; public class LeaderElectionConfiguration { @@ -11,7 +13,7 @@ public class LeaderElectionConfiguration { private final String leaseName; private final String leaseNamespace; - private final String identity; + private String identity; private final Duration leaseDuration; private final Duration renewDeadline; @@ -70,7 +72,18 @@ public Duration getRetryPeriod() { return retryPeriod; } - public Optional getIdentity() { - return Optional.ofNullable(identity); + public String getIdentity() { + if (identity == null) { + identity = System.getenv("HOSTNAME"); + if (Strings.isNullOrEmpty(identity)) { + identity = UUID.randomUUID().toString(); + } else { + identity = identity.trim(); + if (identity.isBlank()) { + identity = UUID.randomUUID().toString(); + } + } + } + return identity; } } From b09bf5c4547e1baad7ff3ae30838899fa14c2f5a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 23 Aug 2022 13:17:28 +0200 Subject: [PATCH 22/31] refactor: isLeaderElectionOn -> isLeaderElectionEnabled --- .../io/javaoperatorsdk/operator/LeaderElectionManager.java | 6 +++--- .../src/main/java/io/javaoperatorsdk/operator/Operator.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index eb5d540d7c..fc7b734f41 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -40,7 +40,7 @@ public void init(LeaderElectionConfiguration config, KubernetesClient client) { .build(); } - public boolean isLeaderElectionOn() { + public boolean isLeaderElectionEnabled() { return leaderElector != null; } @@ -57,12 +57,12 @@ private void stopLeading() { log.info("Stopped leading for identity: {}. Exiting.", identity); // When leader stops leading the process ends immediately to prevent multiple reconciliations // running parallel. - // Note that some reconciliations might run a very long time. + // Note that some reconciliations might run for a very long time. System.exit(1); } public void start() { - if (isLeaderElectionOn()) { + if (isLeaderElectionEnabled()) { leaderElectionFuture = leaderElector.start(); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 14f273b7b4..6548e9500a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -99,7 +99,7 @@ public void start() { ExecutorServiceManager.init(); // first start the controller manager before leader election, // the leader election would start subsequently the processor if on - controllerManager.start(!leaderElectionManager.isLeaderElectionOn()); + controllerManager.start(!leaderElectionManager.isLeaderElectionEnabled()); leaderElectionManager.start(); } catch (Exception e) { log.error("Error starting operator", e); From 951d28dc25f5e110686be832924f0317c962a6be Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 23 Aug 2022 15:51:42 +0200 Subject: [PATCH 23/31] refactor: restore LifecycleAware compatibility --- .../javaoperatorsdk/operator/LeaderElectionManager.java | 8 ++++---- .../javaoperatorsdk/operator/processing/Controller.java | 6 +++++- .../operator/processing/ControllerTest.java | 4 ++-- .../processing/event/ReconciliationDispatcherTest.java | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index fc7b734f41..98e0b3502f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -34,10 +34,10 @@ public void init(LeaderElectionConfiguration config, KubernetesClient client) { // releaseOnCancel is not used in the underlying implementation leaderElector = new LeaderElectorBuilder(client, ConfigurationServiceProvider.instance().getExecutorService()) - .withConfig( - new LeaderElectionConfig(lock, config.getLeaseDuration(), config.getRenewDeadline(), - config.getRetryPeriod(), leaderCallbacks(), true, config.getLeaseName())) - .build(); + .withConfig( + new LeaderElectionConfig(lock, config.getLeaseDuration(), config.getRenewDeadline(), + config.getRetryPeriod(), leaderCallbacks(), true, config.getLeaseName())) + .build(); } public boolean isLeaderElectionEnabled() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 3e4bf1fdb3..92b70e722d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -45,7 +45,7 @@ @SuppressWarnings({"unchecked", "rawtypes"}) @Ignore public class Controller

- implements Reconciler

, Cleaner

, + implements Reconciler

, LifecycleAware, Cleaner

, RegisteredController

{ private static final Logger log = LoggerFactory.getLogger(Controller.class); @@ -263,6 +263,10 @@ public MixedOperation, Resource

> getCRClient() { return kubernetesClient.resources(configuration.getResourceClass()); } + public void start() throws OperatorException { + start(true); + } + /** * Registers the specified controller with this operator, overriding its default configuration by * the specified one (usually created via diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 95504f504b..67e9112b27 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -24,7 +24,7 @@ void crdShouldNotBeCheckedForNativeResources() { final var configuration = MockControllerConfiguration.forResource(Secret.class); final var controller = new Controller(reconciler, configuration, client); - controller.start(true); + controller.start(); verify(client, never()).apiextensions(); } @@ -36,7 +36,7 @@ void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { try { ConfigurationServiceProvider.overrideCurrent(o -> o.checkingCRDAndValidateLocalModel(false)); final var controller = new Controller(reconciler, configuration, client); - controller.start(true); + controller.start(); verify(client, never()).apiextensions(); } finally { ConfigurationServiceProvider.reset(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index dfa01783f9..89dafa9fc4 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -121,7 +121,7 @@ public boolean useFinalizer() { return useFinalizer; } }; - controller.start(true); + controller.start(); return new ReconciliationDispatcher<>(controller, customResourceFacade); } From a2b79d8bb3fb74eac68ee5682c99cdc692e7af17 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 24 Aug 2022 12:14:55 +0200 Subject: [PATCH 24/31] Revert "refactor: move identity generation to configuration" This reverts commit 93a260a7a2ee0d53b1289b70101b168b75969c70. --- .../operator/LeaderElectionManager.java | 18 ++++++++++++---- .../config/LeaderElectionConfiguration.java | 21 ++++--------------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index 98e0b3502f..94d201dce9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator; +import java.util.UUID; import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; @@ -15,7 +16,7 @@ import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; -class LeaderElectionManager { +public class LeaderElectionManager { private static final Logger log = LoggerFactory.getLogger(LeaderElectionManager.class); @@ -29,7 +30,7 @@ public LeaderElectionManager(ControllerManager controllerManager) { } public void init(LeaderElectionConfiguration config, KubernetesClient client) { - this.identity = config.getIdentity(); + this.identity = identity(config); Lock lock = new LeaseLock(config.getLeaseNamespace(), config.getLeaseName(), identity); // releaseOnCancel is not used in the underlying implementation leaderElector = new LeaderElectorBuilder(client, @@ -45,8 +46,9 @@ public boolean isLeaderElectionEnabled() { } private LeaderCallbacks leaderCallbacks() { - return new LeaderCallbacks(this::startLeading, this::stopLeading, - leader -> log.info("New leader with identity: {}", leader)); + return new LeaderCallbacks(this::startLeading, this::stopLeading, leader -> { + log.info("New leader with identity: {}", leader); + }); } private void startLeading() { @@ -61,6 +63,14 @@ private void stopLeading() { System.exit(1); } + private String identity(LeaderElectionConfiguration config) { + String id = config.getIdentity().orElse(System.getenv("HOSTNAME")); + if (id == null || id.isBlank()) { + id = UUID.randomUUID().toString(); + } + return id; + } + public void start() { if (isLeaderElectionEnabled()) { leaderElectionFuture = leaderElector.start(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java index 15691f28bf..67764be655 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java @@ -1,9 +1,7 @@ package io.javaoperatorsdk.operator.api.config; import java.time.Duration; -import java.util.UUID; - -import io.fabric8.zjsonpatch.internal.guava.Strings; +import java.util.Optional; public class LeaderElectionConfiguration { @@ -13,7 +11,7 @@ public class LeaderElectionConfiguration { private final String leaseName; private final String leaseNamespace; - private String identity; + private final String identity; private final Duration leaseDuration; private final Duration renewDeadline; @@ -72,18 +70,7 @@ public Duration getRetryPeriod() { return retryPeriod; } - public String getIdentity() { - if (identity == null) { - identity = System.getenv("HOSTNAME"); - if (Strings.isNullOrEmpty(identity)) { - identity = UUID.randomUUID().toString(); - } else { - identity = identity.trim(); - if (identity.isBlank()) { - identity = UUID.randomUUID().toString(); - } - } - } - return identity; + public Optional getIdentity() { + return Optional.ofNullable(identity); } } From df6f2ef604ea936a9118c1fa9bbdd7c1f5227eba Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 24 Aug 2022 12:21:35 +0200 Subject: [PATCH 25/31] removed builder --- .../LeaderElectionConfigurationBuilder.java | 56 ------------------- .../operator/sample/LeaderElectionE2E.java | 2 +- 2 files changed, 1 insertion(+), 57 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java deleted file mode 100644 index 979d9cb50c..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java +++ /dev/null @@ -1,56 +0,0 @@ -package io.javaoperatorsdk.operator.api.config; - -import java.time.Duration; - -import static io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration.*; - -public final class LeaderElectionConfigurationBuilder { - - private String leaseName; - private String leaseNamespace; - private Duration leaseDuration = LEASE_DURATION_DEFAULT_VALUE; - private Duration renewDeadline = RENEW_DEADLINE_DEFAULT_VALUE; - private Duration retryPeriod = RETRY_PERIOD_DEFAULT_VALUE; - private String identity; - - public LeaderElectionConfigurationBuilder() {} - - public static LeaderElectionConfigurationBuilder aLeaderElectionConfiguration() { - return new LeaderElectionConfigurationBuilder(); - } - - public LeaderElectionConfigurationBuilder withLeaseName(String leaseName) { - this.leaseName = leaseName; - return this; - } - - public LeaderElectionConfigurationBuilder withLeaseNamespace(String leaseNamespace) { - this.leaseNamespace = leaseNamespace; - return this; - } - - public LeaderElectionConfigurationBuilder withLeaseDuration(Duration leaseDuration) { - this.leaseDuration = leaseDuration; - return this; - } - - public LeaderElectionConfigurationBuilder withRenewDeadline(Duration renewDeadline) { - this.renewDeadline = renewDeadline; - return this; - } - - public LeaderElectionConfigurationBuilder withRetryPeriod(Duration retryPeriod) { - this.retryPeriod = retryPeriod; - return this; - } - - public LeaderElectionConfigurationBuilder withIdentity(String identity) { - this.identity = identity; - return this; - } - - public LeaderElectionConfiguration build() { - return new LeaderElectionConfiguration(leaseName, leaseNamespace, leaseDuration, renewDeadline, - retryPeriod, identity); - } -} diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 243ccd8277..f63905ac05 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -47,7 +47,7 @@ class LeaderElectionE2E { @Test // not for local mode by design - @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") +// @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") void otherInstancesTakesOverWhenSteppingDown() { log.info("Deploying operator"); deployOperatorsInOrder(); From e7a80ee40ea91bfb9386c2639328bfa1000016a5 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 24 Aug 2022 12:28:39 +0200 Subject: [PATCH 26/31] no builder in e2e test --- .../api/config/LeaderElectionConfiguration.java | 9 +++++++++ .../operator/sample/LeaderElectionTestOperator.java | 10 ++++------ .../operator/sample/LeaderElectionE2E.java | 3 +-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java index 67764be655..5146fa6a1e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java @@ -17,6 +17,15 @@ public class LeaderElectionConfiguration { private final Duration renewDeadline; private final Duration retryPeriod; + public LeaderElectionConfiguration(String leaseName, String leaseNamespace, String identity) { + this( + leaseName, + leaseNamespace, + LEASE_DURATION_DEFAULT_VALUE, + RENEW_DEADLINE_DEFAULT_VALUE, + RETRY_PERIOD_DEFAULT_VALUE, identity); + } + public LeaderElectionConfiguration(String leaseName, String leaseNamespace) { this( leaseName, diff --git a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java index 32dd9ebefd..47a5a0ed59 100644 --- a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java @@ -6,7 +6,7 @@ import io.fabric8.kubernetes.client.ConfigBuilder; import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.javaoperatorsdk.operator.Operator; -import io.javaoperatorsdk.operator.api.config.LeaderElectionConfigurationBuilder; +import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; public class LeaderElectionTestOperator { @@ -23,11 +23,9 @@ public static void main(String[] args) { .build()).build(); Operator operator = new Operator(client, - c -> c.withLeaderElectionConfiguration(new LeaderElectionConfigurationBuilder() - .withLeaseName("leader-election-test") - .withLeaseNamespace(namespace) - .withIdentity(identity) - .build())); + c -> c.withLeaderElectionConfiguration( + new LeaderElectionConfiguration("leader-election-test", namespace, identity))); + operator.register(new LeaderElectionTestReconciler(identity)); operator.installShutdownHook(); operator.start(); diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index f63905ac05..03802a2244 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -14,7 +14,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.EnabledIfSystemProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,7 +46,7 @@ class LeaderElectionE2E { @Test // not for local mode by design -// @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") + // @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") void otherInstancesTakesOverWhenSteppingDown() { log.info("Deploying operator"); deployOperatorsInOrder(); From f8e221143173857ea04e5db73e1e6bd5c5a8be80 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 24 Aug 2022 12:38:33 +0200 Subject: [PATCH 27/31] put back test constraint --- .../io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 03802a2244..243ccd8277 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -14,6 +14,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfSystemProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -46,7 +47,7 @@ class LeaderElectionE2E { @Test // not for local mode by design - // @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") + @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") void otherInstancesTakesOverWhenSteppingDown() { log.info("Deploying operator"); deployOperatorsInOrder(); From 5dbef6511da805a0cd72fd54e67a911d9326dbc0 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 24 Aug 2022 13:42:19 +0200 Subject: [PATCH 28/31] improve expressiveness on constat --- .../javaoperatorsdk/operator/sample/LeaderElectionE2E.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 243ccd8277..af29f6ed92 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -41,6 +41,7 @@ class LeaderElectionE2E { private static final String OPERATOR_1_POD_NAME = "leader-election-operator-1"; private static final String OPERATOR_2_POD_NAME = "leader-election-operator-2"; + public static final int MINIMAL_EXPECTED_RECONCILIATION = 2; private String namespace; private KubernetesClient client; @@ -62,7 +63,8 @@ void otherInstancesTakesOverWhenSteppingDown() { .inNamespace(namespace).withName(TEST_RESOURCE_NAME).get().getStatus(); assertThat(actualStatus).isNotNull(); - assertThat(actualStatus.getReconciledBy()).hasSizeGreaterThan(2); + assertThat(actualStatus.getReconciledBy()) + .hasSizeGreaterThan(MINIMAL_EXPECTED_RECONCILIATION); }); client.pods().inNamespace(namespace).withName(OPERATOR_1_POD_NAME).delete(); @@ -78,7 +80,8 @@ void otherInstancesTakesOverWhenSteppingDown() { .inNamespace(namespace).withName(TEST_RESOURCE_NAME).get().getStatus(); assertThat(actualStatus).isNotNull(); - assertThat(actualStatus.getReconciledBy()).hasSizeGreaterThan(actualListSize + 2); + assertThat(actualStatus.getReconciledBy()) + .hasSizeGreaterThan(actualListSize + MINIMAL_EXPECTED_RECONCILIATION); }); assertReconciliations( From aec94c2c461456458e878b37346f6f64cf998cb9 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 24 Aug 2022 14:35:56 +0200 Subject: [PATCH 29/31] update stat with optimistic locking --- .../operator/sample/LeaderElectionTestReconciler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java index f4681c677a..f6231d0a52 100644 --- a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestReconciler.java @@ -27,8 +27,8 @@ public UpdateControl reconcile( } resource.getStatus().getReconciledBy().add(reconcilerName); - - return UpdateControl.patchStatus(resource).rescheduleAfter(Duration.ofSeconds(1)); + // update status is with optimistic locking + return UpdateControl.updateStatus(resource).rescheduleAfter(Duration.ofSeconds(1)); } } From e58457f95a52874e0aeac128117ab13caf088a70 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 24 Aug 2022 15:52:32 +0200 Subject: [PATCH 30/31] fix config override sequence issue --- .../io/javaoperatorsdk/operator/Operator.java | 17 ++++++++++++----- .../operator/sample/LeaderElectionE2E.java | 5 ++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 6548e9500a..57faef696a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -31,7 +31,7 @@ public Operator() { } public Operator(KubernetesClient kubernetesClient) { - this(kubernetesClient, ConfigurationServiceProvider.instance()); + this(kubernetesClient, ConfigurationServiceProvider.instance(), null); } /** @@ -39,7 +39,7 @@ public Operator(KubernetesClient kubernetesClient) { */ @Deprecated public Operator(ConfigurationService configurationService) { - this(null, configurationService); + this(null, configurationService, null); } public Operator(Consumer overrider) { @@ -47,8 +47,7 @@ public Operator(Consumer overrider) { } public Operator(KubernetesClient client, Consumer overrider) { - this(client); - ConfigurationServiceProvider.overrideCurrent(overrider); + this(client, ConfigurationServiceProvider.instance(), overrider); } /** @@ -59,10 +58,18 @@ public Operator(KubernetesClient client, Consumer * @param configurationService provides configuration */ public Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService) { + this(kubernetesClient, configurationService, null); + } + + private Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService, + Consumer overrider) { this.kubernetesClient = kubernetesClient != null ? kubernetesClient : new KubernetesClientBuilder().build(); ConfigurationServiceProvider.set(configurationService); - configurationService.getLeaderElectionConfiguration() + if (overrider != null) { + ConfigurationServiceProvider.overrideCurrent(overrider); + } + ConfigurationServiceProvider.instance().getLeaderElectionConfiguration() .ifPresent(c -> leaderElectionManager.init(c, this.kubernetesClient)); } diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index af29f6ed92..9d1080ad1f 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -14,7 +14,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.EnabledIfSystemProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,14 +40,14 @@ class LeaderElectionE2E { private static final String OPERATOR_1_POD_NAME = "leader-election-operator-1"; private static final String OPERATOR_2_POD_NAME = "leader-election-operator-2"; - public static final int MINIMAL_EXPECTED_RECONCILIATION = 2; + public static final int MINIMAL_EXPECTED_RECONCILIATION = 3; private String namespace; private KubernetesClient client; @Test // not for local mode by design - @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") + // @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") void otherInstancesTakesOverWhenSteppingDown() { log.info("Deploying operator"); deployOperatorsInOrder(); From 24e2e3f16f9cc9c8e398a14eb88d60249abeb63d Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 24 Aug 2022 16:15:07 +0200 Subject: [PATCH 31/31] added back test run condition --- .../io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 9d1080ad1f..60b3ad639a 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -14,6 +14,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfSystemProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,7 +48,7 @@ class LeaderElectionE2E { @Test // not for local mode by design - // @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") + @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") void otherInstancesTakesOverWhenSteppingDown() { log.info("Deploying operator"); deployOperatorsInOrder();