Skip to content

feat(charts): add startupProbe to cert controller#5297

Merged
Skarlso merged 8 commits intoexternal-secrets:mainfrom
KyriosGN0:probe
Oct 1, 2025
Merged

feat(charts): add startupProbe to cert controller#5297
Skarlso merged 8 commits intoexternal-secrets:mainfrom
KyriosGN0:probe

Conversation

@KyriosGN0
Copy link
Copy Markdown
Contributor

@KyriosGN0 KyriosGN0 commented Sep 10, 2025

Signed-off-by: AvivGuiser avivguiser@gmail.com

Problem Statement

What is the problem you're trying to solve?

This PR adds a startup probe to the cert controller, to make sure that the pod has started up before checking if its ready to serve trrafic

Related Issue

Fixes #2217

Proposed Changes

How do you like to solve the issue and why?

add a startup probe to the cert-controller pod

Format

Please ensure that your PR follows the following format for the title:

feat(scope): add new feature
fix(scope): fix bug
docs(scope): update documentation
chore(scope): update build tool or dependencies
ref(scope): refactor code
clean(scope): provider cleanup
test(scope): add tests
perf(scope): improve performance
desig(scope): improve design

Where scope is optionally one of:

  • charts
  • release
  • testing
  • security
  • templating

Checklist

  • I have read the contribution guidelines
  • All commits are signed with git commit --signoff
  • My changes have reasonable test coverage
  • All tests pass with make test
  • [] I ensured my PR is ready for review with make reviewable

@KyriosGN0
add startupProbe to cert controller
569287b 
Signed-off-by: AvivGuiser <avivguiser@gmail.com>
@github-actions github-actions bot added area/charts Issues / Pull Requests related to our helm charts kind/feature Categorizes issue or PR as related to a new feature. size/xs labels Sep 10, 2025
@KyriosGN0
add tests and config for startup probe
236c21b 
Signed-off-by: AvivGuiser <avivguiser@gmail.com>
@KyriosGN0
update schema and readme
b8db7b3 
Signed-off-by: AvivGuiser <avivguiser@gmail.com>
@KyriosGN0 KyriosGN0 requested a review from Skarlso September 24, 2025 12:44
Skarlso
Skarlso reviewed Sep 24, 2025
View reviewed changes
deploy/charts/external-secrets/values.yaml Outdated
# -- whether to use the readiness probe port for startup probe.
useReadinessProbePort: true
# -- Port for startup probe.
port: 8081
Copy link
Copy Markdown
Contributor

@Skarlso Skarlso Sep 24, 2025

Choose a reason for hiding this comment

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

I believe port can also be a name / a string. This, now defines it as an integer.

Copy link
Copy Markdown
Contributor Author

@KyriosGN0 KyriosGN0 Sep 25, 2025

Choose a reason for hiding this comment

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

fixed

@KyriosGN0
change port of startupProbe to be string instead of int
af4c890 
Signed-off-by: AvivGuiser <avivguiser@gmail.com>
@KyriosGN0 KyriosGN0 requested a review from Skarlso September 25, 2025 20:41
@Skarlso
Copy link
Copy Markdown
Contributor

Skarlso commented Sep 26, 2025

/ok-to-test sha=af4c8901970a762c9fbd3cc477e0f4e7076d5627

@eso-service-account-app
Copy link
Copy Markdown
Contributor

eso-service-account-app bot commented Sep 26, 2025

[Bot] - ❌ e2e for af4c8901970a762c9fbd3cc477e0f4e7076d5627 failed

@Skarlso
Copy link
Copy Markdown
Contributor

Skarlso commented Sep 26, 2025

/ok-to-test sha=f7e8e3d4313d7f0edf60093009463f08919eba1d

@eso-service-account-app
Copy link
Copy Markdown
Contributor

eso-service-account-app bot commented Sep 26, 2025

[Bot] - ✅ e2e for f7e8e3d4313d7f0edf60093009463f08919eba1d passed

Skarlso
Skarlso reviewed Sep 26, 2025
View reviewed changes
deploy/charts/external-secrets/values.yaml Outdated

startupProbe:
# -- Enabled determines if the startup probe should be used or not. By default it's enabled
enabled: true
Copy link
Copy Markdown
Contributor

@Skarlso Skarlso Sep 26, 2025

Choose a reason for hiding this comment

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

I think this should be off by default, not?

Copy link
Copy Markdown
Contributor Author

@KyriosGN0 KyriosGN0 Sep 26, 2025

Choose a reason for hiding this comment

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

@Skarlso imo its not a breaking change so its fine to enable it by default. (I would not think that people relay on not having a startup probe)
but ultimately i believe you have the final decision as maintainers, if you feel it could become a issue that would pop up more question, then its better to have it set to false

Copy link
Copy Markdown
Contributor

@Skarlso Skarlso Sep 26, 2025

Choose a reason for hiding this comment

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

Let's turn it off by default please. I'm sorry, I'm not trying to be nitpicky, you have to understand that this thing is installed by thousands of people constantly. I'm VERY weary about changing the default behaviour in any way. :/ I'm sorry. 🙇

Copy link
Copy Markdown
Contributor Author

@KyriosGN0 KyriosGN0 Sep 26, 2025

Choose a reason for hiding this comment

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

all is good, updated the values to disable it by default

@KyriosGN0
disable the probe by default
787001f 
Signed-off-by: AvivGuiser <avivguiser@gmail.com>
@KyriosGN0 KyriosGN0 requested a review from Skarlso September 29, 2025 10:27
Skarlso and others added 2 commits October 1, 2025 21:36
@Skarlso
update the README
c6316c1 
Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso
Merge branch 'main' into probe
c72d603
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Oct 1, 2025

Please retry analysis of this Pull-Request directly on SonarQube Cloud

@Skarlso Skarlso merged commit d3de83c into external-secrets:main Oct 1, 2025
29 checks passed
asivanadi0 pushed a commit to asivanadi0/external-secrets that referenced this pull request Oct 22, 2025
@KyriosGN0 @asivanadi0
feat(charts): add startupProbe to cert controller (external-secrets#5297
cd62a59 
)

Signed-off-by: Anpoo Sivanadi <sivanadi08@gmail.com>
SamuelMolling pushed a commit to SamuelMolling/external-secrets that referenced this pull request Oct 24, 2025
@KyriosGN0 @SamuelMolling
feat(charts): add startupProbe to cert controller (external-secrets#5297
4b123f5 
)

Signed-off-by: Samuel Molling <samuelmolling@gmail.com>
@nh4ttruong nh4ttruong mentioned this pull request Nov 6, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Skarlso Skarlso Skarlso approved these changes

@jakobmoellerdev jakobmoellerdev jakobmoellerdev left review comments

Assignees

No one assigned

Labels

area/charts Issues / Pull Requests related to our helm charts kind/feature Categorizes issue or PR as related to a new feature. size/m size/s size/xs

Projects

External Secrets
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

certController: implement startupProbe

3 participants

@KyriosGN0 @Skarlso @jakobmoellerdev