Skip to content

refactor: remove now unneeded KubernetesClientAware #2084

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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