Skip to content
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

helm: Allow configuration of probe timers #16584

Merged
merged 1 commit into from Jul 3, 2021

Conversation

jonkerj
Copy link
Contributor

@jonkerj jonkerj commented Jun 17, 2021

This PR makes the period and threshold of the startup, readiness and liveness probe configurable. It also makes the tolerance of the startupProbe roughly equivalent to the initialDelay construction.

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!
Allow configuration of probe timers in Helm chart

@jonkerj jonkerj requested a review from a team as a code owner June 17, 2021 15:11
@jonkerj jonkerj requested review from a team and kaworu June 17, 2021 15:11
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 17, 2021
Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I only have one small comment, I'm fine either case.

install/kubernetes/cilium/values.yaml Outdated Show resolved Hide resolved
@jonkerj jonkerj force-pushed the configurable-probe-parameters branch from 10236b8 to 6cfa3a3 Compare June 18, 2021 06:17
@kaworu kaworu added the release-note/misc This PR makes changes that have no direct user impact. label Jun 18, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 18, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.2 Jun 18, 2021
@aanm
Copy link
Member

aanm commented Jun 18, 2021

marking for backport since this is currently broken in environments running k8s >= 1.21

@jonkerj jonkerj force-pushed the configurable-probe-parameters branch from 6cfa3a3 to 0593a2e Compare June 18, 2021 13:51
@jonkerj
Copy link
Contributor Author

jonkerj commented Jun 18, 2021

Fixed the docs, should be good now

@jonkerj
Copy link
Contributor Author

jonkerj commented Jun 23, 2021

Anything else I can (or need to) do?

@aanm
Copy link
Member

aanm commented Jun 23, 2021

test-me-please

1 similar comment
@aanm
Copy link
Member

aanm commented Jun 23, 2021

test-me-please

@aanm
Copy link
Member

aanm commented Jun 23, 2021

@jonkerj no, I think everything will work as fine. Thanks for the PR!

@maintainer-s-little-helper
Copy link

Commit 289bd54719221654ace4be1eb8ad9fc6d77895f9 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jun 28, 2021
This commit makes the period and threshold of the startup, readiness and
liveness probe configurable. It also makes the tolerance of the
startupProbe roughly equivalent to the initialDelay construction.

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
@jonkerj jonkerj force-pushed the configurable-probe-parameters branch from 289bd54 to 84edffb Compare June 28, 2021 07:16
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jun 28, 2021
@jonkerj
Copy link
Contributor Author

jonkerj commented Jun 28, 2021

rebased, fixed conflicts. Quick, merge! 🤣

@aanm
Copy link
Member

aanm commented Jun 28, 2021

test-me-please

@jonkerj
Copy link
Contributor Author

jonkerj commented Jul 1, 2021

I don't think the failing tests are due to my changes, but please let me know if you expect something from me (this is my first Cilium PR).

@aanm aanm added this to Needs backport from master in 1.10.3 Jul 2, 2021
@aanm aanm removed this from Needs backport from master in 1.10.2 Jul 2, 2021
@aanm aanm merged commit 9a73c92 into cilium:master Jul 3, 2021
@aanm aanm moved this from Needs backport from master to Backport done to v1.10 in 1.10.3 Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.10.3
Backport done to v1.10
Development

Successfully merging this pull request may close these issues.

None yet

6 participants