Container: T7473: Fix show/monitor container log failed when log-driver is journald #4522

opswill · 2025-05-21T11:28:08Z

Change summary

This PR introduces the following changes:

Refactored log-driver for conf mode containers: Refactored the log driver in conf mode containers to allow individual configuration per container. In addition to k8s-file and journald, a none option was added to disable logging.
Fixed op mode logging failures: fix the show/monitor commands failed to display journald logs.

When the container's log-driver is configured to journald, the commands show log container <container_name> and monitor log container <container_name> fail with the error message:

"Error: journald logging only enabled with systemd on linux: no support for this OS yet".

Current Configuration:

vyos@vyos# show container log-driver
 log-driver journald

journald logs are able to retrieve and display:

vyos@vyos# sudo su -
root@vyos:~# journalctl -xeu vyos-container-v2fly.service
May 21 18:56:58 vyos v2fly[21149]: 2025/05/21 18:56:58 [Info] [1140924250] proxy/http: request to Method [GET] Host [10.0.0.1:1082] >
May 21 18:57:03 vyos v2fly[21149]: 2025/05/21 18:57:03 [Info] [3501276925] proxy/http: request to Method [GET] Host [10.0.0.1:1082] >

run vyos op mode commands:

vyos@vyos# run monitor log container v2fly
Error: journald logging only enabled with systemd on linux: no support for this OS yet
[edit]
vyos@vyos# run show  log container v2fly
Error: journald logging only enabled with systemd on linux: no support for this OS yet
[edit]

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Code style update (formatting, renaming)
Refactoring (no functional changes)
Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
Other (please describe):

Related Task(s)

https://vyos.dev/T7473

Related PR(s)

How to test / Smoketest result

I built the vyos-1x deb package installed and tested it, and everything seems good.
smoketest result:

root@vyos:/tmp#  /usr/libexec/vyos/tests/smoke/cli/test_container.py
test_api_socket (__main__.TestContainer.test_api_socket) ... ok
test_basic (__main__.TestContainer.test_basic) ... ok
test_cpu_limit (__main__.TestContainer.test_cpu_limit) ... ok
test_dual_stack_network (__main__.TestContainer.test_dual_stack_network) ... ok
test_ipv4_network (__main__.TestContainer.test_ipv4_network) ... ok
test_ipv6_network (__main__.TestContainer.test_ipv6_network) ... ok
test_name_server (__main__.TestContainer.test_name_server) ... ok
test_network_mtu (__main__.TestContainer.test_network_mtu) ... ok
test_no_name_server (__main__.TestContainer.test_no_name_server) ... ok
test_uid_gid (__main__.TestContainer.test_uid_gid) ... ok

----------------------------------------------------------------------
Ran 10 tests in 162.290s

OK

container with log-driver journald enabled

show logs:

root@vyos:~#  /usr/libexec/vyos/op_mode/container.py  show_log  --name v2fly
May 21 19:21:08 vyos v2fly[21149]: 2025/05/21 19:21:08 [Info] [3196855156] proxy/http: request to Method [GET] Host [10.0.0.1:1082] >
May 21 19:21:09 vyos v2fly[21149]: 2025/05/21 19:21:09 [Info] [751398583] proxy/socks: TCP Connect request to tcp:8.8.8.8:443

monitor logs:

root@vyos:~#  /usr/libexec/vyos/op_mode/container.py  show_log --follow --name v2fly
May 21 19:22:24 vyos v2fly[21149]: 2025/05/21 19:22:24 tcp:10.0.0.1:50082 accepted tcp:8.8.8.8:443 [out-v2all-home]
May 21 19:22:28 vyos v2fly[21149]: 2025/05/21 19:22:28 [Info] [322429111] proxy/http: request to Method [GET] Host [10.0.0.1:1082] with URL [/]

containers with log-driver k8s-file enabled
show logs:

root@vyos:~#  /usr/libexec/vyos/op_mode/container.py  show_log  --name operensty
Container 'operensty' not found.
root@vyos:~#  /usr/libexec/vyos/op_mode/container.py  show_log  --name openresty
2025/05/21 13:53:46 [error] 2#2: *7 connect() to [2406:da14:fba:1600:53d:57cf:e2a1:6939]:443 failed (101: Network is unreachable), context: ngx.timer
2025/05/21 13:53:46 [error] 2#2: *7 [lua] healthcheck.lua:61: errlog(): healthcheck: failed to connect to [2406:da14:fba:1600:53d:57cf:e2a1:6939]:443: network is unreachable, context: ngx.timer

monitor logs:

root@vyos:~#  /usr/libexec/vyos/op_mode/container.py  show_log --follow --name openresty
2025/05/21 13:53:46 [error] 2#2: *7 connect() to [2406:da14:fba:1600:53d:57cf:e2a1:6939]:443 failed (101: Network is unreachable), context: ngx.timer
2025/05/21 13:53:46 [error] 2#2: *7 [lua] healthcheck.lua:61: errlog(): healthcheck: failed to connect to [2406:da14:fba:1600:53d:57cf:e2a1:6939]:443: network is unreachable, context: ngx.timer
2025/05/21 13:53:48 [error] 3#3: *38 connect() to [2406:da14:fba:1600:53d:57cf:e2a1:6939]:443 failed (101: Network is unreachable), context: ngx.timer

container with log-driver none:

vyos@vyos# run show container log openresty
Container "openresty" has disabled logs.

Checklist:

I have read the CONTRIBUTING document
I have linked this PR to one or more Phabricator Task(s)
I have run the components SMOKETESTS if applicable
My commit headlines contain a valid Task id
My change requires a change to the documentation
I have updated the documentation accordingly

github-actions · 2025-05-21T11:28:32Z

❌
PR title does not match the required format

nvollmar · 2025-05-22T13:54:22Z

👍

c-po

Except some minor changes the overall implementation looks good to me.

data/templates/container/containers.conf.j2

interface-definitions/container.xml.in

op-mode-definitions/container.xml.in

…er is journald

data/templates/container/containers.conf.j2

Copilot

Pull Request Overview

This PR refactors how container log drivers are configured per-container and restores show log/monitor log for the journald driver by routing through a new show_log wrapper script.

Added a show_log function in the op-mode script to invoke Podman or journalctl and support a none driver.
Updated conf-mode generation to include --log-driver flags and amended interface and template definitions for the new log-driver option.
Adjusted smoke tests and op-mode XML to call the new show_log command instead of raw podman logs.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/op_mode/container.py	Added `show_log` implementation and `subprocess` import
src/conf_mode/container.py	Inject `--log-driver` into container run args
smoketest/scripts/cli/test_container.py	Set `log-driver` in test and update inspect command
op-mode-definitions/container.xml.in	Changed log commands to call the wrapper script
interface-definitions/container.xml.in	Added `log-driver` leaf with `none` option
data/templates/container/containers.conf.j2	Removed stale log_driver template snippet

Comments suppressed due to low confidence (2)

src/op_mode/container.py:148

The module does not import vyos, so vyos.opmode.InternalError will cause a NameError. Import the vyos package or the exception class.

        raise vyos.opmode.InternalError(f"Error starting logging command: {e} ")

smoketest/scripts/cli/test_container.py:113

There's an extra double-quote at the end of the format string, which will break the command. Remove the redundant quote.

        tmp = cmd(f'podman container inspect {cont_name} --format "{{{{ .HostConfig.LogConfig.Type }}}}""')

src/op_mode/container.py

optimize code

c-po

All changes to the logging driver look solid.

data/templates/container/containers.conf.j2

nvollmar · 2025-06-03T10:27:25Z

I think there was a misunderstanding regarding my comment.
I still would like to keep the option to set the log_driver in the container.conf, with the log driver being set optionally per container.
In my case, I want to set the log_driver to journald once, not for every container individually.

In short:

Keep the global log config option
No default for per container log config option (omit log driver parameter on the container if not set)

I could also rework that in a followup merge request.

github-actions · 2025-06-03T14:11:33Z

❌ Conflicts Found. This pull request has conflicts. Please resolve them before we can evaluate the pull request.

github-actions · 2025-06-04T01:49:22Z

✅ Conflicts Resolved. Conflicts have been resolved. A maintainer will review the pull request shortly.

…rent

opswill · 2025-06-04T02:01:11Z

@nvollmar I have updated the containers' log driver default value to journald, I hope this change can solve your concern.

sever-sever · 2025-06-04T04:59:14Z

Could you check smoketest again?
CI doesn’t like this test
/usr/libexec/vyos/tests/smoke/cli/test_container.py

nvollmar · 2025-06-04T08:44:02Z

Changing the default would accommodate my use case, but might be a breaking change for users currently expecting k8s_file or wanting to use k8s_file on all containers.

I'd still think keeping the global config option (with k8s_file as default to not break users) and having per container optional override would be the best of both.

opswill · 2025-06-04T10:19:06Z

@sever-sever I have fixed the smoketest errors

github-actions · 2025-06-05T06:21:12Z

✅ No issues found in unused-imports check.. Please refer the workflow run

opswill · 2025-06-05T06:24:41Z

@nvollmar I have added a migration script to remove the global log-driver option

nvollmar · 2025-06-05T08:04:31Z

@opswill As said, I'd rather keep the global config option. I don't see a reason to remove that, does not block in any way a per-container option to override.

github-actions · 2025-06-05T08:22:30Z

CI integration ❌ failed!

Details

CI logs

CLI Smoketests (no interfaces) ❌ failed
CLI Smoketests (interfaces only) 👍 passed
Config tests ❌ failed
RAID1 tests 👍 passed
TPM tests 👍 passed

opswill · 2025-06-05T11:46:44Z

@nvollmar

I've followed your suggestions and made the code changes as much as possible. Regarding the global config, I disagree with keeping it.

In my view, the current implementation already gives each container a default value, which effectively acts as a global config. So keeping a separate global config feels redundant.

The only scenario where a global config might help reduce repetitive work is when running many containers on VyOS and needing to change the log driver in bulk. But changes still require a container manually restart to take effect. So this isn't really an issue for VyOS.

You're welcome to describe your use case in more detail or provide further context. But since I'm mainly focused on fixing issues I've encountered while using VyOS, and I think the current implementation works fine, I'll keep this PR open. Let the VyOS maintainers decide whether to merge it or not.

nvollmar · 2025-06-05T13:23:47Z

k8s_file is probably was most people will just use as default, and if you want to transfer logs remotely you'd set it up globally for journald. That's why I added the global configuration option in the first place. So far I don't really see a use case where I want to configure a different log_driver for each container.

Making the log op command working when journald log driver is used is a good fix. I'm also not against having a per-container option, but I don't see a need or benefit to remove the global config option.

I can also make a followup PR to add back the global option and make the per-container optional.

github-actions bot assigned opswill May 21, 2025

github-actions bot added current rebase labels May 21, 2025

opswill force-pushed the current branch from 3d0d6cc to e67430b Compare May 22, 2025 03:30

opswill force-pushed the current branch 3 times, most recently from 491b56b to 2e80fb1 Compare May 26, 2025 11:06

c-po requested changes May 26, 2025

View reviewed changes

c-po added the bp/circinus Create automatic backport for circinus label May 26, 2025

Container: T7473: Fix show/monitor container log failed when log-driv…

527cafb

…er is journald

opswill force-pushed the current branch from 2e80fb1 to 527cafb Compare May 27, 2025 05:46

opswill requested a review from c-po May 27, 2025 05:49

nvollmar reviewed May 27, 2025

View reviewed changes

data/templates/container/containers.conf.j2 Show resolved Hide resolved

opswill mentioned this pull request May 27, 2025

Container: T7473: update document for the log-driver vyos/vyos-documentation#1639

Open

1 task

sever-sever requested a review from Copilot May 27, 2025 19:46

Copilot AI reviewed May 27, 2025

View reviewed changes

src/op_mode/container.py Show resolved Hide resolved

src/op_mode/container.py Show resolved Hide resolved

optimize code

2330bbf

optimize code

c-po approved these changes May 29, 2025

View reviewed changes

data/templates/container/containers.conf.j2 Show resolved Hide resolved

c-po requested a review from nvollmar May 29, 2025 12:44

github-actions bot added the conflicts label Jun 3, 2025

opswill closed this Jun 4, 2025

opswill force-pushed the current branch from 2330bbf to a711ceb Compare June 4, 2025 01:48

github-actions bot removed the conflicts label Jun 4, 2025

Merge branch 'current' of https://github.com/opswill/vyos-1x into cur…

8d51eb8

…rent

opswill reopened this Jun 4, 2025

update log-driver default value to journald

9bbc7d7

opswill force-pushed the current branch 2 times, most recently from c5567ad to 05b7a13 Compare June 4, 2025 07:17

fix smoketest errors

4da8505

opswill force-pushed the current branch from 05b7a13 to 4da8505 Compare June 4, 2025 08:19

opswill force-pushed the current branch from 30b747a to 007bd47 Compare June 5, 2025 06:20

add migration script

a505633

opswill force-pushed the current branch from 007bd47 to a505633 Compare June 5, 2025 06:22

Container: T7473: Fix show/monitor container log failed when log-driver is journald #4522

Are you sure you want to change the base?

Container: T7473: Fix show/monitor container log failed when log-driver is journald #4522

Uh oh!

Conversation

opswill commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change summary

Types of changes

Related Task(s)

Related PR(s)

How to test / Smoketest result

Checklist:

Uh oh!

github-actions bot commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nvollmar commented May 22, 2025

Uh oh!

c-po left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

c-po left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nvollmar commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 3, 2025

Uh oh!

github-actions bot commented Jun 4, 2025

Uh oh!

opswill commented Jun 4, 2025

Uh oh!

sever-sever commented Jun 4, 2025

Uh oh!

nvollmar commented Jun 4, 2025

Uh oh!

opswill commented Jun 4, 2025

Uh oh!

github-actions bot commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

opswill commented Jun 5, 2025

Uh oh!

nvollmar commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 5, 2025

Details

Uh oh!

opswill commented Jun 5, 2025

Uh oh!

nvollmar commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

opswill commented May 21, 2025 •

edited

Loading

github-actions bot commented May 21, 2025 •

edited

Loading

nvollmar commented Jun 3, 2025 •

edited

Loading

github-actions bot commented Jun 5, 2025 •

edited

Loading

nvollmar commented Jun 5, 2025 •

edited

Loading

nvollmar commented Jun 5, 2025 •

edited

Loading