Bug #18487
closed
controller fails to start when vocab properties file is invalid (e.g. duplicate synonyms)
Description
Need to check vocabulary file as part of config-check.
Since config-check is also meant to be usable on nodes that aren't running controller and don't need access to the vocabulary file, either- detect whether this host matches a controller InternalURL, and if not, don't treat ENOENT as an error, or
- don't ever treat ENOENT as an error
Should print out enough information so that the admin can edit the file to work around the problem.
Need to document the monitoring features that were added.
Document controller behavior when the vocab changes at runtime.
Updated by Ward Vandewege about 3 years ago
- Related to Bug #18488: [controller] does not release pg_advisory_lock($1) when it fails to start added
Updated by Peter Amstutz about 3 years ago
- Category set to API
- Description updated (diff)
- Subject changed from [controller] fails to start when vocab properties file is invalid (e.g. duplicate synonyms) to controller fails to start when vocab properties file is invalid (e.g. duplicate synonyms)
Updated by Peter Amstutz about 3 years ago
- Target version changed from 2021-12-08 sprint to 2022-01-05 sprint
Updated by
Updated by Lucas Di Pentima almost 3 years ago
- Target version changed from 2022-01-05 sprint to 2022-01-19 sprint
Updated by Lucas Di Pentima almost 3 years ago
- Related to Idea #17454: Vocabulary checking of properties by API server/controller added
Updated by Lucas Di Pentima almost 3 years ago
- Status changed from New to In Progress
Updated by Lucas Di Pentima almost 3 years ago
- Target version changed from 2022-01-19 sprint to 2022-02-02 sprint
Updated by Lucas Di Pentima almost 3 years ago
Updates at b701b0f38 - branch 18487-vocab-config-check
Test run: developer-run-tests: #2882
- Adds vocabulary checking to
arvados-server config-check, ignoring the special case when there's anAPI.VocabularyPathvalue set, but no file is found, to avoid false positives on non-controller nodes. - Improves error detection by also checking for duplicated JSON keys that were previously ignored.
- Improves error reporting by collecting all the issues before returning, so that the
config-checkcommand and also the/_health/vocabularyendpoint do a full issue report and not annoy the site admin by providing errors one at a time. - Improves the Metadata vocabulary documentation page on the Admin section.
- Adds tests.
Updated by Ward Vandewege almost 3 years ago
Lucas Di Pentima wrote:
Updates at b701b0f38 - branch
18487-vocab-config-check
Test run: developer-run-tests: #2882
- Adds vocabulary checking to
arvados-server config-check, ignoring the special case when there's anAPI.VocabularyPathvalue set, but no file is found, to avoid false positives on non-controller nodes.- Improves error detection by also checking for duplicated JSON keys that were previously ignored.
- Improves error reporting by collecting all the issues before returning, so that the
config-checkcommand and also the/_health/vocabularyendpoint do a full issue report and not annoy the site admin by providing errors one at a time.- Improves the Metadata vocabulary documentation page on the Admin section.
- Adds tests.
In lib/config/cmd.go:
The addition to config-check isn't ideal; if there are any warnings from the checks above it (e.g. if ManagementToken is not set), config-check aborts without even doing the new test, and nothing is printed.
Would it be possible to move the new tests above the diff command, and send their output to logbuf rather than returning immediately so they are handled the same way as other warnings? That would also make the output more uniform, for better or worse we output structured logs from config-check, which is hard to read, but at least it will be consistent.
Otherwise, LGTM, thanks.
Updated by Lucas Di Pentima almost 3 years ago
Updates at 4f1341cf0
Test run: developer-run-tests: #2888
Thanks for the suggestions, I totally missed the logging part.
I've moved the vocabulary checking up a little and use the logger to output the reporting (sadly, it's kind of unreadable now) so that it can be handled the same way as the rest.
Had to adjust a test regexp, too.
Updated by Ward Vandewege almost 3 years ago
Lucas Di Pentima wrote:
Updates at 4f1341cf0
Test run: developer-run-tests: #2888Thanks for the suggestions, I totally missed the logging part.
I've moved the vocabulary checking up a little and use the logger to output the reporting (sadly, it's kind of unreadable now) so that it can be handled the same way as the rest.
Had to adjust a test regexp, too.
LGTM, thanks! I think we need to change the default output for config-check to human readable, perhaps with a machine-friendly output format as an optional flag. But not as part of this ticket.
Updated by Lucas Di Pentima almost 3 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados-private:commit:arvados|107af77a83788ebdd0cbcfdcae91fed44deec11d.