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

extend timeout to avoid not ready containers #1325

Merged
merged 3 commits into from
May 28, 2021

Conversation

paulgmiller
Copy link
Contributor

Description

This happens when machines are slow/busy. calico-node containers show as unready.
See issue #1324 but readiness probe timeout seems to be abit short.

For PR author

  • Tests for change.
  • If changing pkg/apis/, run make gen-files
  • If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

@@ -1296,6 +1296,9 @@ func (c *nodeComponent) nodeLivenessReadinessProbes() (*v1.Probe, *v1.Probe) {
}
rp := &v1.Probe{
Handler: v1.Handler{Exec: &v1.ExecAction{Command: readinessCmd}},
//period is default but timeout is half to account for some slow machines
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer this explain why we're not using the default but with a caveat of keeping it less than the period. Maybe something like

Set the TimeoutSeconds greater than the default of 1 to allow additional time on loaded nodes. Keep this time less than the PeriodSeconds.

@tmjd
Copy link
Member

tmjd commented May 27, 2021

Also it looks like you'll need to fix up some tests. I don't know if you can see the test output but if not you can hopefully run make ci to see the test failures.

Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

LGTM

@tmjd tmjd merged commit 0203e06 into tigera:master May 28, 2021
tmjd added a commit to tmjd/operator that referenced this pull request May 28, 2021
* extend timeout to avoid not ready containers that are slow but otherwise fine

Co-authored-by: Erik Stidham <erik@tigera.io>
tmjd added a commit that referenced this pull request May 28, 2021
extend timeout to avoid not ready containers  (pick #1325 r1.17)
tmjd added a commit to tmjd/operator that referenced this pull request Jun 1, 2021
* extend timeout to avoid not ready containers that are slow but otherwise fine

Co-authored-by: Erik Stidham <erik@tigera.io>
tmjd added a commit that referenced this pull request Jun 1, 2021
extend timeout to avoid not ready containers  (pick #1325 r1.18)
@tmjd
Copy link
Member

tmjd commented Jun 16, 2021

Fixes #1324

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

Successfully merging this pull request may close these issues.

3 participants