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

Deployer pod may be unable to observe started pod from API #15056

Merged
merged 1 commit into from
Jul 5, 2017

Conversation

smarterclayton
Copy link
Contributor

The code in the deployer pod may reach the lifecycle hook execution
prior to the kubelet being able to update the pod status to "running"
with a valid start time. This caused the deployer pod to panic because
start time was empty.

Since the window for this race is small, if the deployer pod can't see a
start time it can use time.Now() to bound the expiration window. Since
the process is reentrant, we can't use a start time at the beginning of
the deployer process (it's no more accurate that way).

A future change should guarantee the start time is available to the
container (via downward API, probably).

Fixes #15049

[test] @Kargakis @sjennings

The code in the deployer pod may reach the lifecycle hook execution
prior to the kubelet being able to update the pod status to "running"
with a valid start time. This caused the deployer pod to panic because
start time was empty.

Since the window for this race is small, if the deployer pod can't see a
start time it can use `time.Now()` to bound the expiration window. Since
the process is reentrant, we can't use a start time at the beginning of
the deployer process (it's no more accurate that way).

A future change should guarantee the start time is available to the
container (via downward API, probably).
@openshift-bot
Copy link
Contributor

Evaluated for origin test up to f347877

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2973/) (Base Commit: 22888b2) (PR Branch Commit: f347877)

@smarterclayton
Copy link
Contributor Author

Green, easy fix, fixes flakes, everyone else out :(

@Kargakis if you can post review I'd appreciate it and I'll fix anything else.

@smarterclayton smarterclayton merged commit 3a2b566 into openshift:master Jul 5, 2017
@0xmichalis
Copy link
Contributor

lgtm

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