[v3-0-test] Allow Remote logging providers to load connections from the API Server (#53719) #53761
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.
Often remote logging is down using automatic instance profiles, but not
always. If you tried to configure a logger by a connection defined in the
metadata DB it would have not worked (it either caused the supervise job to
fail early, or to just behave as if the connection didn't exist, depending on
the hook's behaviour)
Unfortunately, the way of knowing what the default connection ID various hooks
use is not easily discoverable, at least not easily from the outside (we can't
look at
remote.hook
as for most log providers that would try to load theconnection, failing in the way we are trying to fix) so I updated the log
config module to keep track of what the default conn id is for the modern log
providers.
Once we have the connection ID we know (or at least have a good idea that
we've got the right one) we then pre-emptively check the secrets backends for
it, if not found there load it from the API server, and then either way. if we
find a connection we put it in the env variable so that it is available.
The reason we use this approach, is that are running in the supervisor process
itself, so SUPERVISOR_COMMS is not and cannot be set yet.
(cherry picked from commit e4fb686)
Co-authored-by: Ash Berlin-Taylor [email protected]