-
Notifications
You must be signed in to change notification settings - Fork 880
Add getGCMarkedTime() and return the timestamp in rkt api service. #2402
Conversation
@@ -770,6 +771,8 @@ func (p *pod) getModTime(path string) (time.Time, error) { | |||
return time.Time{}, err | |||
} | |||
|
|||
defer f.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this to right after you open the file?
a9bfc30
to
e4ccc9f
Compare
@iaguis @alban @vcaputo @sjpotter @euank Hmm, I added some changes to the rkt api service test today and expected it to pass. However I found it was not the case: Same thing could happen when Ideas? |
I think we can only mark the pod as finished on I think showing the finished time in rkt list will be confusing for users, it will be empty unless they mark the pod with gc... |
@iaguis OOB discussed with @euank a little bit. Another idea would be adding a flag (e.g. --show-gc-info) to rkt list/status, when the flag is added, then it will require root and show the Finished time. I am not totally happy with all there options.. |
We could make this feel slightly less wrong by not calling it It also makes sense to me to gate it behind a flag (e.g. This does make the feature less obvious, but I'm also not sure that's a real issue. |
@euank gctime or gc-marked-time sounds good to me. so if the pod is not gc marked, then just return nothing. We can also just return this in api service. We do that for the cgroup-path already. |
Maybe for this one, just make rkt api service to return gc time. |
This returns the timestamp when the pod is moved to garbage/exited-garbage.
Updated the PR and description message. |
gcCmd := fmt.Sprintf("%s gc --mark- ctx.Cmd()) | ||
waitOrFail(t, spawnOrFail(t, gcCmd), 0) | ||
|
||
gcTime := time.Now() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since compareTime checks if the times are equal within a 2 second precision, I think the test would be more meaningful if we wait some time before marking the pod. In the rkt list test we wait 4 seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's address this on #2420
LGTM |
Follows #2400
This PR adds getGCMarkedTime(), which when is called, it will return the ctime of the pod's dir if the pod is in exited-garbage or garbage dir.
If the pod is running, not starting or gone, it returns zero.
Also updates the rkt api service test to test the GCMarkedTime is returned correctly.
cc @vcaputo @jonboulle @euank @sjpotter @alban @iaguis