Skip to content

fix-1828: add HA support for config watcher - #2280

Open
wind57 wants to merge 38 commits into
spring-cloud:mainfrom
wind57:fix-1828
Open

wind57 wants to merge 38 commits into
spring-cloud:mainfrom
wind57:fix-1828

Conversation

@wind57

@wind57 wind57 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: wind57 <eugen.rabii@gmail.com>
wind57 added 19 commits July 31, 2026 18:09
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
@wind57

wind57 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

@ryanjbaxter can you trigger copilot in here please?

}

private boolean isDeploymentReady(String deploymentName, String namespace) throws ApiException {
private boolean isDeploymentReady(String deploymentName, String namespace, int expectedReplicas)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the new IT that I added, there is a need for two replicas, to really test the HA set-up

Signed-off-by: wind57 <eugen.rabii@gmail.com>
envVars.add(new V1EnvVar().name("SPRING_CLOUD_KUBERNETES_SECRETS_ENABLED").value("TRUE"));

if (enableHa) {
envVars.add(new V1EnvVar().name("SPRING_CLOUD_KUBERNETES_LEADER_ELECTION_ENABLED").value("true"));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two properties are needed to enable HA

}

/**
* <pre>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a single IT, that goes through a cycle of leader / no leader / leader

*
* @author wind57
*/
sealed interface ConfigurationWatcherStateStore permits LeaseConfigurationWatcherStateStore {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the definition of the resource version store. Methods in this one are executed only by the leader

wind57 added 2 commits August 26, 2026 14:12
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
}

@Override
public ConfigurationWatcherState readOrCreate() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if HA is enabled and this is the first call, this will create an empty lease. Otherwise, it will read whatever is stored there.

We store in the spring.cloud.kubernetes.configuration.watcher/configmap-resource-version annotation, something like : "default=123,prod=456", so each namespace tracks its own resource version checkpoint.

We need such a store because during a downtime when there is no leader at all, resource version can progress ( meaning configmap is updated ), but since there is no leader, events can get lost. As such, we always increment and "store" ( via this implementation ) the most recent resource version we have observed. This happens in the handlers onAdd / onDelete / onUpdate.

So for example:

  • we are now the leader and the resourceVersion is at 1.
  • we lose leadership, so our store stays at 1.
  • configmap progresses to resourceVersion=2
  • another leader is elected, it reads the store, sees that it holds 1
  • starts the informer at resourceVersion=1, so it can replay events it has not seen

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that necessary? In reality if a value was updated while there was no monitor and then updated again the app likely only cares about what the latest value is and don't really care about what it was changed to inbetween.

Signed-off-by: wind57 <eugen.rabii@gmail.com>
@wind57

wind57 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

TL;DR

This PR adds high-availability support to the Kubernetes client-based Configuration Watcher.

The Configuration Watcher can now run with multiple replicas while ensuring that only one replica actively watches ConfigMaps and Secrets at a time. If the leader fails, another replica acquires leadership, restores the persisted informer checkpoints, and continues processing changes.

How It Works

HA requires both properties to be enabled:

spring:
  cloud:
    kubernetes:
      leader:
        election:
          enabled: true
      configuration:
        watcher:
          ha:
            enabled: true

The two properties have separate responsibilities:

  • spring.cloud.kubernetes.leader.election.enabled enables native Kubernetes leader election.
  • spring.cloud.kubernetes.configuration.watcher.ha.enabled makes the Configuration Watcher start and stop its informers according to leadership.

When HA is disabled, the existing behavior is unchanged. ConfigMap and Secret informers start during normal bean initialization.

When HA is enabled:

  • Informers are not started during initialization.
  • The leader-election callback notifies the Configuration Watcher that leadership was acquired.
  • The Configuration Watcher reads its persisted state.
  • ConfigMap and Secret informers start using the stored resource-version checkpoints.
  • When leadership is lost, both informers are stopped.
  • The next leader restores the checkpoints and resumes watching.

The HA coordinator is initialized before the leader-election callbacks so that it is ready to receive leadership events.

Persistent State

The Configuration Watcher stores its state in a Kubernetes Lease. The leader-election lock and the Configuration Watcher state lease are separate resources.

The state lease stores the last processed resource version independently for:

  • each watched namespace
  • ConfigMaps
  • Secrets

The values are stored in annotations on the lease. For example:

spring.cloud.kubernetes.configuration.watcher/configmap-resource-version:
  default=123,other-namespace=456

The default configuration is:

spring:
  cloud:
    kubernetes:
      configuration:
        watcher:
          ha:
            enabled: false
            lease-name: configuration-watcher-ha
            lease-namespace: default

The lease is created automatically when the first leader starts, if it does not already exist.

Resource-Version Replay

  • When a new leader starts, the persisted resource version is used only for the first informer request for each namespace. This request restores the informer from the exact stored checkpoint.
  • After that first request, the informer controls its own resource-version progression. Each subsequent list or watch request uses the resource version supplied by the informer.
  • ConfigMap and Secret namespaces are tracked independently, so different namespaces can progress at different resource versions.
  • Resource versions are persisted after the corresponding resource event has been handled. This provides at-least-once event processing:
    • If a leader processes an event and persists the checkpoint successfully, the next leader continues after that event.
    • If the leader fails after processing the event but before persisting the checkpoint, the replacement leader may process the event again.

Repeated refresh notifications must therefore be tolerated by refresh targets.

  • The watcher does not refresh every ConfigMap or Secret after a leadership change. It only replays events for the configured informer scopes.

  • At least two Configuration Watcher replicas are required for failover:

spec:
  replicas: 2

@wind57
wind57 marked this pull request as ready for review August 26, 2026 17:07
@wind57

wind57 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

@ryanjbaxter in case you missed this... can you trigger copilot please?

@ryanjbaxter

Copy link
Copy Markdown
Contributor

Have no missed it, just need to set aside some time to look at it. Probably next week

@wind57

wind57 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

ok, thank you! as usual, Im available on any ways to answer any questions.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds High Availability (HA) support for the Kubernetes-client Configuration Watcher by deferring informer startup until leadership is acquired, persisting last-processed informer resourceVersions in a Kubernetes Lease, and replaying missed events after leader loss. This extends the configuration-watcher controller with HA auto-configuration and introduces new integration/unit tests to validate lease-backed checkpointing and leader failover behavior.

Changes:

  • Introduce HA coordinator + lease-backed state store to persist and restore informer checkpoints (ConfigMap/Secret resourceVersions).
  • Update ConfigMap/Secret event-based change detectors to support HA mode (deferred start, explicit start/stop, checkpoint replay).
  • Add/adjust integration and unit tests plus documentation for HA enablement, RBAC, and deployment replicas.

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/native_client/NativeClientKubernetesFixture.java Add HA env vars + configurable replicas; wait for expected replica count.
spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/k3s/NativeClientIntegrationTestExtension.java Wire scenario HA flags/replicas into fixture calls.
spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/k3s/NativeClientIntegrationTest.java Extend test annotation with HA + replicas options.
spring-cloud-kubernetes-integration-tests/spring-cloud-kubernetes-k8s-client-reload/src/test/java/org/springframework/cloud/kubernetes/k8s/client/reload/it/K8sClientConfigMapLabelEventTriggeredIT.java Update asserted log message for informer startup.
spring-cloud-kubernetes-integration-tests/spring-cloud-kubernetes-k8s-client-reload/src/test/java/org/springframework/cloud/kubernetes/k8s/client/reload/it/K8sClientConfigMapEventTriggeredIT.java Update asserted log message for informer startup.
spring-cloud-kubernetes-integration-tests/spring-cloud-kubernetes-k8s-client-configuration-watcher/src/test/java/org/springframework/cloud/kubernetes/configuration/watcher/KubernetesClientConfigurationWatcherHaIT.java New k3s-based HA integration test (leader lease + replay).
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/test/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/LeaseConfigurationWatcherStateStoreTests.java New unit tests for lease read/create/update failure modes.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/test/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/ConfigurationWatcherHACoordinatorTests.java New unit tests for leader start/stop wiring and validation.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/test/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/ConfigurationWatcherHAAutoConfigurationTests.java New auto-config tests for HA coordinator conditional creation.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/test/java/org/springframework/cloud/kubernetes/configuration/watcher/ConfigurationWatcherConfigurationPropertiesTests.java Verify new HA properties defaults + binding.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports Register HA auto-configuration.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/SecretsWatcherChangeDetector.java Pass HA enabled flag into base change detector.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/LeaseConfigurationWatcherStateStore.java New lease-backed state store implementation.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/ConfigurationWatcherStateStore.java New state store interface.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/ConfigurationWatcherState.java New state record (per-namespace checkpoints).
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/ConfigurationWatcherHaProperties.java New HA properties (enabled, lease name/namespace).
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/ConfigurationWatcherHACoordinator.java New coordinator listening to leader election events.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/ConfigurationWatcherHAAutoConfiguration.java New HA auto-config (store + coordinator beans).
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/ConditionalOnConfigurationWatcherHAEnabled.java New conditional annotation for HA enablement.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ConfigurationWatcherConfigurationProperties.java Add HA properties to watcher configuration properties.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ConfigMapWatcherChangeDetector.java Pass HA enabled flag into base change detector.
spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/pom.xml Add leader-election dependency for HA support.
spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/reload/KubernetesClientEventBasedSecretsChangeDetectorTests.java Expand tests for HA deferred start + resourceVersion behavior.
spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/reload/KubernetesClientEventBasedConfigMapChangeDetectorTests.java Expand tests for HA deferred start + resourceVersion behavior.
spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/reload/InformerResourceVersionResolverTests.java New unit tests for resolver semantics in HA/non-HA.
spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/reload/SecretResourceEventHandler.java Add optional resourceVersion writer callback.
spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/reload/NamespaceAndResourceVersion.java New value record for persisted checkpoints.
spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/reload/KubernetesClientEventBasedSecretsChangeDetector.java Add HA mode: deferred informers + explicit start/stop + replay logic.
spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/reload/KubernetesClientEventBasedConfigMapChangeDetector.java Add HA mode: deferred informers + explicit start/stop + replay logic.
spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/reload/InformerResourceVersionResolver.java New helper to consume checkpoints once per namespace.
spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/reload/ConfigMapResourceEventHandler.java Add optional resourceVersion writer callback.
docs/modules/ROOT/pages/spring-cloud-kubernetes-configuration-watcher.adoc Document HA enablement, leases, RBAC, and replicas.
Suppressed comments (2)

spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/reload/KubernetesClientEventBasedSecretsChangeDetector.java:121

  • Log message mentions "configmap informer" in the Secrets change detector; should refer to secret informers to avoid confusion in HA mode.
			LOG.info(() -> "config watcher HA is enabled : deferring configmap informer startup "
					+ "until leadership is acquired");

spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ha/LeaseConfigurationWatcherStateStore.java:166

  • parseResourceVersions() throws "Invalid ConfigMap resource version entry" even when parsing the Secret annotation, which can mislead debugging. The message should be resource-type agnostic.
				throw new IllegalStateException("Invalid ConfigMap resource version entry: " + entry);
			}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

wind57 added 2 commits August 28, 2026 09:15
Signed-off-by: wind57 <eugen.rabii@gmail.com>

@ryanjbaxter ryanjbaxter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might also be an opportunity to unify the logic in KubernetesClientEventBasedConfigMapChangeDetector and KubernetesClientEventBasedSecretsChangeDetector as well as ConfigMapResourceEventHandler and SecretResourceEventHandler since they are mostly identifcal.

Comment thread docs/modules/ROOT/pages/spring-cloud-kubernetes-configuration-watcher.adoc Outdated
}

@Override
public ConfigurationWatcherState readOrCreate() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that necessary? In reality if a value was updated while there was no monitor and then updated again the app likely only cares about what the latest value is and don't really care about what it was changed to inbetween.

@wind57

wind57 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

I stated looking at your comments, some of them can be made as separate PRs so that its easier to review this way. I'll tag you when those are ready.

@wind57 wind57 changed the title fix-1828: started work fix-1828: add HA support for config watcher Sep 16, 2026
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
@wind57

wind57 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor Author

Is that necessary? In reality if a value was updated while there was no monitor and then updated again the app likely only cares about what the latest value is and don't really care about what it was changed to inbetween.

I've been debating about this also, but decided initially to start with it, I really happy you commented on this and now that I looked for a few days into it, I agree, it is not needed so I dropped the idea in general. Meaning there is no storage at all happening in the code anymore.

Could we not use KubernetesNamespaceProvider here instead of falling back to default?

Per the comment above, we do not require this anymore at all.

This will march the resource consumed before the actual API call to consume the resource, what happens if the subsiquent call does not actually succeed? The resource will be marked consumed when it hasn't been

Same as above, this logic was dropped.

@ryanjbaxter I've simplified the code and dropped the idea of storing resource versions. You can have another look and also trigger copilot if you may. thank you

Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>
Signed-off-by: wind57 <eugen.rabii@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +54 to +56
ConfigurationWatcherHACoordinator configurationWatcherHACoordinator(
ObjectProvider<@NonNull KubernetesClientEventBasedConfigMapChangeDetector> configMapDetector,
ObjectProvider<@NonNull KubernetesClientEventBasedSecretsChangeDetector> secretsDetector) {
Comment on lines +69 to +70
configMapDetector.ifAvailable(KubernetesClientEventBasedConfigMapChangeDetector::start);
secretsDetector.ifAvailable(KubernetesClientEventBasedSecretsChangeDetector::start);
ObjectProvider<@NonNull KubernetesClientEventBasedConfigMapChangeDetector> configMapDetector,
ObjectProvider<@NonNull KubernetesClientEventBasedSecretsChangeDetector> secretsDetector) {
if (configMapDetector.getIfAvailable() == null && secretsDetector.getIfAvailable() == null) {
throw new IllegalStateException(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under what condition would both change detectors not be available? It feels like that it would be a prerequisite to enabling HA

ObjectProvider<@NonNull KubernetesClientEventBasedConfigMapChangeDetector> configMapDetector,
ObjectProvider<@NonNull KubernetesClientEventBasedSecretsChangeDetector> secretsDetector) {
if (configMapDetector.getIfAvailable() == null && secretsDetector.getIfAvailable() == null) {
throw new IllegalStateException(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a similar vein what happens if the user sets leader.election.publish-events=false?

@Documented
@Inherited
@ConditionalOnProperty(value = LEADER_ELECTION_ENABLED_PROPERTY, havingValue = "true", matchIfMissing = false)
@Conditional(LeaderElectionEnabledCondition.class)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change will impact Fabric8LeaderAutoConfiguration. If for some reason spring.cloud.kubernetes.configuration.watcher.ha.enabled=true leaks into an app where Fabric8LeaderAutoConfiguration is being used today @ConditionalOnLeaderElectionEnabled will be true meaning @ConditionalOnLeaderElectionDisabled will be false and Fabric8LeaderAutoConfiguration won't be used.

@ConditionalOnLeaderElectionDisabled
public class Fabric8LeaderAutoConfiguration {

@Inherited
@ConditionalOnProperty(value = LEADER_ELECTION_ENABLED_PROPERTY, havingValue = "true", matchIfMissing = false)
@Conditional(LeaderElectionEnabledCondition.class)
public @interface ConditionalOnLeaderElectionEnabled {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we do this and get rid of LeaderElectionEnabledCondition?

@Conditional(ConditionalOnLeaderElectionEnabled.OnLeaderElectionEnabled.class)
public @interface ConditionalOnLeaderElectionEnabled {

    class OnLeaderElectionEnabled extends AnyNestedCondition {
        OnLeaderElectionEnabled() {
            super(ConfigurationPhase.REGISTER_BEAN);
        }

        @ConditionalOnProperty(prefix = LeaderUtils.LEADER_ELECTION_PROPERTY_PREFIX, name = "enabled", havingValue = "true")
        static class OnLeaderElectionPropertyEnabled { }

        @ConditionalOnConfigurationWatcherHAEnabled
        static class OnConfigurationWatcherHaEnabled { }
    }
}

}
}

public final void start() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be public? And if so can we guard against it being called more than once and precent starting the informers again?

this::onEvent);

// HA enabled for configuration watcher
private final boolean haEnabled;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having any HA logic baked into this class when it is also used when not doing HA feels wrong to me. Maybe we can subclass it and stick the logic there?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an interesting idea and initially I really liked it.

we could easily have KubernetesClientEventBasedConfigMapChangeDetector and for example : KubernetesClientEventBasedConfigMapHAChangeDetector and their implementations would actually be rather clean ( I tried it ).

But then, the problem would become that lots of new classes would have to be created, because we currently have:

abstract sealed class ConfigMapWatcherChangeDetector extends KubernetesClientEventBasedConfigMapChangeDetector

and there are two implementations of the above: HttpBasedConfigMapWatchChangeDetector and BusEventBasedConfigMapWatcherChangeDetector.

That means we would have to create, for example:

ConfigMapWatcherChangeDetector
    extends KubernetesClientEventBasedConfigMapChangeDetector

HAConfigMapWatcherChangeDetector
    extends KubernetesClientEventBasedHAConfigMapChangeDetector

same applies to Secrets.

The HTTP/Bus concrete detectors also need normal and HA variants so each auto-configuration creates the correct hierarchy, meaning we would need:

HttpBasedConfigMapWatchChangeDetector
    extends ConfigMapWatcherChangeDetector
    // non-HA

HAHttpBasedConfigMapWatchChangeDetector
    extends HAConfigMapWatcherChangeDetector
    // HA

and so on.

I'm going to wait for your input here cause it matters how I work on the other PR comments.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants