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

api: add CreatedAt to v1.Pod #3797

Merged
merged 2 commits into from Sep 20, 2017

Conversation

Projects
None yet
2 participants
@iaguis
Member

iaguis commented Sep 18, 2017

It might happen that the pod is created but we can't get its start time
(e.g. we can't find the CNI network corresponding to --net=NETWORK).

This means StartedAt will not be set and the kubelet will error out and
ignore the pod, so it won't try to start it again.

Let's introduce CreatedAt to express the time when the pod was created
(even if it doesn't start) to handle this. This works because this time
is available after pod preparation.

We'll need to change rktlet to use CreatedAt instead of StartedAt when
getting a pod sandbox status.

@lucab

This comment has been minimized.

Member

lucab commented Sep 19, 2017

For reference, what's the CRI field this is forwarded to and what is its expected semantics?

@iaguis

This comment has been minimized.

Member

iaguis commented Sep 19, 2017

The CRI field is CreatedAt and it is the "Creation timestamps of the PodSandbox in nanoseconds".

I think it matches better with rkt's creation time than start time.

@lucab

This comment has been minimized.

Member

lucab commented Sep 19, 2017

@iaguis agreed, it makes sense. If we already have some testing around status output can we at least introduce a check for the always >0 invariant to avoid future regressions?

@iaguis

This comment has been minimized.

Member

iaguis commented Sep 19, 2017

@iaguis agreed, it makes sense. If we already have some testing around status output can we at least introduce a check for the always >0 invariant to avoid future regressions?

I can add an invariant check on the function that parses the output rkt status $UUID so we make sure we always have the state and created fields.

Then I can add a test that does rkt app --net=nonexistent sandbox and checks its status.

@iaguis iaguis added the area/cri label Sep 20, 2017

@lucab

This comment has been minimized.

Member

lucab commented Sep 20, 2017

@iaguis I'm personally happy enough with the first check and the always-positive check.

@lucab lucab added the needs/tests label Sep 20, 2017

@lucab lucab added this to the 1.29.0 milestone Sep 20, 2017

iaguis added some commits Sep 18, 2017

api: add CreatedAt to v1.Pod
It might happen that the pod is created but we can't get its start time
(e.g. we can't find the CNI network corresponding to `--net=NETWORK`).

This means StartedAt will not be set and the kubelet will error out and
ignore the pod, so it won't try to start it again.

Let's introduce CreatedAt to express the time when the pod was created
(even if it doesn't start) to handle this. This works because this time
is available after pod preparation.

We'll need to change rktlet to use CreatedAt instead of StartedAt when
getting a pod sandbox status.
@iaguis

This comment has been minimized.

Member

iaguis commented Sep 20, 2017

Updated.

@lucab

lucab approved these changes Sep 20, 2017

LGTM

@iaguis iaguis merged commit 0051bc1 into rkt:master Sep 20, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details

@iaguis iaguis deleted the kinvolk:iaguis/created-at branch Jan 9, 2018

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