-
Notifications
You must be signed in to change notification settings - Fork 884
api_service: optionally build systemd-journal support #2868
Conversation
nice! LGTM |
) | ||
|
||
func (s *v1AlphaAPIServer) constrainedGetLogs(request *v1alpha.GetLogsRequest, server v1alpha.PublicAPI_GetLogsServer) error { | ||
return fmt.Errorf("missing host systemd-journal support") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather say logs unsupported
without any reference to systemd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, unsupported logging subsystem
seems better for the generic case.
Just a small nitpick, else lgtm 👍 |
33d4d72
to
46b101c
Compare
) | ||
|
||
func (s *v1AlphaAPIServer) constrainedGetLogs(request *v1alpha.GetLogsRequest, server v1alpha.PublicAPI_GetLogsServer) error { | ||
return fmt.Errorf("unsupported logging subsystem") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is misleading. How about "rkt built without logging support"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also ok for me. Further proposal: api-service has another "not implemented yet" stub for ListenEvents; what about unifying both under a ErrApiNotSupported
= "operation not supported"? @s-urbaniak too broad?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error is more useful for console output rather than an err equality check like io.EOF
. I'd keep them as separate errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhm, ok.
46b101c
to
c2a51c6
Compare
This will probably conflict with #2684, holding it for the moment (to be serialized after that). |
c2a51c6
to
6c62fa3
Compare
This introduces a 'sdjournal' tag and corresponding stubs in api_service, turning libsystemd headers into a soft-dependency. If systemd-journal support is enabled and `sd-journal.h` found at build time, api_service will use it for logs retrieval. This is enabled by default, but downstream can turn it off via `--enable-sdjournal=no`.
6c62fa3
to
e1dd05a
Compare
It looks like other PRs are being blocked by this, and the conflicting one is not landing as quick as expected, so I'm merging this first. |
posthumous LGTM |
This includes the patch from rkt/rkt#2868 and --enable-sdjournal=no is passed to econf when USE=systemd is not enabled. Package-Manager: portage-2.3.0
This introduces a 'sdjournal' tag and corresponding stubs in api_service,
turning libsystemd headers into a soft-dependency.
If systemd-journal support is enabled and
sd-journal.h
found at buildtime, api_service will use it for logs retrieval.
This is enabled by default, but downstream can turn it off via
--enable-sdjournal=no
.Fixes #2866
Ref #2797