Skip to content

Commit

Permalink
refactor: remove now unneeded KubernetesClientAware (#2084)
Browse files Browse the repository at this point in the history
  • Loading branch information
metacosm committed Oct 18, 2023
1 parent cb18124 commit ec48dec
Show file tree
Hide file tree
Showing 30 changed files with 80 additions and 355 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.fabric8.kubernetes.api.model.*;
import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.*;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.junit.KubernetesClientAware;
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
import io.javaoperatorsdk.operator.processing.event.source.cache.BoundedItemStore;
import io.javaoperatorsdk.operator.processing.event.source.cache.CaffeineBoundedItemStores;
Expand All @@ -27,38 +28,35 @@
import com.github.benmanes.caffeine.cache.Caffeine;

public abstract class AbstractTestReconciler<P extends CustomResource<BoundedCacheTestSpec, BoundedCacheTestStatus>>
implements KubernetesClientAware, Reconciler<P>,
EventSourceInitializer<P> {
implements Reconciler<P>, EventSourceInitializer<P> {

private static final Logger log =
LoggerFactory.getLogger(BoundedCacheClusterScopeTestReconciler.class);

public static final String DATA_KEY = "dataKey";

protected KubernetesClient client;

@Override
public UpdateControl<P> reconcile(
P resource,
Context<P> context) {
var maybeConfigMap = context.getSecondaryResource(ConfigMap.class);
maybeConfigMap.ifPresentOrElse(
cm -> updateConfigMapIfNeeded(cm, resource),
() -> createConfigMap(resource));
cm -> updateConfigMapIfNeeded(cm, resource, context),
() -> createConfigMap(resource, context));
ensureStatus(resource);
log.info("Reconciled: {}", resource.getMetadata().getName());
return UpdateControl.patchStatus(resource);
}

protected void updateConfigMapIfNeeded(ConfigMap cm, P resource) {
protected void updateConfigMapIfNeeded(ConfigMap cm, P resource, Context<P> context) {
var data = cm.getData().get(DATA_KEY);
if (data == null || data.equals(resource.getSpec().getData())) {
cm.setData(Map.of(DATA_KEY, resource.getSpec().getData()));
client.configMaps().resource(cm).replace();
context.getClient().configMaps().resource(cm).replace();
}
}

protected void createConfigMap(P resource) {
protected void createConfigMap(P resource, Context<P> context) {
var cm = new ConfigMapBuilder()
.withMetadata(new ObjectMetaBuilder()
.withName(resource.getMetadata().getName())
Expand All @@ -67,17 +65,7 @@ protected void createConfigMap(P resource) {
.withData(Map.of(DATA_KEY, resource.getSpec().getData()))
.build();
cm.addOwnerReference(resource);
client.configMaps().resource(cm).create();
}

@Override
public KubernetesClient getKubernetesClient() {
return client;
}

@Override
public void setKubernetesClient(KubernetesClient kubernetesClient) {
this.client = kubernetesClient;
context.getClient().configMaps().resource(cm).create();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package io.javaoperatorsdk.operator.api.reconciler.dependent.managed;

import io.fabric8.kubernetes.client.KubernetesClient;
import io.javaoperatorsdk.operator.api.reconciler.Context;

/**
* @deprecated It shouldn't be needed to pass a {@link KubernetesClient} instance anymore as the
* client should be accessed via {@link Context#getClient()} instead.
*/
@Deprecated(since = "4.5.0", forRemoval = true)
public interface KubernetesClientAware {
void setKubernetesClient(KubernetesClient kubernetesClient);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package io.javaoperatorsdk.operator.processing.dependent;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller;
import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
Expand All @@ -18,7 +17,6 @@ public abstract class AbstractExternalDependentResource<R, P extends HasMetadata
@SuppressWarnings("rawtypes")
private DependentResourceWithExplicitState dependentResourceWithExplicitState;
private InformerEventSource<?, P> externalStateEventSource;
private KubernetesClient kubernetesClient;

@SuppressWarnings("unchecked")
protected AbstractExternalDependentResource(Class<R> resourceType) {
Expand Down Expand Up @@ -65,14 +63,13 @@ public void delete(P primary, Context<P> context) {
@SuppressWarnings({"unchecked", "unused"})
private void handleExplicitStateDelete(P primary, R secondary, Context<P> context) {
var res = dependentResourceWithExplicitState.stateResource(primary, secondary);
dependentResourceWithExplicitState.getKubernetesClient().resource(res).delete();
context.getClient().resource(res).delete();
}

@SuppressWarnings({"rawtypes", "unchecked", "unused"})
protected void handleExplicitStateCreation(P primary, R created, Context<P> context) {
var resource = dependentResourceWithExplicitState.stateResource(primary, created);
var stateResource =
dependentResourceWithExplicitState.getKubernetesClient().resource(resource).create();
var stateResource = context.getClient().resource(resource).create();
if (externalStateEventSource != null) {
((RecentOperationCacheFiller) externalStateEventSource)
.handleRecentResourceCreate(ResourceID.fromResource(primary), stateResource);
Expand All @@ -84,7 +81,7 @@ protected void handleExplicitStateCreation(P primary, R created, Context<P> cont
public void deleteTargetResource(P primary, R resource, String key,
Context<P> context) {
if (isDependentResourceWithExplicitState) {
getKubernetesClient()
context.getClient()
.resource(dependentResourceWithExplicitState.stateResource(primary, resource))
.delete();
}
Expand All @@ -100,18 +97,4 @@ public void handleDeleteTargetResource(P primary, R resource, String key,
protected InformerEventSource getExternalStateEventSource() {
return externalStateEventSource;
}

/**
* It's here just to manage the explicit state resource in case the dependent resource implements
* {@link RecentOperationCacheFiller}.
*
* @return kubernetes client.
*/
public KubernetesClient getKubernetesClient() {
return kubernetesClient;
}

public void setKubernetesClient(KubernetesClient kubernetesClient) {
this.kubernetesClient = kubernetesClient;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware;

/**
* Handles external resources where in order to address the resource additional information or
Expand All @@ -14,7 +13,7 @@
* for a resource that extends {@link AbstractExternalDependentResource}.
*/
public interface DependentResourceWithExplicitState<R, P extends HasMetadata, S extends HasMetadata>
extends Creator<R, P>, Deleter<P>, KubernetesClientAware {
extends Creator<R, P>, Deleter<P> {

/**
* Only needs to be implemented if multiple event sources are present for the target resource
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import org.slf4j.LoggerFactory;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.dsl.Resource;
import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.api.config.dependent.Configured;
Expand All @@ -19,7 +18,6 @@
import io.javaoperatorsdk.operator.api.reconciler.Ignore;
import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware;
import io.javaoperatorsdk.operator.processing.dependent.AbstractEventSourceHolderDependentResource;
import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher.GenericResourceUpdaterMatcher;
Expand All @@ -33,17 +31,14 @@
converter = KubernetesDependentConverter.class)
public abstract class KubernetesDependentResource<R extends HasMetadata, P extends HasMetadata>
extends AbstractEventSourceHolderDependentResource<R, P, InformerEventSource<R, P>>
implements KubernetesClientAware,
DependentResourceConfigurator<KubernetesDependentResourceConfig<R>> {
implements DependentResourceConfigurator<KubernetesDependentResourceConfig<R>> {

private static final Logger log = LoggerFactory.getLogger(KubernetesDependentResource.class);

protected KubernetesClient client;
private final ResourceUpdaterMatcher<R> updaterMatcher;
private final boolean garbageCollected = this instanceof GarbageCollected;
private KubernetesDependentResourceConfig<R> kubernetesDependentResourceConfig;

private boolean usingCustomResourceUpdateMatcher;
private final boolean usingCustomResourceUpdateMatcher;

@SuppressWarnings("unchecked")
public KubernetesDependentResource(Class<R> resourceType) {
Expand Down Expand Up @@ -117,7 +112,7 @@ public R create(R desired, P primary, Context<P> context) {
}
}
addMetadata(false, null, desired, primary, context);
final var resource = prepare(desired, primary, "Creating");
final var resource = prepare(context, desired, primary, "Creating");
return useSSA(context)
? resource
.fieldManager(context.getControllerConfiguration().fieldManager())
Expand All @@ -134,12 +129,12 @@ public R update(R actual, R desired, P primary, Context<P> context) {
R updatedResource;
addMetadata(false, actual, desired, primary, context);
if (useSSA(context)) {
updatedResource = prepare(desired, primary, "Updating")
updatedResource = prepare(context, desired, primary, "Updating")
.fieldManager(context.getControllerConfiguration().fieldManager())
.forceConflicts().serverSideApply();
} else {
var updatedActual = updaterMatcher.updateResource(actual, desired, context);
updatedResource = prepare(updatedActual, primary, "Updating").update();
updatedResource = prepare(context, updatedActual, primary, "Updating").update();
}
log.debug("Resource version after update: {}",
updatedResource.getMetadata().getResourceVersion());
Expand Down Expand Up @@ -216,23 +211,23 @@ private boolean usePreviousAnnotation(Context<P> context) {
@Override
protected void handleDelete(P primary, R secondary, Context<P> context) {
if (secondary != null) {
client.resource(secondary).delete();
context.getClient().resource(secondary).delete();
}
}

@SuppressWarnings("unused")
public void deleteTargetResource(P primary, R resource, String key, Context<P> context) {
client.resource(resource).delete();
context.getClient().resource(resource).delete();
}

@SuppressWarnings("unused")
protected Resource<R> prepare(R desired, P primary, String actionName) {
protected Resource<R> prepare(Context<P> context, R desired, P primary, String actionName) {
log.debug("{} target resource with type: {}, with id: {}",
actionName,
desired.getClass(),
ResourceID.fromResource(desired));

return client.resource(desired);
return context.getClient().resource(desired);
}

protected void addReferenceHandlingMetadata(R desired, P primary) {
Expand Down Expand Up @@ -292,16 +287,6 @@ protected boolean addOwnerReference() {
return garbageCollected;
}

@Override
public void setKubernetesClient(KubernetesClient kubernetesClient) {
this.client = kubernetesClient;
}

@Override
public KubernetesClient getKubernetesClient() {
return client;
}

@Override
protected R desired(P primary, Context<P> context) {
return super.desired(primary, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public abstract class ManagedInformerEventSource<R extends HasMetadata, P extend

private static final Logger log = LoggerFactory.getLogger(ManagedInformerEventSource.class);
private final InformerManager<R, C> cache;

protected TemporaryResourceCache<R> temporaryResourceCache;
protected MixedOperation<R, KubernetesResourceList<R>, Resource<R>> client;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package io.javaoperatorsdk.operator.junit;

import io.fabric8.kubernetes.client.KubernetesClient;
import io.javaoperatorsdk.operator.api.reconciler.Context;

/**
* @deprecated It shouldn't be needed to pass a {@link KubernetesClient} instance to the reconciler
* anymore as the client should be accessed via {@link Context#getClient()} instead.
*/
@Deprecated(since = "4.5.0", forRemoval = true)
public interface KubernetesClientAware extends HasKubernetesClient {
void setKubernetesClient(KubernetesClient kubernetesClient);
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,6 @@ protected void before(ExtensionContext context) {
applyCrd(config.getResourceTypeName());
}

if (ref.reconciler instanceof KubernetesClientAware) {
((KubernetesClientAware) ref.reconciler).setKubernetesClient(kubernetesClient);
}

var registeredController = this.operator.register(ref.reconciler, oconfig.build());
registeredControllers.put(ref.reconciler, registeredController);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ private static void startEndStopOperator(KubernetesClient client,
Operator o1 = new Operator(o -> o
.withCloseClientOnStop(false)
.withKubernetesClient(client));
o1.register(new DependentReInitializationReconciler(dependent, client));
o1.register(new DependentReInitializationReconciler(dependent));
o1.start();
o1.stop();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,9 @@ private DependnetSSACustomResource reconcileWithLegacyOperator(Operator legacyOp

private Operator createOperator(KubernetesClient client, boolean legacyDependentHandling,
String fieldManager) {
Operator operator = new Operator(client,
o -> o.withCloseClientOnStop(false));
Operator operator =
new Operator(o -> o.withKubernetesClient(client).withCloseClientOnStop(false));
var reconciler = new DependentSSAReconciler(!legacyDependentHandling);
reconciler.setKubernetesClient(client);
operator.register(reconciler, o -> {
o.settingNamespace(namespace);
if (fieldManager != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
package io.javaoperatorsdk.operator;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.*;

import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
import io.javaoperatorsdk.operator.sample.restart.RestartTestCustomResource;
import io.javaoperatorsdk.operator.sample.restart.RestartTestReconciler;
Expand All @@ -17,14 +11,15 @@
import static org.awaitility.Awaitility.await;

class OperatorRestartIT {
private final static KubernetesClient client = new KubernetesClientBuilder().build();

private final static Operator operator = new Operator(o -> o.withCloseClientOnStop(false));
private final static RestartTestReconciler reconciler = new RestartTestReconciler();
private static int reconcileNumberBeforeStop = 0;

@BeforeAll
static void registerReconciler() {
LocallyRunOperatorExtension.applyCrd(RestartTestCustomResource.class, client);
LocallyRunOperatorExtension.applyCrd(RestartTestCustomResource.class,
operator.getKubernetesClient());
operator.register(reconciler);
}

Expand All @@ -41,7 +36,7 @@ void stopOperator() {
@Test
@Order(1)
void createResource() {
client.resource(testCustomResource()).createOrReplace();
operator.getKubernetesClient().resource(testCustomResource()).createOrReplace();
await().untilAsserted(() -> assertThat(reconciler.getNumberOfExecutions()).isGreaterThan(0));
reconcileNumberBeforeStop = reconciler.getNumberOfExecutions();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public ConfigMap desired(BulkDependentTestCustomResource primary, String key,
.withLabels(Map.of(LABEL_KEY, LABEL_VALUE))
.build());
configMap.setData(
Map.of("number", "" + key, ADDITIONAL_DATA_KEY, primary.getSpec().getAdditionalData()));
Map.of("number", key, ADDITIONAL_DATA_KEY, primary.getSpec().getAdditionalData()));
return configMap;
}

Expand Down
Loading

0 comments on commit ec48dec

Please sign in to comment.