Skip to content

T7363: Add vyconf aware initialization of Config #4505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
May 23, 2025

Conversation

jestabro
Copy link
Contributor

@jestabro jestabro commented May 12, 2025

Change summary

NOTE: this requires PR:
vyos/vyconf#21
and the corresponding update of libvyosconfig/vyos-1x commit hashes.

Add necessary requests and utilities for Config instantiation from vyconfd session data.
This and the supporting PRs provide a basic framework for running smoketests using vyconf.

In addition to what is needed for T7363, this includes the legacy-compatible lock of https://vyos.dev/T7365, and a simple util for enabling the backend during smoketests (cf. vyos/vyos-build#963).

One goal here is to provide the framework for smoketests, to guide remaining integration and the addition of missing features for a full replacement of the backend. Only certain smoketests currently pass; of failures, some are due to known issues, tested but not yet included; some are not yet possible due to the pending conversion of config_mgmt; the remaining are under investigation in subtasks of https://vyos.dev/T7352.

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)

Related PR(s)

vyos/vyconf#21
vyos/libvyosconfig#36

How to test / Smoketest result

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

@jestabro jestabro self-assigned this May 12, 2025
Copy link

github-actions bot commented May 12, 2025

👍
No issues in PR Title / Commit Title

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces vyconf-aware initialization of the Config system and provides new utilities to manage configuration sessions and backend setup for smoketests. Key changes include:

  • Adding the --legacy-config-path flag to the vyconfd systemd service.
  • Implementing new helper scripts for tearing down configuration sessions and enabling/disabling the vyconf backend.
  • Updating the vyconf session module and related configuration sources for improved session management and integration with the new vyconf backend.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/systemd/vyconfd.service Added a new flag in the ExecStart command to support legacy config.
src/helpers/teardown-config-session.py New utility script for tearing down a vyconf session.
src/helpers/set_vyconf_backend.py New script to toggle the vyconf backend for smoketests with match-case usage.
python/vyos/vyconf_session.py Updated session initialization and added config mode decorators.
python/vyos/utils/commit.py Updated commit lock handling and error message formatting.
python/vyos/utils/backend.py New backend utilities for enabling/disabling vyconf backend.
python/vyos/defaults.py Added a new default for vyconf session directory.
python/vyos/configtree.py Minor update initializing a version string.
python/vyos/configsource.py Introduced a new ConfigSourceVyconfSession for vyconf session integration.
python/vyos/configsession.py Updated config session management to incorporate vyconf session logic.
python/vyos/config.py Updated config source selection based on the new vyconf backend.
Comments suppressed due to low confidence (1)

python/vyos/utils/commit.py:44

  • Consider correcting the error message to 'This function needs to be run as root to return correct results!'
raise OSError('This functions needs to be run as root to return correct results!')

Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no objections against the logic. It shouldn't break anything.

@jestabro
Copy link
Contributor Author

Regenerated submodule reference for libvyosconfig update; rebased over current.

jestabro added 12 commits May 22, 2025 13:26
The environment variable _OFR_CONFIGURE is used by bash completion to
setup the config mode environment. We check this setting to coordinate
vyconf config mode and CLI config mode, independent of the legacy
backend Cstore check.
In the absence of Cstore, the env var remains as the sole indication of
config mode for the legacy CLI, and its emulation here.
We maintain compatibility with the legacy commit lock file until all
other references are resolved; this requires a POSIX-type lock instead
of the BSD-type lock of vyos.utils.locking.
@jestabro
Copy link
Contributor Author

jestabro commented May 23, 2025

Now that pr-conflict check workflow is corrected, integration tests can run; this reveals that generated python module files are needed for nosetest import check, so now included in PR.

Copy link

CI integration 👍 passed!

Details

CI logs

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

@jestabro jestabro merged commit 590843d into vyos:current May 23, 2025
15 of 16 checks passed
@github-actions github-actions bot added the mirror-initiated This PR initiated for mirror sync workflow label May 23, 2025
@vyosbot vyosbot added mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants