Skip to content

Commit

Permalink
Use number of logs instead of timestamp in get_auditd_config_reload (#…
Browse files Browse the repository at this point in the history
…16723)

Fixes: #16709

change_and_wait_aaa_config_update validates that the auditd service publishes the log audisp-tacplus re-initializing configuration after executing the passed in command.

However, it did this by checking that the timestamp in the log message was different than the previously emitted log.
As shown in #16709 these message can appear within the same second and cause the test to fail.

Instead this PR changes the approach to look at how many logs have been emitted before and after the command rather than the timestamp changes.
  • Loading branch information
arista-nwolfe authored Feb 6, 2025
1 parent d45f0fe commit 323ed51
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 13 deletions.
6 changes: 3 additions & 3 deletions tests/tacacs/test_accounting.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from tests.common.devices.ptf import PTFHost
from tests.common.helpers.tacacs.tacacs_helper import stop_tacacs_server, start_tacacs_server, \
per_command_accounting_skip_versions, remove_all_tacacs_server
from .utils import check_server_received, change_and_wait_aaa_config_update, get_auditd_config_reload_timestamp, \
from .utils import check_server_received, change_and_wait_aaa_config_update, get_auditd_config_reload_line_count, \
ensure_tacacs_server_running_after_ut, ssh_connect_remote_retry, ssh_run_command # noqa: F401
from tests.common.errors import RunAnsibleModuleFail
from tests.common.helpers.assertions import pytest_assert
Expand Down Expand Up @@ -253,15 +253,15 @@ def test_accounting_tacacs_only_some_tacacs_server_down(
# when tacacs config change multiple time in short time
# auditd service may been request reload during reloading
# when this happen, auditd will ignore request and only reload once
last_timestamp = get_auditd_config_reload_timestamp(duthost)
last_line_count = get_auditd_config_reload_line_count(duthost)

duthost.shell("sudo config tacacs timeout 1")
remove_all_tacacs_server(duthost)
duthost.shell("sudo config tacacs add %s --port 59" % invalid_tacacs_server_ip)
duthost.shell("sudo config tacacs add %s --port 59" % tacacs_server_ip)
change_and_wait_aaa_config_update(duthost,
"sudo config aaa accounting tacacs+",
last_timestamp)
last_line_count)

cleanup_tacacs_log(ptfhost, rw_user_client)

Expand Down
17 changes: 7 additions & 10 deletions tests/tacacs/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,27 +65,24 @@ def log_exist(ptfhost, sed_command):
pytest_assert(exist, "Not found data: {} in tacplus server log".format(data))


def get_auditd_config_reload_timestamp(duthost):
def get_auditd_config_reload_line_count(duthost):
res = duthost.shell("sudo journalctl -u auditd --boot --no-pager | grep 'audisp-tacplus re-initializing configuration'") # noqa E501
logger.info("aaa config file timestamp {}".format(res["stdout_lines"]))

if len(res["stdout_lines"]) == 0:
return ""
return len(res["stdout_lines"])

return res["stdout_lines"][-1]


def change_and_wait_aaa_config_update(duthost, command, last_timestamp=None, timeout=10):
if not last_timestamp:
last_timestamp = get_auditd_config_reload_timestamp(duthost)
def change_and_wait_aaa_config_update(duthost, command, last_line_count=None, timeout=10):
if not last_line_count:
last_line_count = get_auditd_config_reload_line_count(duthost)

duthost.shell(command)

# After AAA config update, hostcfgd will modify config file and notify auditd reload config
# Wait auditd reload config finish
def log_exist(duthost):
latest_timestamp = get_auditd_config_reload_timestamp(duthost)
return latest_timestamp != last_timestamp
latest_line_count = get_auditd_config_reload_line_count(duthost)
return latest_line_count > last_line_count

exist = wait_until(timeout, 1, 0, log_exist, duthost)
pytest_assert(exist, "Not found aaa config update log: {}".format(command))
Expand Down

0 comments on commit 323ed51

Please sign in to comment.