Skip to content

Inquiry about operator gracefulshutdown function #2476

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

Closed
10000-ki opened this issue Aug 2, 2024 · 2 comments
Closed

Inquiry about operator gracefulshutdown function #2476

10000-ki opened this issue Aug 2, 2024 · 2 comments
Milestone

Comments

@10000-ki
Copy link
Contributor

10000-ki commented Aug 2, 2024

The current gracefulshutdown support method is as follows.

  public void stop(Duration gracefulShutdownTimeout) throws OperatorException {
    if (!started) {
      return;
    }
    log.info(
        "Operator SDK {} is shutting down...", configurationService.getVersion().getSdkVersion());
    controllerManager.stop();

    configurationService.getExecutorServiceManager().stop(gracefulShutdownTimeout);
    leaderElectionManager.stop();
    if (configurationService.closeClientOnStop()) {
      getKubernetesClient().close();
    }

    started = false;
  }

Receiving timeoutseconds as a method parameter has several disadvantages.

1. Difficult to utilize directly in spring boot

스크린샷 2024-08-02 오후 1 34 57

Difficult to pass parameters to bean destroy configuration

2. Sometimes graceful shutdown is not performed unintentionally

스크린샷 2024-08-02 오후 1 36 57

There are quite a few places that call the stop method, and I don't know if it was intentional, but in most cases, the terminateseconds setting is ignored.

Suggestion

  @Override
  public void stop() throws OperatorException {
    int timeoutSeconds = configurationService.getTerminationTimeoutSeconds();
    if (timeoutSeconds > 0) {
      stop(Duration.ofSeconds(timeoutSeconds));
    } else {
      stop(Duration.ZERO);
    }
  }

configurationService.getTerminationTimeoutSeconds

I know the above setting is no longer supported, but

To solve the two problems above, I think it would be a good idea to continue supporting this setting.

Or any other good ideas?

@csviri csviri added this to the 5.0 milestone Aug 4, 2024
@csviri
Copy link
Collaborator

csviri commented Aug 4, 2024

Hi @10000-ki, Thanks. Yes, this makes sense. We should definitely consider this for v5. @metacosm, what do you think?

@10000-ki would it be possible to create a PR for this (targeting next branch) ?

@10000-ki
Copy link
Contributor Author

10000-ki commented Aug 4, 2024

would it be possible to create a PR for this (targeting next branch) ?

@csviri
Sure I can work on it.
I will work on it, create a PR, and request a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants