Workaround for false positive Rawls healthcheck issue (SCP-5916) #2198
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
set rawls_ok to always be true
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## development #2198 +/- ##
===============================================
- Coverage 70.15% 69.68% -0.48%
===============================================
Files 332 332
Lines 28240 28240
Branches 2377 2377
===============================================
- Hits 19812 19679 -133
- Misses 8281 8414 +133
Partials 147 147
|
eweitz
approved these changes
Jan 31, 2025
app/controllers/site_controller.rb
Outdated
| @@ -666,7 +666,10 @@ def set_firecloud_permissions(study_detached) | |||
| if api_status.is_a?(Hash) | |||
| system_status = api_status['systems'] | |||
| sam_ok = system_status.dig(FireCloudClient::SAM_SERVICE, 'ok') == true # do equality check in case 'ok' node isn't present | |||
| rawls_ok = system_status.dig(FireCloudClient::RAWLS_SERVICE, 'ok') == true | |||
| # 2025-01-31 quick and dirty workaround for false negative Rawls availability issue | |||
There was a problem hiding this comment.
The current phrasing is subtly but importantly incorrect. For reference:
- False positive: non-problem reported as problem
- False negative: problem reported as non-problem
- True positive: problem reported as problem
- True negative: non-problem reported as non-problem
The issue is that Rawls is effectively OK but is reported as not-OK, so the status report is a false positive.
Suggested change
| # 2025-01-31 quick and dirty workaround for false negative Rawls availability issue | |
| # 2025-01-31 quick and dirty workaround for false positive Rawls availability issue |
There was a problem hiding this comment.
Thanks @eweitz. I always think this through the wrong way - ie. Rawls was reporting it was "not OK" (ie. a negative that was incorrect)and so I end up using the wrong term. Updated in 1cbcc606f.
| @@ -1148,7 +1148,10 @@ def check_edit_permissions | |||
|
|
|||
| # check on FireCloud API status and respond accordingly | |||
| def check_firecloud_status | |||
| unless ApplicationController.firecloud_client.services_available?(FireCloudClient::SAM_SERVICE, FireCloudClient::RAWLS_SERVICE) | |||
| # 2025-01-31 quick and dirty workaround for false negative Rawls availability issue | |||
There was a problem hiding this comment.
Suggested change
| # 2025-01-31 quick and dirty workaround for false negative Rawls availability issue | |
| # 2025-01-31 quick and dirty workaround for false positive Rawls availability issue |
There was a problem hiding this comment.
Updated in 1cbcc606f.
eweitz
changed the title
eweitz
changed the title
|
#dsp-core-services updated Rawls to stop checking GoogleGenomics which resolves our issue so we don't need to consider this workaround anymore. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue: false positives checking on Rawls health
Starting 2025-01-31, Rawls health from https://api.firecloud.org/status was not "ok". Discussion with #dsp-core-services confirmed that the GoogleGenomics service is irrelevant now.
Current output from
/statusendpoint:{"ok":false,"systems":{"Thurloe":{"ok":true},"Sam":{"ok":true},"Rawls":{"messages":["BillingProfileManager: CloudSQL: (ok: true, message: none), Sam: (ok: true, message: none)","GoogleGenomics: Timed out after 1 minute waiting for a response from GoogleGenomics"],"ok":false},"Agora":{"ok":true},"GoogleBuckets":{"ok":true},"LibraryIndex":{"ok":true},"OntologyIndex":{"ok":true}}}External mitigation - Rawls to remove GoogleGenomics check
#dsp-core-services is removing that check from Rawls but recommend not using the /status endpoint to check for Rawls health but rather to look for
500errors from actual requests. (see also SCP-5647)Short-term mitigation for SCP - this PR
IF it takes a while for the Rawls change to take effect, we can use this PR to bypass rawls health checks until the change from #dsp-core-services is in production. If the status endpoint has updated (ie. you can access study settings tab in production), this PR is no longer needed.
MANUAL TESTING