Revist address_root != "observatory" #327
Merged
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
Also changed OCSAgent to fail if any startup subscriptions fail.
... even when looping on failed connections to influxdb.
This SCF param actually wasn't used anywhere, except optionally in the
ocs-agent-cli (which now simply defaults to {address_root}.registry
and can still be overridden).
The command-line argument remains, but is doc'ed as deprecated and has
no effect on anything.
for more information, see https://pre-commit.ci
BrianJKoopman
added
enhancement
There was a problem hiding this comment.
This is great! However now we have to deal with users configuring the crossbar server, which we avoided originally, bundling a sensible default in the crossbar Docker image we provide (with the observatory. URI.) I think our documentation on this needs some updating.
... in response to requests for "generate_crossbar_config" and "start crossbar". Also update erroneous docs for the former.
Now the crossbar config can be bind-mounted in, or else some envvars are used to generate one from a template before crossbar is started.
Also the SCF docs warn about updating the crossbar config.
|
Thanks -- I think I've addressed all your suggestions now. The connection error now presents a more detailed error message, with advice if possible. |
BrianJKoopman
approved these changes
May 16, 2023
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.
Description
The
address_roothas always been set toobservatoryin OCS installations. But this prefix is configurable in the SCF, and for the most part wasn't hard-coded anywhere in ocs (nor socs).I have removed a small number of hard-coded occurrences of
observatory.. These were in the feed subscription wildcards the registry, aggregator and influxdbpublisher.The address_root was intended to be user configurable; I consider this to be a bugfix.
Some additional changes along the way:
Motivation and Context
At the observatory we'll have a few very similar systems, e.g. sat1 and sat2, running as independent OCS instances. In some situations (influxdb and grafana, primarily) we will have data from all systems available, and so the feeds can't have the exact same names. That means we either need to carefully include "sat1" / "sat2" in every instance-id ... e.g. "observatory.sat1_acu", "observatory.sat2_acu" OR take advantage of the configurable prefix and have the agent addresses be "sat1.acu", "sat2.acu".
How Has This Been Tested?
I ran an OCS setup on a single host, with all the affected agents, plus a fake data agent. Confirmed I could see the data appear in grafana and that data was showing up in aggregator output files. All tests/ pass.
Types of changes
Checklist: