FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: support for graceful shutdown based on configuration by 10000-ki · Pull Request #2479 · operator-framework/java-operator-sdk · GitHub

/ java-operator-sdk Public

feat: support for graceful shutdown based on configuration - #2479

Merged
metacosm merged 4 commits into
operator-framework:nextfrom
10000-ki:settingbased-gracefulshutdown
Aug 6, 2024
ConversationCommits (4)ChecksFiles changed
Merged

feat: support for graceful shutdown based on configuration#2479
metacosm merged 4 commits into
operator-framework:nextfrom
10000-ki:settingbased-gracefulshutdown

Conversation

10000-ki commented Aug 4, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

resolved: #2476

What i am working on in this PR

  • Allows configuration of graceful shutdown settings in the operator configuration.
  • If a graceful shutdown timeout value is set, it will be used within the stop method.
  • I have enhanced the documentation related to graceful shutdown.

openshift-ci Bot requested review from adam-sandor and csviri August 4, 2024 15:46
10000-ki commented Aug 4, 2024
View reviewed changes
Comment thread docs/content/en/docs/patterns-and-best-practices/_index.md
.withReconciliationTerminationTimeout(Duration.ofSeconds(5));

final var operator = new Operator(overridden);
```

10000-ki Aug 4, 2024 •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

In my opinion, it is beneficial for users to be aware of this feature, so I have documented it.

However, in most users' cases, the graceful shutdown feature may not be necessary.
If you think this documentation is unnecessary, please let me know and I will remove it.

10000-ki commented Aug 4, 2024
View reviewed changes
Comment thread ...ramework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java
* @return The duration of time to wait before terminating the reconciliation threads
*/
default Duration reconciliationTerminationTimeout() {
return Duration.ZERO;

10000-ki Aug 4, 2024

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The default is immediate shutdown

10000-ki commented Aug 4, 2024
View reviewed changes
Comment thread operator-framework/src/test/java/io/javaoperatorsdk/operator/GracefulStopIT.java
.build();

@Test
void stopsGracefullyWIthTimeout() {

10000-ki Aug 4, 2024

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I also corrected a typo in the method name while I was at it.

10000-ki commented Aug 4, 2024
View reviewed changes
Comment thread ...tor-framework/src/test/java/io/javaoperatorsdk/operator/GracefulStopWithConfigurationIT.java Outdated
LocallyRunOperatorExtension operator =
LocallyRunOperatorExtension.builder()
.withConfigurationService(o -> o.withCloseClientOnStop(false)
.withReconciliationTerminationTimeout(Duration.ofSeconds(RECONCILER_SLEEP)))

10000-ki Aug 4, 2024 •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

.withReconciliationTerminationTimeout(Duration.ofMillis(RECONCILER_SLEEP)))

User can configure the graceful shutdown time in the Operator settings.

10000-ki changed the title Support for graceful shutdown based on configuration feat: support for graceful shutdown based on configuration Aug 5, 2024
feat: support for graceful shutdown based on configuration
b288737 
Support for graceful shutdown based on configuration

Signed-off-by: 10000-ki <10000ki6472@gmail.com>

Fix naming

Signed-off-by: 10000-ki <10000ki6472@gmail.com>

Fix lint error

Signed-off-by: 10000-ki <10000ki6472@gmail.com>

Fix lint error

Signed-off-by: 10000-ki <10000ki6472@gmail.com>

Fix lint error

Signed-off-by: 10000-ki <10000ki6472@gmail.com>

Fix test duration

Signed-off-by: 10000-ki <10000ki6472@gmail.com>

csviri commented Aug 6, 2024

Copy link
Copy Markdown
Collaborator

I was wondering if we should keep the stop(timeout) variant at all. What do you think? @10000-ki @metacosm

10000-ki commented Aug 6, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I was wondering if we should keep the stop(timeout) variant at all. What do you think?

@csviri
In my opinion

Configuration-based graceful shutdown can be difficult to modify at runtime, so supporting features like stop(timeout) might be useful in test code.

However, from an actual user's perspective, it doesn't seem very beneficial. It might be clearer to specify the graceful shutdown timeout value when setting up the Operator Configuration

Personally, I think it's fine to no longer support it, but there may be users using it in previous versions. Therefore, I didn't make any changes to it in this PR.

Should we mark it as deprecated??

metacosm commented Aug 6, 2024

Copy link
Copy Markdown
Collaborator

I was wondering if we should keep the stop(timeout) variant at all. What do you think? @10000-ki @metacosm

I think it's fine to remove it and use whatever value is configured.

csviri commented Aug 6, 2024

Copy link
Copy Markdown
Collaborator

I was wondering if we should keep the stop(timeout) variant at all. What do you think? @10000-ki @metacosm

I think it's fine to remove it and use whatever value is configured.

ok, agreed, @10000-ki could you update the PR pls?

10000-ki added 3 commits August 6, 2024 22:57
feat: remove parameter-based graceful shutdown function
4152258 
Signed-off-by: 10000-ki <10000ki6472@gmail.com>
feat: do not use static import
200ab73 
Signed-off-by: 10000-ki <10000ki6472@gmail.com>
feat: edit guide docs
1c7ddfb 
Signed-off-by: 10000-ki <10000ki6472@gmail.com>

10000-ki commented Aug 6, 2024

Copy link
Copy Markdown
Contributor Author

ok, agreed, @10000-ki could you update the PR pls?

@csviri @metacosm

All changes have been applied to the current PR
Thank you for the review.

metacosm merged commit e1f2da0 into operator-framework:next Aug 6, 2024

metacosm commented Aug 6, 2024

Copy link
Copy Markdown
Collaborator

Thank you!

csviri commented Aug 6, 2024

Copy link
Copy Markdown
Collaborator

Awesome, thank you!

csviri pushed a commit that referenced this pull request Aug 8, 2024
feat: support for graceful shutdown based on configuration (#2479)
dd104ab 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
csviri mentioned this pull request Aug 14, 2024
Merged
csviri pushed a commit that referenced this pull request Aug 15, 2024
feat: support for graceful shutdown based on configuration (#2479)
bbd492a 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
metacosm pushed a commit that referenced this pull request Aug 29, 2024
feat: support for graceful shutdown based on configuration (#2479)
4f444c6 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
csviri mentioned this pull request Sep 17, 2024
Merged
csviri pushed a commit that referenced this pull request Sep 20, 2024
feat: support for graceful shutdown based on configuration (#2479)
0fe4ebc 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
metacosm pushed a commit that referenced this pull request Oct 10, 2024
feat: support for graceful shutdown based on configuration (#2479)
3ea3e96 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
metacosm pushed a commit that referenced this pull request Nov 5, 2024
feat: support for graceful shutdown based on configuration (#2479)
5f27945 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
metacosm pushed a commit that referenced this pull request Nov 6, 2024
feat: support for graceful shutdown based on configuration (#2479)
270c752 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
csviri pushed a commit that referenced this pull request Nov 13, 2024
feat: support for graceful shutdown based on configuration (#2479)
889d454 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
metacosm pushed a commit that referenced this pull request Nov 19, 2024
feat: support for graceful shutdown based on configuration (#2479)
efd3cf8 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
metacosm pushed a commit that referenced this pull request Nov 20, 2024
feat: support for graceful shutdown based on configuration (#2479)
c0298fc 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
metacosm pushed a commit that referenced this pull request Nov 27, 2024
feat: support for graceful shutdown based on configuration (#2479)
3c82578 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
This was referenced Dec 3, 2024
Merged
Closed
csviri pushed a commit that referenced this pull request Dec 6, 2024
feat: support for graceful shutdown based on configuration (#2479)
9390d89 
---------

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
This was referenced May 25, 2026
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

metacosm metacosm approved these changes
adam-sandor Awaiting requested review from adam-sandor
csviri Awaiting requested review from csviri

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL