Skip to content

Commit

Permalink
fix: avoid computing potentially unused values (#1759)
Browse files Browse the repository at this point in the history
Fixes #1758
  • Loading branch information
metacosm authored Feb 9, 2023
1 parent 57a41b1 commit 5d8c567
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ public class MDCUtils {
private static final String RESOURCE_VERSION = "resource.resourceVersion";
private static final String GENERATION = "resource.generation";
private static final String UID = "resource.uid";
private static final String NO_NAMESPACE = "no namespace";

public static void addResourceIDInfo(ResourceID resourceID) {
MDC.put(NAME, resourceID.getName());
MDC.put(NAMESPACE, resourceID.getNamespace().orElse("no namespace"));
MDC.put(NAMESPACE, resourceID.getNamespace().orElse(NO_NAMESPACE));
}

public static void removeResourceIDInfo() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ protected ReconcileResult<R> reconcile(P primary, R actualResource, Context<P> c
if (updatable) {
final Matcher.Result<R> match = match(actualResource, primary, context);
if (!match.matched()) {
final var desired = match.computedDesired().orElse(desired(primary, context));
final var desired = match.computedDesired().orElseGet(() -> desired(primary, context));
throwIfNull(desired, primary, "Desired");
logForOperation("Updating", primary, desired);
var updatedResource = handleUpdate(actualResource, desired, primary, context);
Expand Down Expand Up @@ -125,7 +125,7 @@ protected R handleCreate(R desired, P primary, Context<P> context) {
* @param primary the {@link ResourceID} of the primary resource associated with the newly created
* resource
* @param created the newly created resource
* @param context
* @param context the context in which this operation is called
*/
protected abstract void onCreated(P primary, R created, Context<P> context);

Expand All @@ -136,7 +136,7 @@ protected R handleCreate(R desired, P primary, Context<P> context) {
* resource
* @param updated the updated resource
* @param actual the resource as it was before the update
* @param context
* @param context the context in which this operation is called
*/
protected abstract void onUpdated(P primary, R updated, R actual, Context<P> context);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ private EventProcessor(
.map(c -> Map.of(
Constants.RESOURCE_GVK_KEY, c.getAssociatedGroupVersionKind(),
Constants.CONTROLLER_NAME, controllerConfiguration.getName()))
.orElse(new HashMap<>());
.orElseGet(HashMap::new);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,30 @@ public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromAnnotation
return fromMetadata(nameKey, null, false);
}

@SuppressWarnings("unused")
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromAnnotation(
String nameKey, String namespaceKey) {
return fromMetadata(nameKey, namespaceKey, false);
}

public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(
String nameKey) {
@SuppressWarnings("unused")
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(String nameKey) {
return fromMetadata(nameKey, null, true);
}

public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromDefaultAnnotations() {
return fromMetadata(DEFAULT_ANNOTATION_FOR_NAME, DEFAULT_ANNOTATION_FOR_NAMESPACE, false);
}

@SuppressWarnings("unused")
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(
String nameKey, String namespaceKey) {
return fromMetadata(nameKey, namespaceKey, true);
}

public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReference() {
return resource -> ResourceID.fromFirstOwnerReference(resource).map(Set::of)
.orElse(Collections.emptySet());
.orElseGet(Collections::emptySet);
}

private static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromMetadata(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.*;
import io.javaoperatorsdk.operator.api.reconciler.Cleaner;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
import io.javaoperatorsdk.operator.junit.KubernetesClientAware;
import io.javaoperatorsdk.operator.processing.event.ResourceID;
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
Expand Down Expand Up @@ -89,9 +96,7 @@ private void createExternalResource(ExternalStateCustomResource resource) {
public DeleteControl cleanup(ExternalStateCustomResource resource,
Context<ExternalStateCustomResource> context) {
var externalResource = context.getSecondaryResource(ExternalResource.class);
externalResource.ifPresent(er -> {
externalService.delete(er.getId());
});
externalResource.ifPresent(er -> externalService.delete(er.getId()));
client.configMaps().inNamespace(resource.getMetadata().getNamespace())
.withName(resource.getMetadata().getName()).delete();
return DeleteControl.defaultDelete();
Expand All @@ -116,7 +121,7 @@ public Map<String, EventSource> prepareEventSources(
}
var id = configMap.getData().get(ID_KEY);
var externalResource = externalService.read(id);
return externalResource.map(Set::of).orElse(Collections.emptySet());
return externalResource.map(Set::of).orElseGet(Collections::emptySet);
}, context.getPrimaryCache(), 300L, ExternalResource.class);

return EventSourceInitializer.nameEventSources(configMapEventSource,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ public Set<ExternalResource> fetchResources(
return configMapOptional.map(configMap -> {
var id = configMap.getData().get(ID_KEY);
var externalResource = externalService.read(id);
return externalResource.map(Set::of).orElse(Collections.emptySet());
}).orElse(Collections.emptySet());
return externalResource.map(Set::of).orElseGet(Collections::emptySet);
}).orElseGet(Collections::emptySet);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public static String decode(String value) {
public Set<Schema> fetchResources(MySQLSchema primaryResource) {
try (Connection connection = getConnection()) {
return SchemaService.getSchema(connection, primaryResource.getMetadata().getName())
.map(Set::of).orElse(Collections.emptySet());
.map(Set::of).orElseGet(Collections::emptySet);
} catch (SQLException e) {
throw new RuntimeException("Error while trying read Schema", e);
}
Expand All @@ -123,7 +123,7 @@ public ResourcePollerConfig configFrom(SchemaConfig configAnnotation,
Class<SchemaDependentResource> originatingClass) {
if (configAnnotation != null) {
return new ResourcePollerConfig(configAnnotation.pollPeriod(),
new MySQLDbConfig(configAnnotation.host(), "" + configAnnotation.port(),
new MySQLDbConfig(configAnnotation.host(), String.valueOf(configAnnotation.port()),
configAnnotation.user(), configAnnotation.password()));
}
return new ResourcePollerConfig(SchemaConfig.DEFAULT_POLL_PERIOD,
Expand Down

0 comments on commit 5d8c567

Please sign in to comment.