Skip to content

feat: add terminationGracePeriodSeconds field#7439

Merged
simonpasquier merged 1 commit intoprometheus-operator:mainfrom
simonpasquier:fix-3433
Apr 15, 2025
Merged

feat: add terminationGracePeriodSeconds field#7439
simonpasquier merged 1 commit intoprometheus-operator:mainfrom
simonpasquier:fix-3433

Conversation

@simonpasquier
Copy link
Contributor

@simonpasquier simonpasquier commented Apr 4, 2025

Description

This change adds the terminationGracePeriodSeconds field to the Alertmanager, Prometheus, PrometheusAgent and ThanosRuler CRDs.

Closes #3433

Type of change

What type of changes does your code introduce to the Prometheus operator? Put an x in the box that apply.

  • CHANGE (fix or feature that would cause existing functionality to not work as expected)
  • FEATURE (non-breaking change which adds functionality)
  • BUGFIX (non-breaking change which fixes an issue)
  • ENHANCEMENT (non-breaking change which improves existing functionality)
  • NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Verification

Please check the Prometheus-Operator testing guidelines for recommendations about automated tests.

Changelog entry

Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.


@simonpasquier simonpasquier requested a review from a team as a code owner April 4, 2025 11:23
@simonpasquier simonpasquier requested a review from slashpai April 7, 2025 21:22
@simonpasquier simonpasquier enabled auto-merge (squash) April 7, 2025 21:22
@simonpasquier
Copy link
Contributor Author

simonpasquier commented Apr 9, 2025

cc @saswatamcode
wrong PR!

@simonpasquier
Copy link
Contributor Author

The e2e failure is unrelated, see #7455

This change adds the `terminationGracePeriodSeconds` field to the
Alertmanager, Prometheus, PrometheusAgent and ThanosRuler CRDs.

Closes prometheus-operator#3433

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
Copy link
Contributor

@slashpai slashpai left a comment

Choose a reason for hiding this comment

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

lgtm just had a question for my understanding :)

// Value must be non-negative integer. The value zero indicates stop immediately via
// the kill signal (no opportunity to shut down) which may lead to data corruption.
//
// Defaults to 120 seconds.
Copy link
Contributor

Choose a reason for hiding this comment

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

@simonpasquier curious why we didn't add default in API validation but as a constant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, we don't use default in the API definition so I wanted to stay consistent. Default values are returned in the response of the Kubernetes API which may cause issues with some clients (e.g. they might interpret the value being present as a need for another update).

Copy link
Contributor

Choose a reason for hiding this comment

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

We do have // +kubebuilder:default: set for many fields do you think we should remove it then for them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that we can remove any of them for compatibility reasons but I'd rather not add more :)

@simonpasquier simonpasquier merged commit a5d494d into prometheus-operator:main Apr 15, 2025
23 checks passed
@simonpasquier simonpasquier deleted the fix-3433 branch April 15, 2025 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hope add terminationGracePeriodSeconds support

2 participants