Bug #15554

[crunchstat-summary] Fix live log reporting for crunch 2 v1.4+

Added by Tom Morris 2 months ago. Updated about 1 month ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
08/28/2019
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

Currently crunchstat-summary assumes that if the log collection is available, that the container is no longer running, but this is no longer true, so it should be fixed to instead explicitly check the container state and use the live crunchstat data from the log table if the container is still running.


Subtasks

Task #15576: Review 15554-crunchstat-summary-live-log-no-sdkResolvedTom Morris

Task #15598: Review 15554-python-sdk-constantsResolvedEric Biagiotti

Associated revisions

Revision 6de58d02
Added by Tom Morris about 2 months ago

Merge branch '15554-python-sdk-constants'

refs #15554

Arvados-DCO-1.1-Signed-off-by: Tom Morris <>

Revision b1f01918
Added by Tom Morris about 1 month ago

Merge branch '15554-crunchstat-summary-live-log-no-sdk'

Fixes #15554

Arvados-DCO-1.1-Signed-off-by: Tom Morris <>

History

#1 Updated by Tom Morris about 2 months ago

  • Status changed from New to In Progress

#2 Updated by Tom Morris about 2 months ago

  • Target version changed from 2019-08-28 Sprint to 2019-09-11 Sprint

#3 Updated by Tom Morris about 2 months ago

To work around the CI build issue where it installs the published Python SDK, rather than the SDK from the branch being tested, I've split this into two branches, both of which are ready for review, but which need to be merged separately.

15554-python-sdk-constants @3c248f44a39ab0b792a8a91f875c4e09875bf54f just adds the three Python SDK constants
15554-crunchstat-summary-live-log @b3cf6fae4df6d9254a2dfa71a3ba555f4633127f has been rebased on top of it and force pushed

#4 Updated by Eric Biagiotti about 2 months ago

Tom Morris wrote:

15554-crunchstat-summary-live-log @b3cf6fae4df6d9254a2dfa71a3ba555f4633127f has been rebased on top of it and force pushed

It seems as though summarizer.py ln 109 passes ProcessSummarizer a string, but it doesn't look like it can handle it unless I am missing something. If I am correct, we should probably fix it and add test coverage for that case. Other than that it LGTM!

#5 Updated by Tom Morris about 1 month ago

  • Release set to 26

#6 Updated by Tom Morris about 1 month ago

Eric Biagiotti wrote:

It seems as though summarizer.py ln 109 passes ProcessSummarizer a string, but it doesn't look like it can handle it unless I am missing something. If I am correct, we should probably fix it and add test coverage for that case. Other than that it LGTM!

Good catch! I've fixed the bug, but didn't create a new test because it's a Crunch 1 only issue.

I gave up on trying to get the Python SDK dependencies to work with Jenkins. I switched back to a hardcoded string. Not ideal, but I've wasted waaaay too much time on this already.

15554-crunchstat-summary-live-log-no-sd @0c3e58f58b9fbcbbea69c7856ce70f73cbfbd73a

tests pass locally and running on Jenkins here: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1516/

#7 Updated by Lucas Di Pentima about 1 month ago

Updates on the 15554-crunchstat-summary-live-log-no-sdk branch LGTM. Some Jenkins tests failed, but re-ran them locally without issues.

#8 Updated by Tom Morris about 1 month ago

Merged @ b1f01918cfcd953e906d32691ddf76c0253a1948

#9 Updated by Tom Morris about 1 month ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF