From d5a573744352af1e1ba53d65d264bbbdb9c8a448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 8 Aug 2023 09:34:00 +0200 Subject: [PATCH 1/5] feat: create resource only if not exists (#2001) --- .../kubernetes/KubernetesDependent.java | 9 ++- .../KubernetesDependentConverter.java | 12 +++- .../KubernetesDependentResource.java | 7 ++- .../KubernetesDependentResourceConfig.java | 17 +++++- ...eateOnlyIfNotExistingDependentWithSSA.java | 57 +++++++++++++++++++ .../ConfigMapDependentResource.java | 30 ++++++++++ ...xistingDependentWithSSACustomResource.java | 13 +++++ ...NotExistingDependentWithSSAReconciler.java | 32 +++++++++++ 8 files changed, 169 insertions(+), 8 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateOnlyIfNotExistingDependentWithSSA.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/ConfigMapDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/CreateOnlyIfNotExistingDependentWithSSACustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/CreateOnlyIfNotExistingDependentWithSSAReconciler.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java index 868ae30dbc..c3f7be408a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java @@ -7,7 +7,10 @@ import io.javaoperatorsdk.operator.api.reconciler.Constants; import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; -import io.javaoperatorsdk.operator.processing.event.source.filter.*; +import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter; +import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter; +import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter; +import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET; @@ -69,4 +72,8 @@ Class resourceDiscriminator() default ResourceDiscriminator.class; + /** + * Creates the resource only if did not exist before, this applies only if SSA is used. + */ + boolean createResourceOnlyIfNotExistingWithSSA() default KubernetesDependentResourceConfig.DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverter.java index a9a60f8e0a..6ab07a9462 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverter.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverter.java @@ -14,6 +14,8 @@ import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; +import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig.DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA; + public class KubernetesDependentConverter implements ConfigurationConverter, KubernetesDependentResource> { @@ -25,6 +27,8 @@ public KubernetesDependentResourceConfig configFrom(KubernetesDependent confi var namespaces = parentConfiguration.getNamespaces(); var configuredNS = false; String labelSelector = null; + var createResourceOnlyIfNotExistingWithSSA = + DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA; OnAddFilter onAddFilter = null; OnUpdateFilter onUpdateFilter = null; OnDeleteFilter onDeleteFilter = null; @@ -39,9 +43,8 @@ public KubernetesDependentResourceConfig configFrom(KubernetesDependent confi final var fromAnnotation = configAnnotation.labelSelector(); labelSelector = Constants.NO_VALUE_SET.equals(fromAnnotation) ? null : fromAnnotation; - final var context = - Utils.contextFor(parentConfiguration, originatingClass, - configAnnotation.annotationType()); + final var context = Utils.contextFor(parentConfiguration, originatingClass, + configAnnotation.annotationType()); onAddFilter = Utils.instantiate(configAnnotation.onAddFilter(), OnAddFilter.class, context); onUpdateFilter = Utils.instantiate(configAnnotation.onUpdateFilter(), OnUpdateFilter.class, context); @@ -53,9 +56,12 @@ public KubernetesDependentResourceConfig configFrom(KubernetesDependent confi resourceDiscriminator = Utils.instantiate(configAnnotation.resourceDiscriminator(), ResourceDiscriminator.class, context); + createResourceOnlyIfNotExistingWithSSA = + configAnnotation.createResourceOnlyIfNotExistingWithSSA(); } return new KubernetesDependentResourceConfig(namespaces, labelSelector, configuredNS, + createResourceOnlyIfNotExistingWithSSA, resourceDiscriminator, onAddFilter, onUpdateFilter, onDeleteFilter, genericFilter); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 20e83f02bf..8d32892e10 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -130,7 +130,12 @@ protected R handleUpdate(R actual, R desired, P primary, Context

context) { public R create(R target, P primary, Context

context) { if (useSSA(context)) { // setting resource version for SSA so only created if it doesn't exist already - target.getMetadata().setResourceVersion("1"); + var createIfNotExisting = kubernetesDependentResourceConfig == null + ? KubernetesDependentResourceConfig.DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA + : kubernetesDependentResourceConfig.createResourceOnlyIfNotExistingWithSSA(); + if (createIfNotExisting) { + target.getMetadata().setResourceVersion("1"); + } } final var resource = prepare(target, primary, "Creating"); return useSSA(context) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java index 4047b25a13..33f4f91d1f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java @@ -13,9 +13,12 @@ public class KubernetesDependentResourceConfig { + public static final boolean DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA = true; + private Set namespaces = Constants.SAME_AS_CONTROLLER_NAMESPACES_SET; private String labelSelector = NO_VALUE_SET; private boolean namespacesWereConfigured = false; + private boolean createResourceOnlyIfNotExistingWithSSA; private ResourceDiscriminator resourceDiscriminator; private OnAddFilter onAddFilter; @@ -28,14 +31,18 @@ public class KubernetesDependentResourceConfig { public KubernetesDependentResourceConfig() {} - public KubernetesDependentResourceConfig(Set namespaces, String labelSelector, - boolean configuredNS, ResourceDiscriminator resourceDiscriminator, + public KubernetesDependentResourceConfig(Set namespaces, + String labelSelector, + boolean configuredNS, + boolean createResourceOnlyIfNotExistingWithSSA, + ResourceDiscriminator resourceDiscriminator, OnAddFilter onAddFilter, OnUpdateFilter onUpdateFilter, OnDeleteFilter onDeleteFilter, GenericFilter genericFilter) { this.namespaces = namespaces; this.labelSelector = labelSelector; this.namespacesWereConfigured = configuredNS; + this.createResourceOnlyIfNotExistingWithSSA = createResourceOnlyIfNotExistingWithSSA; this.onAddFilter = onAddFilter; this.onUpdateFilter = onUpdateFilter; this.onDeleteFilter = onDeleteFilter; @@ -44,7 +51,8 @@ public KubernetesDependentResourceConfig(Set namespaces, String labelSel } public KubernetesDependentResourceConfig(Set namespaces, String labelSelector) { - this(namespaces, labelSelector, true, null, null, null, + this(namespaces, labelSelector, true, DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA, + null, null, null, null, null); } @@ -70,6 +78,9 @@ public OnAddFilter onAddFilter() { return onAddFilter; } + public boolean createResourceOnlyIfNotExistingWithSSA() { + return createResourceOnlyIfNotExistingWithSSA; + } public OnUpdateFilter onUpdateFilter() { return onUpdateFilter; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateOnlyIfNotExistingDependentWithSSA.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateOnlyIfNotExistingDependentWithSSA.java new file mode 100644 index 0000000000..8a347f632e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateOnlyIfNotExistingDependentWithSSA.java @@ -0,0 +1,57 @@ +package io.javaoperatorsdk.operator; + +import java.time.Duration; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.createonlyifnotexistsdependentwithssa.CreateOnlyIfNotExistingDependentWithSSACustomResource; +import io.javaoperatorsdk.operator.sample.createonlyifnotexistsdependentwithssa.CreateOnlyIfNotExistingDependentWithSSAReconciler; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class CreateOnlyIfNotExistingDependentWithSSA { + + public static final String TEST_RESOURCE_NAME = "test1"; + public static final String KEY = "key"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(new CreateOnlyIfNotExistingDependentWithSSAReconciler()) + .build(); + + + @Test + void createsResourceOnlyIfNotExisting() { + var cm = new ConfigMapBuilder().withMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .build()) + .withData(Map.of(KEY, "val")) + .build(); + + extension.create(cm); + extension.create(testResource()); + + await().pollDelay(Duration.ofMillis(200)).untilAsserted(() -> { + var currentCM = extension.get(ConfigMap.class, TEST_RESOURCE_NAME); + assertThat(currentCM.getData()).containsKey(KEY); + }); + } + + CreateOnlyIfNotExistingDependentWithSSACustomResource testResource() { + var res = new CreateOnlyIfNotExistingDependentWithSSACustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .build()); + + return res; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/ConfigMapDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/ConfigMapDependentResource.java new file mode 100644 index 0000000000..d6947c2834 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/ConfigMapDependentResource.java @@ -0,0 +1,30 @@ +package io.javaoperatorsdk.operator.sample.createonlyifnotexistsdependentwithssa; + +import java.util.Map; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +public class ConfigMapDependentResource extends + CRUDKubernetesDependentResource { + + public ConfigMapDependentResource() { + super(ConfigMap.class); + } + + @Override + protected ConfigMap desired(CreateOnlyIfNotExistingDependentWithSSACustomResource primary, + Context context) { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()); + configMap.setData(Map.of("drkey", "v")); + return configMap; + } +} + + diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/CreateOnlyIfNotExistingDependentWithSSACustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/CreateOnlyIfNotExistingDependentWithSSACustomResource.java new file mode 100644 index 0000000000..23f06c365f --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/CreateOnlyIfNotExistingDependentWithSSACustomResource.java @@ -0,0 +1,13 @@ +package io.javaoperatorsdk.operator.sample.createonlyifnotexistsdependentwithssa; + +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.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +public class CreateOnlyIfNotExistingDependentWithSSACustomResource + extends CustomResource + implements Namespaced { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/CreateOnlyIfNotExistingDependentWithSSAReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/CreateOnlyIfNotExistingDependentWithSSAReconciler.java new file mode 100644 index 0000000000..884b5a859d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createonlyifnotexistsdependentwithssa/CreateOnlyIfNotExistingDependentWithSSAReconciler.java @@ -0,0 +1,32 @@ +package io.javaoperatorsdk.operator.sample.createonlyifnotexistsdependentwithssa; + +import java.util.concurrent.atomic.AtomicInteger; + +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; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; + +@ControllerConfiguration(dependents = { + @Dependent(type = ConfigMapDependentResource.class)}) +public class CreateOnlyIfNotExistingDependentWithSSAReconciler + implements Reconciler { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + CreateOnlyIfNotExistingDependentWithSSACustomResource resource, + Context context) { + numberOfExecutions.addAndGet(1); + return UpdateControl.noUpdate(); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + + + +} From ffdcc10c5f2c684afc6cb8d812aefb2d24eeab64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 14 Aug 2023 12:07:21 +0200 Subject: [PATCH 2/5] feat: leader election callbacks (#2015) --- .../operator/LeaderElectionManager.java | 19 ++++-- .../config/LeaderElectionConfiguration.java | 20 ++++-- .../LeaderElectionConfigurationBuilder.java | 61 +++++++++++++++++++ 3 files changed, 90 insertions(+), 10 deletions(-) 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/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index 1509d87f2a..316fdb4524 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 @@ -66,7 +66,7 @@ private void init(LeaderElectionConfiguration config) { config.getLeaseDuration(), config.getRenewDeadline(), config.getRetryPeriod(), - leaderCallbacks(), + leaderCallbacks(config), true, config.getLeaseName())) .build(); @@ -74,11 +74,20 @@ private void init(LeaderElectionConfiguration config) { - private LeaderCallbacks leaderCallbacks() { + private LeaderCallbacks leaderCallbacks(LeaderElectionConfiguration config) { return new LeaderCallbacks( - this::startLeading, - this::stopLeading, - leader -> log.info("New leader with identity: {}", leader)); + () -> { + config.getLeaderCallbacks().ifPresent(LeaderCallbacks::onStartLeading); + LeaderElectionManager.this.startLeading(); + }, + () -> { + config.getLeaderCallbacks().ifPresent(LeaderCallbacks::onStopLeading); + LeaderElectionManager.this.stopLeading(); + }, + leader -> { + config.getLeaderCallbacks().ifPresent(cb -> cb.onNewLeader(leader)); + log.info("New leader with identity: {}", leader); + }); } private void startLeading() { 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 5a2c322657..0ab72ff165 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 @@ -3,6 +3,8 @@ import java.time.Duration; import java.util.Optional; +import io.fabric8.kubernetes.client.extended.leaderelection.LeaderCallbacks; + public class LeaderElectionConfiguration { public static final Duration LEASE_DURATION_DEFAULT_VALUE = Duration.ofSeconds(15); @@ -17,13 +19,15 @@ public class LeaderElectionConfiguration { private final Duration renewDeadline; private final Duration retryPeriod; + private final LeaderCallbacks leaderCallbacks; + public LeaderElectionConfiguration(String leaseName, String leaseNamespace, String identity) { this( leaseName, leaseNamespace, LEASE_DURATION_DEFAULT_VALUE, RENEW_DEADLINE_DEFAULT_VALUE, - RETRY_PERIOD_DEFAULT_VALUE, identity); + RETRY_PERIOD_DEFAULT_VALUE, identity, null); } public LeaderElectionConfiguration(String leaseName, String leaseNamespace) { @@ -32,7 +36,7 @@ public LeaderElectionConfiguration(String leaseName, String leaseNamespace) { leaseNamespace, LEASE_DURATION_DEFAULT_VALUE, RENEW_DEADLINE_DEFAULT_VALUE, - RETRY_PERIOD_DEFAULT_VALUE, null); + RETRY_PERIOD_DEFAULT_VALUE, null, null); } public LeaderElectionConfiguration(String leaseName) { @@ -41,7 +45,7 @@ public LeaderElectionConfiguration(String leaseName) { null, LEASE_DURATION_DEFAULT_VALUE, RENEW_DEADLINE_DEFAULT_VALUE, - RETRY_PERIOD_DEFAULT_VALUE, null); + RETRY_PERIOD_DEFAULT_VALUE, null, null); } public LeaderElectionConfiguration( @@ -50,7 +54,7 @@ public LeaderElectionConfiguration( Duration leaseDuration, Duration renewDeadline, Duration retryPeriod) { - this(leaseName, leaseNamespace, leaseDuration, renewDeadline, retryPeriod, null); + this(leaseName, leaseNamespace, leaseDuration, renewDeadline, retryPeriod, null, null); } public LeaderElectionConfiguration( @@ -59,13 +63,15 @@ public LeaderElectionConfiguration( Duration leaseDuration, Duration renewDeadline, Duration retryPeriod, - String identity) { + String identity, + LeaderCallbacks leaderCallbacks) { this.leaseName = leaseName; this.leaseNamespace = leaseNamespace; this.leaseDuration = leaseDuration; this.renewDeadline = renewDeadline; this.retryPeriod = retryPeriod; this.identity = identity; + this.leaderCallbacks = leaderCallbacks; } public Optional getLeaseNamespace() { @@ -91,4 +97,8 @@ public Duration getRetryPeriod() { public Optional getIdentity() { return Optional.ofNullable(identity); } + + public Optional getLeaderCallbacks() { + return Optional.ofNullable(leaderCallbacks); + } } 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..4b21dd9d2d --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java @@ -0,0 +1,61 @@ +package io.javaoperatorsdk.operator.api.config; + +import java.time.Duration; + +import io.fabric8.kubernetes.client.extended.leaderelection.LeaderCallbacks; + +import static io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration.*; + +public final class LeaderElectionConfigurationBuilder { + + private String leaseName; + private String leaseNamespace; + private String identity; + private Duration leaseDuration = LEASE_DURATION_DEFAULT_VALUE; + private Duration renewDeadline = RENEW_DEADLINE_DEFAULT_VALUE; + private Duration retryPeriod = RETRY_PERIOD_DEFAULT_VALUE; + private LeaderCallbacks leaderCallbacks; + + private LeaderElectionConfigurationBuilder(String leaseName) { + this.leaseName = leaseName; + } + + public static LeaderElectionConfigurationBuilder aLeaderElectionConfiguration(String leaseName) { + return new LeaderElectionConfigurationBuilder(leaseName); + } + + public LeaderElectionConfigurationBuilder withLeaseNamespace(String leaseNamespace) { + this.leaseNamespace = leaseNamespace; + return this; + } + + public LeaderElectionConfigurationBuilder withIdentity(String identity) { + this.identity = identity; + 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 withLeaderCallbacks(LeaderCallbacks leaderCallbacks) { + this.leaderCallbacks = leaderCallbacks; + return this; + } + + public LeaderElectionConfiguration build() { + return new LeaderElectionConfiguration(leaseName, leaseNamespace, leaseDuration, renewDeadline, + retryPeriod, identity, leaderCallbacks); + } +} From 5c458e56756430087481858af8d720ec5ed5cd00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 14 Aug 2023 14:17:29 +0200 Subject: [PATCH 3/5] discriminator improvements (#2013) --- .../api/reconciler/IndexDiscriminator.java | 50 +++++++++++++++++++ .../ResourceIDMatcherDiscriminator.java | 26 ++++++++-- .../IndexDiscriminator.java | 41 --------------- .../IndexDiscriminatorTestReconciler.java | 4 +- .../TestIndexDiscriminator.java | 14 ++++++ 5 files changed, 89 insertions(+), 46 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/IndexDiscriminator.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/IndexDiscriminator.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/TestIndexDiscriminator.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/IndexDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/IndexDiscriminator.java new file mode 100644 index 0000000000..7a27397b26 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/IndexDiscriminator.java @@ -0,0 +1,50 @@ +package io.javaoperatorsdk.operator.api.reconciler; + +import java.util.Optional; +import java.util.function.Function; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +/** + * Uses a custom index of {@link InformerEventSource} to access the target resource. The index needs + * to be explicitly created when the event source is defined. This approach improves the performance + * to access the resource. + */ +public class IndexDiscriminator + implements ResourceDiscriminator { + + private final String indexName; + private final String eventSourceName; + private final Function keyMapper; + + public IndexDiscriminator(String indexName, Function keyMapper) { + this(indexName, null, keyMapper); + } + + public IndexDiscriminator(String indexName, String eventSourceName, + Function keyMapper) { + this.indexName = indexName; + this.eventSourceName = eventSourceName; + this.keyMapper = keyMapper; + } + + @Override + public Optional distinguish(Class resource, + P primary, + Context

context) { + + InformerEventSource eventSource = + (InformerEventSource) context + .eventSourceRetriever() + .getResourceEventSourceFor(resource, eventSourceName); + var resources = eventSource.byIndex(indexName, keyMapper.apply(primary)); + if (resources.isEmpty()) { + return Optional.empty(); + } else if (resources.size() > 1) { + throw new IllegalStateException("More than one resource found"); + } else { + return Optional.of(resources.get(0)); + } + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceIDMatcherDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceIDMatcherDiscriminator.java index d23459e271..da773fc210 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceIDMatcherDiscriminator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceIDMatcherDiscriminator.java @@ -5,21 +5,41 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.Cache; public class ResourceIDMatcherDiscriminator implements ResourceDiscriminator { + + private final String eventSourceName; private final Function mapper; public ResourceIDMatcherDiscriminator(Function mapper) { + this(null, mapper); + } + + public ResourceIDMatcherDiscriminator(String eventSourceName, Function mapper) { + this.eventSourceName = eventSourceName; this.mapper = mapper; } + @SuppressWarnings("unchecked") @Override public Optional distinguish(Class resource, P primary, Context

context) { var resourceID = mapper.apply(primary); - return context.getSecondaryResourcesAsStream(resource) - .filter(resourceID::isSameResource) - .findFirst(); + if (eventSourceName != null) { + return ((Cache) context.eventSourceRetriever().getResourceEventSourceFor(resource, + eventSourceName)) + .get(resourceID); + } else { + var eventSources = context.eventSourceRetriever().getResourceEventSourcesFor(resource); + if (eventSources.size() == 1) { + return ((Cache) eventSources.get(0)).get(resourceID); + } else { + return context.getSecondaryResourcesAsStream(resource) + .filter(resourceID::isSameResource) + .findFirst(); + } + } } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/IndexDiscriminator.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/IndexDiscriminator.java deleted file mode 100644 index eb6e193479..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/IndexDiscriminator.java +++ /dev/null @@ -1,41 +0,0 @@ -package io.javaoperatorsdk.operator.sample.indexdiscriminator; - -import java.util.Optional; - -import io.fabric8.kubernetes.api.model.ConfigMap; -import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; -import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; - -import static io.javaoperatorsdk.operator.sample.indexdiscriminator.IndexDiscriminatorTestReconciler.configMapKeyFromPrimary; - -public class IndexDiscriminator - implements ResourceDiscriminator { - - private final String indexName; - private final String nameSuffix; - - public IndexDiscriminator(String indexName, String nameSuffix) { - this.indexName = indexName; - this.nameSuffix = nameSuffix; - } - - @Override - public Optional distinguish(Class resource, - IndexDiscriminatorTestCustomResource primary, - Context context) { - - InformerEventSource eventSource = - (InformerEventSource) context - .eventSourceRetriever() - .getResourceEventSourceFor(ConfigMap.class); - var resources = eventSource.byIndex(indexName, configMapKeyFromPrimary(primary, nameSuffix)); - if (resources.isEmpty()) { - return Optional.empty(); - } else if (resources.size() > 1) { - throw new IllegalStateException("more than one resource"); - } else { - return Optional.of(resources.get(0)); - } - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/IndexDiscriminatorTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/IndexDiscriminatorTestReconciler.java index 0b0af2a1cc..b988c93491 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/IndexDiscriminatorTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/IndexDiscriminatorTestReconciler.java @@ -81,10 +81,10 @@ public Map prepareEventSources( firstDependentResourceConfigMap .setResourceDiscriminator( - new IndexDiscriminator(CONFIG_MAP_INDEX_1, FIRST_CONFIG_MAP_SUFFIX_1)); + new TestIndexDiscriminator(CONFIG_MAP_INDEX_1, FIRST_CONFIG_MAP_SUFFIX_1)); secondDependentResourceConfigMap .setResourceDiscriminator( - new IndexDiscriminator(CONFIG_MAP_INDEX_2, FIRST_CONFIG_MAP_SUFFIX_2)); + new TestIndexDiscriminator(CONFIG_MAP_INDEX_2, FIRST_CONFIG_MAP_SUFFIX_2)); return EventSourceInitializer.nameEventSources(eventSource); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/TestIndexDiscriminator.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/TestIndexDiscriminator.java new file mode 100644 index 0000000000..a56e44ced8 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/TestIndexDiscriminator.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.sample.indexdiscriminator; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.reconciler.IndexDiscriminator; + +import static io.javaoperatorsdk.operator.sample.indexdiscriminator.IndexDiscriminatorTestReconciler.configMapKeyFromPrimary; + +public class TestIndexDiscriminator + extends IndexDiscriminator { + + public TestIndexDiscriminator(String indexName, String nameSuffix) { + super(indexName, p -> configMapKeyFromPrimary(p, nameSuffix)); + } +} From 8675f3f3983aabfb9e6ef6bfc1f268b3def2c8ff Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Fri, 11 Aug 2023 07:00:17 -0400 Subject: [PATCH 4/5] generalization of the update matcher --- .../GenericKubernetesResourceMatcher.java | 47 ++++++++++++------ ...sterRoleBindingResourceUpdaterMatcher.java | 23 --------- .../ClusterRoleResourceUpdaterMatcher.java | 22 --------- .../ConfigMapResourceUpdaterMatcher.java | 24 --------- .../EndpointSliceResourceUpdateMatcher.java | 25 ---------- .../EndpointsResourceUpdaterMatcher.java | 20 -------- .../GenericResourceUpdaterMatcher.java | 49 ++++++------------- .../RoleBindingResourceUpdaterMatcher.java | 23 --------- .../RoleResourceUpdaterMatcher.java | 19 ------- .../SecretResourceUpdaterMatcher.java | 25 ---------- .../ServiceAccountResourceUpdaterMatcher.java | 25 ---------- .../GenericResourceUpdaterMatcherTest.java | 2 +- 12 files changed, 49 insertions(+), 255 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleBindingResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ConfigMapResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointSliceResourceUpdateMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointsResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleBindingResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/SecretResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ServiceAccountResourceUpdaterMatcher.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java index 36935e08a0..48908310b6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.Set; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; @@ -24,6 +25,9 @@ public class GenericKubernetesResourceMatcher NOT_OTHER_FIELDS = Set.of(SPEC, "/metadata", "/apiVersion", "/kind"); private static final String PATH = "path"; private static final String[] EMPTY_ARRAY = {}; @@ -192,11 +196,11 @@ public static Result match(R d } } - final var matched = matchSpec(actualResource, desired, specEquality, context, ignoredPaths); + final var matched = match(actualResource, desired, specEquality, context, ignoredPaths); return Result.computed(matched, desired); } - private static boolean matchSpec(R actual, R desired, boolean equality, + private static boolean match(R actual, R desired, boolean equality, Context context, String[] ignoredPaths) { @@ -208,25 +212,38 @@ private static boolean matchSpec(R actual, R desired, bo final List ignoreList = ignoredPaths != null && ignoredPaths.length > 0 ? Arrays.asList(ignoredPaths) : Collections.emptyList(); - // reflection will be replaced by this: - // https://github.com/fabric8io/kubernetes-client/issues/3816 - var specDiffJsonPatch = getDiffsImpactingPathsWithPrefixes(wholeDiffJsonPatch, SPEC); + boolean specMatch = match(equality, wholeDiffJsonPatch, ignoreList, SPEC); + if (!specMatch) { + return false; + } + // expect everything else to be equal + var names = desiredNode.fieldNames(); + List prefixes = new ArrayList<>(); + while (names.hasNext()) { + String prefix = "/" + names.next(); + if (!NOT_OTHER_FIELDS.contains(prefix)) { + prefixes.add(prefix); + } + } + return match(true, wholeDiffJsonPatch, ignoreList, prefixes.toArray(String[]::new)); + } + + private static boolean match(boolean equality, JsonNode wholeDiffJsonPatch, + final List ignoreList, String... prefixes) { + var diffJsonPatch = getDiffsImpactingPathsWithPrefixes(wholeDiffJsonPatch, prefixes); // In case of equality is set to true, no diffs are allowed, so we return early if diffs exist // On contrary (if equality is false), "add" is allowed for cases when for some // resources Kubernetes fills-in values into spec. - if (equality && !specDiffJsonPatch.isEmpty()) { + if (diffJsonPatch.isEmpty()) { + return true; + } + if (equality) { return false; } - if (!equality && !ignoreList.isEmpty()) { - if (!allDiffsOnIgnoreList(specDiffJsonPatch, ignoreList)) { - return false; - } - } else { - if (!allDiffsAreAddOps(specDiffJsonPatch)) { - return false; - } + if (!ignoreList.isEmpty()) { + return allDiffsOnIgnoreList(diffJsonPatch, ignoreList); } - return true; + return allDiffsAreAddOps(diffJsonPatch); } private static boolean allDiffsOnIgnoreList(List metadataJSonDiffs, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleBindingResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleBindingResourceUpdaterMatcher.java deleted file mode 100644 index f6f6d1ef54..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleBindingResourceUpdaterMatcher.java +++ /dev/null @@ -1,23 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class ClusterRoleBindingResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(ClusterRoleBinding actual, ClusterRoleBinding desired) { - actual.setRoleRef(desired.getRoleRef()); - actual.setSubjects(desired.getSubjects()); - } - - @Override - public boolean matches(ClusterRoleBinding actual, ClusterRoleBinding desired, - Context context) { - return Objects.equals(actual.getRoleRef(), desired.getRoleRef()) && - Objects.equals(actual.getSubjects(), desired.getSubjects()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleResourceUpdaterMatcher.java deleted file mode 100644 index da7997c040..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleResourceUpdaterMatcher.java +++ /dev/null @@ -1,22 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.rbac.ClusterRole; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class ClusterRoleResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(ClusterRole actual, ClusterRole desired) { - actual.setAggregationRule(desired.getAggregationRule()); - actual.setRules(desired.getRules()); - } - - @Override - public boolean matches(ClusterRole actual, ClusterRole desired, Context context) { - return Objects.equals(actual.getRules(), desired.getRules()) && - Objects.equals(actual.getAggregationRule(), desired.getAggregationRule()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ConfigMapResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ConfigMapResourceUpdaterMatcher.java deleted file mode 100644 index 7f89d45ff5..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ConfigMapResourceUpdaterMatcher.java +++ /dev/null @@ -1,24 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.ConfigMap; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class ConfigMapResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(ConfigMap actual, ConfigMap desired) { - actual.setData(desired.getData()); - actual.setBinaryData((desired.getBinaryData())); - actual.setImmutable(desired.getImmutable()); - } - - @Override - public boolean matches(ConfigMap actual, ConfigMap desired, Context context) { - return Objects.equals(actual.getImmutable(), desired.getImmutable()) && - Objects.equals(actual.getData(), desired.getData()) && - Objects.equals(actual.getBinaryData(), desired.getBinaryData()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointSliceResourceUpdateMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointSliceResourceUpdateMatcher.java deleted file mode 100644 index dc83c86dbf..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointSliceResourceUpdateMatcher.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.discovery.v1.EndpointSlice; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class EndpointSliceResourceUpdateMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(EndpointSlice actual, EndpointSlice desired) { - actual.setEndpoints(desired.getEndpoints()); - actual.setAddressType(desired.getAddressType()); - actual.setPorts(desired.getPorts()); - } - - @Override - public boolean matches(EndpointSlice actual, EndpointSlice desired, Context context) { - return Objects.equals(actual.getEndpoints(), desired.getEndpoints()) && - Objects.equals(actual.getAddressType(), desired.getAddressType()) && - Objects.equals(actual.getPorts(), desired.getPorts()); - } - -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointsResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointsResourceUpdaterMatcher.java deleted file mode 100644 index 3dcfabde28..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointsResourceUpdaterMatcher.java +++ /dev/null @@ -1,20 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.Endpoints; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class EndpointsResourceUpdaterMatcher extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(Endpoints actual, Endpoints desired) { - actual.setSubsets(desired.getSubsets()); - } - - @Override - public boolean matches(Endpoints actual, Endpoints desired, Context context) { - return Objects.equals(actual.getSubsets(), desired.getSubsets()); - } - -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/GenericResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/GenericResourceUpdaterMatcher.java index cd9fbe2b16..2a5bae03b9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/GenericResourceUpdaterMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/GenericResourceUpdaterMatcher.java @@ -2,13 +2,8 @@ import java.util.Map; -import io.fabric8.kubernetes.api.model.*; -import io.fabric8.kubernetes.api.model.discovery.v1.EndpointSlice; -import io.fabric8.kubernetes.api.model.rbac.ClusterRole; -import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding; -import io.fabric8.kubernetes.api.model.rbac.Role; -import io.fabric8.kubernetes.api.model.rbac.RoleBinding; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.utils.KubernetesSerialization; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceUpdaterMatcher; @@ -16,34 +11,31 @@ public class GenericResourceUpdaterMatcher implements ResourceUpdaterMatcher { + private static final String METADATA = "metadata"; private static final ResourceUpdaterMatcher INSTANCE = new GenericResourceUpdaterMatcher<>(); - @SuppressWarnings("rawtypes") - private static final Map processors = Map.of( - Secret.class, new SecretResourceUpdaterMatcher(), - ConfigMap.class, new ConfigMapResourceUpdaterMatcher(), - ServiceAccount.class, new ServiceAccountResourceUpdaterMatcher(), - Role.class, new RoleResourceUpdaterMatcher(), - ClusterRole.class, new ClusterRoleResourceUpdaterMatcher(), - RoleBinding.class, new RoleBindingResourceUpdaterMatcher(), - ClusterRoleBinding.class, new ClusterRoleBindingResourceUpdaterMatcher(), - Endpoints.class, new EndpointsResourceUpdaterMatcher(), - EndpointSlice.class, new EndpointSliceResourceUpdateMatcher()); - protected GenericResourceUpdaterMatcher() {} @SuppressWarnings("unchecked") public static ResourceUpdaterMatcher updaterMatcherFor( Class resourceType) { - final var processor = processors.get(resourceType); - return processor != null ? processor : (ResourceUpdaterMatcher) INSTANCE; + return (ResourceUpdaterMatcher) INSTANCE; } + @SuppressWarnings("unchecked") + @Override public R updateResource(R actual, R desired, Context context) { - var clonedActual = context.getControllerConfiguration().getConfigurationService() - .getResourceCloner().clone(actual); + KubernetesSerialization kubernetesSerialization = + context.getClient().getKubernetesSerialization(); + Map actualMap = kubernetesSerialization.convertValue(actual, Map.class); + Map desiredMap = kubernetesSerialization.convertValue(desired, Map.class); + // replace all top level fields from actual with desired, but merge metadata separately + var metadata = actualMap.remove(METADATA); + actualMap.replaceAll((k, v) -> desiredMap.get(k)); + actualMap.putAll(desiredMap); + actualMap.put(METADATA, metadata); + var clonedActual = (R) kubernetesSerialization.convertValue(actualMap, desired.getClass()); updateLabelsAndAnnotation(clonedActual, desired); - updateClonedActual(clonedActual, desired); return clonedActual; } @@ -53,15 +45,6 @@ public boolean matches(R actual, R desired, Context context) { false, false, context).matched(); } - protected void updateClonedActual(R actual, R desired) { - updateSpec(actual, desired); - } - - public static void updateSpec(K actual, K desired) { - var desiredSpec = ReconcilerUtils.getSpec(desired); - ReconcilerUtils.setSpec(actual, desiredSpec); - } - public static void updateLabelsAndAnnotation(K actual, K desired) { actual.getMetadata().getLabels().putAll(desired.getMetadata().getLabels()); actual.getMetadata().getAnnotations().putAll(desired.getMetadata().getAnnotations()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleBindingResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleBindingResourceUpdaterMatcher.java deleted file mode 100644 index c6a87cdae3..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleBindingResourceUpdaterMatcher.java +++ /dev/null @@ -1,23 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.rbac.RoleBinding; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class RoleBindingResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(RoleBinding actual, RoleBinding desired) { - actual.setRoleRef(desired.getRoleRef()); - actual.setSubjects(desired.getSubjects()); - } - - @Override - public boolean matches(RoleBinding actual, RoleBinding desired, - Context context) { - return Objects.equals(actual.getRoleRef(), desired.getRoleRef()) && - Objects.equals(actual.getSubjects(), desired.getSubjects()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleResourceUpdaterMatcher.java deleted file mode 100644 index f02d946db8..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleResourceUpdaterMatcher.java +++ /dev/null @@ -1,19 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.rbac.Role; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class RoleResourceUpdaterMatcher extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(Role actual, Role desired) { - actual.setRules(desired.getRules()); - } - - @Override - public boolean matches(Role actual, Role desired, Context context) { - return Objects.equals(actual.getRules(), desired.getRules()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/SecretResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/SecretResourceUpdaterMatcher.java deleted file mode 100644 index 14e8696704..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/SecretResourceUpdaterMatcher.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.Secret; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class SecretResourceUpdaterMatcher extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(Secret actual, Secret desired) { - actual.setData(desired.getData()); - actual.setStringData(desired.getStringData()); - actual.setImmutable(desired.getImmutable()); - actual.setType(desired.getType()); - } - - @Override - public boolean matches(Secret actual, Secret desired, Context context) { - return Objects.equals(actual.getImmutable(), desired.getImmutable()) && - Objects.equals(actual.getType(), desired.getType()) && - Objects.equals(actual.getData(), desired.getData()) && - Objects.equals(actual.getStringData(), desired.getStringData()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ServiceAccountResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ServiceAccountResourceUpdaterMatcher.java deleted file mode 100644 index f3d625c778..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ServiceAccountResourceUpdaterMatcher.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.ServiceAccount; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class ServiceAccountResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(ServiceAccount actual, ServiceAccount desired) { - actual.setAutomountServiceAccountToken(desired.getAutomountServiceAccountToken()); - actual.setImagePullSecrets(desired.getImagePullSecrets()); - actual.setSecrets(desired.getSecrets()); - } - - @Override - public boolean matches(ServiceAccount actual, ServiceAccount desired, Context context) { - return Objects.equals(actual.getAutomountServiceAccountToken(), - desired.getAutomountServiceAccountToken()) && - Objects.equals(actual.getImagePullSecrets(), desired.getImagePullSecrets()) && - Objects.equals(actual.getSecrets(), desired.getSecrets()); - } -} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java index c1d9b4a5a5..d53c0a53e0 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java @@ -33,7 +33,7 @@ static void setUp() { final var client = MockKubernetesClient.client(HasMetadata.class); when(configService.getKubernetesClient()).thenReturn(client); when(configService.getResourceCloner()).thenCallRealMethod(); - + when(context.getClient()).thenReturn(client); when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); } From 0357ac3065d6719e038438fcd5c3714d07e4636a Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Mon, 14 Aug 2023 07:40:53 -0400 Subject: [PATCH 5/5] consolidating the diffs also adding a test that removes a field not present in the desired --- .../GenericKubernetesResourceMatcher.java | 133 +++--------------- .../GenericKubernetesResourceMatcherTest.java | 10 ++ .../GenericResourceUpdaterMatcherTest.java | 18 +++ 3 files changed, 50 insertions(+), 111 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java index 48908310b6..98198bf39a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java @@ -1,12 +1,8 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; @@ -21,13 +17,12 @@ public class GenericKubernetesResourceMatcher { private static final String SPEC = "/spec"; + private static final String METADATA = "/metadata"; private static final String ADD = "add"; private static final String OP = "op"; + private static final List IGNORED_FIELDS = List.of("/apiVersion", "/kind", "/status"); public static final String METADATA_LABELS = "/metadata/labels"; public static final String METADATA_ANNOTATIONS = "/metadata/annotations"; - // without knowing the CRD we cannot ignore status as it may not be a subresource, so if it's - // included we expect it to match - private static Set NOT_OTHER_FIELDS = Set.of(SPEC, "/metadata", "/apiVersion", "/kind"); private static final String PATH = "path"; private static final String[] EMPTY_ARRAY = {}; @@ -188,100 +183,39 @@ public static Result match(R d "Equality should be false in case of ignore list provided"); } - if (considerMetadata) { - Optional> res = - matchMetadata(desired, actualResource, labelsAndAnnotationsEquality, context); - if (res.isPresent()) { - return res.orElseThrow(); - } - } - - final var matched = match(actualResource, desired, specEquality, context, ignoredPaths); - return Result.computed(matched, desired); - } - - private static boolean match(R actual, R desired, boolean equality, - Context context, - String[] ignoredPaths) { - final var kubernetesSerialization = context.getClient().getKubernetesSerialization(); var desiredNode = kubernetesSerialization.convertValue(desired, JsonNode.class); - var actualNode = kubernetesSerialization.convertValue(actual, JsonNode.class); + var actualNode = kubernetesSerialization.convertValue(actualResource, JsonNode.class); var wholeDiffJsonPatch = JsonDiff.asJson(desiredNode, actualNode); - final List ignoreList = - ignoredPaths != null && ignoredPaths.length > 0 ? Arrays.asList(ignoredPaths) - : Collections.emptyList(); - boolean specMatch = match(equality, wholeDiffJsonPatch, ignoreList, SPEC); - if (!specMatch) { - return false; - } - // expect everything else to be equal - var names = desiredNode.fieldNames(); - List prefixes = new ArrayList<>(); - while (names.hasNext()) { - String prefix = "/" + names.next(); - if (!NOT_OTHER_FIELDS.contains(prefix)) { - prefixes.add(prefix); + boolean matched = true; + for (int i = 0; i < wholeDiffJsonPatch.size() && matched; i++) { + var node = wholeDiffJsonPatch.get(i); + if (nodeIsChildOf(node, List.of(SPEC))) { + matched = match(specEquality, node, ignoreList); + } else if (nodeIsChildOf(node, List.of(METADATA))) { + // conditionally consider labels and annotations + if (considerMetadata + && nodeIsChildOf(node, List.of(METADATA_LABELS, METADATA_ANNOTATIONS))) { + matched = match(labelsAndAnnotationsEquality, node, Collections.emptyList()); + } + } else if (!nodeIsChildOf(node, IGNORED_FIELDS)) { + matched = match(true, node, ignoreList); } } - return match(true, wholeDiffJsonPatch, ignoreList, prefixes.toArray(String[]::new)); + + return Result.computed(matched, desired); } - private static boolean match(boolean equality, JsonNode wholeDiffJsonPatch, - final List ignoreList, String... prefixes) { - var diffJsonPatch = getDiffsImpactingPathsWithPrefixes(wholeDiffJsonPatch, prefixes); - // In case of equality is set to true, no diffs are allowed, so we return early if diffs exist - // On contrary (if equality is false), "add" is allowed for cases when for some - // resources Kubernetes fills-in values into spec. - if (diffJsonPatch.isEmpty()) { - return true; - } + private static boolean match(boolean equality, JsonNode diff, + final List ignoreList) { if (equality) { return false; } if (!ignoreList.isEmpty()) { - return allDiffsOnIgnoreList(diffJsonPatch, ignoreList); - } - return allDiffsAreAddOps(diffJsonPatch); - } - - private static boolean allDiffsOnIgnoreList(List metadataJSonDiffs, - List ignoreList) { - if (metadataJSonDiffs.isEmpty()) { - return false; - } - return metadataJSonDiffs.stream().allMatch(n -> nodeIsChildOf(n, ignoreList)); - } - - private static Optional> matchMetadata( - R desired, - R actualResource, - boolean labelsAndAnnotationsEquality, Context

context) { - - if (labelsAndAnnotationsEquality) { - final var desiredMetadata = desired.getMetadata(); - final var actualMetadata = actualResource.getMetadata(); - - final var matched = - Objects.equals(desiredMetadata.getAnnotations(), actualMetadata.getAnnotations()) && - Objects.equals(desiredMetadata.getLabels(), actualMetadata.getLabels()); - if (!matched) { - return Optional.of(Result.computed(false, desired)); - } - } else { - final var objectMapper = context.getClient().getKubernetesSerialization(); - var desiredNode = objectMapper.convertValue(desired, JsonNode.class); - var actualNode = objectMapper.convertValue(actualResource, JsonNode.class); - var wholeDiffJsonPatch = JsonDiff.asJson(desiredNode, actualNode); - var metadataJSonDiffs = getDiffsImpactingPathsWithPrefixes(wholeDiffJsonPatch, - METADATA_LABELS, - METADATA_ANNOTATIONS); - if (!allDiffsAreAddOps(metadataJSonDiffs)) { - return Optional.of(Result.computed(false, desired)); - } + return nodeIsChildOf(diff, ignoreList); } - return Optional.empty(); + return ADD.equals(diff.get(OP).asText()); } static boolean nodeIsChildOf(JsonNode n, List prefixes) { @@ -293,29 +227,6 @@ static String getPath(JsonNode n) { return n.get(PATH).asText(); } - static boolean allDiffsAreAddOps(List metadataJSonDiffs) { - if (metadataJSonDiffs.isEmpty()) { - return true; - } - return metadataJSonDiffs.stream().allMatch(n -> ADD.equals(n.get(OP).asText())); - } - - public static List getDiffsImpactingPathsWithPrefixes(JsonNode diffJsonPatch, - String... prefixes) { - if (prefixes != null && prefixes.length > 0) { - var res = new ArrayList(); - var prefixList = Arrays.asList(prefixes); - for (int i = 0; i < diffJsonPatch.size(); i++) { - var node = diffJsonPatch.get(i); - if (nodeIsChildOf(node, prefixList)) { - res.add(node); - } - } - return res; - } - return Collections.emptyList(); - } - @Deprecated(forRemoval = true) public static Result match( KubernetesDependentResource dependentResource, R actualResource, P primary, diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java index 41bafe00e4..b1ff214f3b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java @@ -10,6 +10,7 @@ import io.fabric8.kubernetes.api.model.ServiceAccountBuilder; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; +import io.fabric8.kubernetes.api.model.apps.DeploymentStatusBuilder; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -79,6 +80,15 @@ void doesNotMatchChangedValues() { .isFalse(); } + @Test + void ignoreStatus() { + actual = createDeployment(); + actual.setStatus(new DeploymentStatusBuilder().withReadyReplicas(1).build()); + assertThat(matcher.match(actual, null, context).matched()) + .withFailMessage("Should ignore status in actual") + .isTrue(); + } + @Test void doesNotMatchChangedValuesWhenNoIgnoredPathsAreProvided() { actual = createDeployment(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java index d53c0a53e0..44f3fb51ea 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java @@ -2,6 +2,7 @@ import java.util.HashMap; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -99,6 +100,23 @@ void checkSecret() { assertThat(secret.getData()).containsOnlyKeys("foo"); } + @Test + void checkSeviceAccount() { + var processor = GenericResourceUpdaterMatcher.updaterMatcherFor(ServiceAccount.class); + var desired = new ServiceAccountBuilder() + .withMetadata(new ObjectMetaBuilder().addToLabels("new", "label").build()) + .build(); + var actual = new ServiceAccountBuilder() + .withMetadata(new ObjectMetaBuilder().addToLabels("a", "label").build()) + .withImagePullSecrets(new LocalObjectReferenceBuilder().withName("secret").build()) + .build(); + + final var serviceAccount = processor.updateResource(actual, desired, context); + assertThat(serviceAccount.getMetadata().getLabels()) + .isEqualTo(Map.of("a", "label", "new", "label")); + assertThat(serviceAccount.getImagePullSecrets()).isNullOrEmpty(); + } + Deployment createDeployment() { return ReconcilerUtils.loadYaml( Deployment.class, GenericResourceUpdaterMatcherTest.class, "nginx-deployment.yaml");