Skip to content

refactor: remove deprecated getTerminationTimeoutSeconds and associated #2297

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions docs/documentation/v5-0-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@ permalink: /docs/v5-0-migration
now contains all the utility methods used for event sources naming that were previously defined in
the `EventSourceInitializer` interface.
3. Patching status through `UpdateControl` like the `patchStatus` method now by default
uses Server Side Apply instead of simple patch. To use the former approach, use the feature flag
uses Server Side Apply instead of simple patch. To use the former approach, use the feature flag
in [`ConfigurationService`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java#L400-L400)
!!! IMPORTANT !!!
Migration from a non-SSA based controller to SSA based controller can cause problems, due to known issues.
See the following [integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L71-L82) where it is demonstrated.
Also, the related part of a [workaround](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L110-L116).
Migration from a non-SSA based controller to SSA based controller can cause problems, due to known issues.
See the
following [integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L71-L82)
where it is demonstrated. Also, the related part of
a [workaround](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L110-L116).
4. `ManagedDependentResourceContext` has been renamed to `ManagedWorkflowAndDependentResourceContext` and is accessed
via the accordingly renamed `managedWorkflowAndDependentResourceContext` method.
5. `ResourceDiscriminator` was removed. In most of the cases you can just delete the discriminator, everything should
work without it by default. To optimize and handle special cases see the relevant section in [Dependent Resource documentation](/docs/dependent-resources#multiple-dependent-resources-of-same-type).
6. `ConfigurationService.getTerminationTimeoutSeconds` and associated overriding mechanism have been removed,
use `Operator.stop(Duration)` instead.
7. `Operator.installShutdownHook()` has been removed, use `Operator.installShutdownHook(Duration)` instead
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,6 @@ private static ConfigurationService initConfigurationService(KubernetesClient cl
return ConfigurationService.newOverriddenConfigurationService(overrider);
}

/**
* Uses {@link ConfigurationService#getTerminationTimeoutSeconds()} for graceful shutdown timeout
*
* @deprecated use the overloaded version with graceful shutdown timeout parameter.
*
*/
@Deprecated(forRemoval = true)
public void installShutdownHook() {
installShutdownHook(Duration.ofSeconds(configurationService.getTerminationTimeoutSeconds()));
}

/**
* Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. Note
* that graceful shutdown is usually not needed, but some {@link Reconciler} implementations might
Expand All @@ -120,6 +109,7 @@ public void installShutdownHook() {
* @param gracefulShutdownTimeout timeout to wait for executor threads to complete actual
* reconciliations
*/
@SuppressWarnings("unused")
public void installShutdownHook(Duration gracefulShutdownTimeout) {
if (!leaderElectionManager.isLeaderElectionEnabled()) {
Runtime.getRuntime().addShutdownHook(new Thread(() -> stop(gracefulShutdownTimeout)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,22 +170,6 @@ default int minConcurrentWorkflowExecutorThreads() {
return MIN_DEFAULT_WORKFLOW_EXECUTOR_THREAD_NUMBER;
}

int DEFAULT_TERMINATION_TIMEOUT_SECONDS = 10;

/**
* Retrieves the number of seconds the SDK waits for reconciliation threads to terminate before
* shutting down.
*
* @deprecated use {@link io.javaoperatorsdk.operator.Operator#stop(Duration)} instead. Where the
* parameter can be passed to specify graceful timeout.
*
* @return the number of seconds to wait before terminating reconciliation threads
*/
@Deprecated(forRemoval = true)
default int getTerminationTimeoutSeconds() {
return DEFAULT_TERMINATION_TIMEOUT_SECONDS;
}

default Metrics getMetrics() {
return Metrics.NOOP;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public class ConfigurationServiceOverrider {
private Integer concurrentWorkflowExecutorThreads;
private Integer minConcurrentWorkflowExecutorThreads;
private Cloner cloner;
private Integer timeoutSeconds;
private Boolean closeClientOnStop;
private KubernetesClient client;
private ExecutorService executorService;
Expand Down Expand Up @@ -91,11 +90,6 @@ public ConfigurationServiceOverrider withResourceCloner(Cloner cloner) {
return this;
}

public ConfigurationServiceOverrider withTerminationTimeoutSeconds(int timeoutSeconds) {
this.timeoutSeconds = timeoutSeconds;
return this;
}

public ConfigurationServiceOverrider withMetrics(Metrics metrics) {
this.metrics = metrics;
return this;
Expand Down Expand Up @@ -236,11 +230,6 @@ public int minConcurrentWorkflowExecutorThreads() {
: original.minConcurrentWorkflowExecutorThreads();
}

@Override
public int getTerminationTimeoutSeconds() {
return timeoutSeconds != null ? timeoutSeconds : original.getTerminationTimeoutSeconds();
}

@Override
public Metrics getMetrics() {
return metrics != null ? metrics : original.getMetrics();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public <R extends HasMetadata> R clone(R object) {
}
})
.withConcurrentReconciliationThreads(25)
.withTerminationTimeoutSeconds(100)
.withMetrics(new Metrics() {})
.withLeaderElectionConfiguration(new LeaderElectionConfiguration("newLease", "newLeaseNS"))
.withInformerStoppedHandler((informer, ex) -> {
Expand All @@ -72,8 +71,6 @@ public <R extends HasMetadata> R clone(R object) {
overridden.checkCRDAndValidateLocalModel());
assertNotEquals(config.concurrentReconciliationThreads(),
overridden.concurrentReconciliationThreads());
assertNotEquals(config.getTerminationTimeoutSeconds(),
overridden.getTerminationTimeoutSeconds());
assertNotEquals(config.getExecutorService(), overridden.getExecutorService());
assertNotEquals(config.getWorkflowExecutorService(), overridden.getWorkflowExecutorService());
assertNotEquals(config.getMetrics(), overridden.getMetrics());
Expand Down
Loading