Skip to content

Comments

feat(T7206): Fix vyos default config path#89

Merged
dmbaturin merged 1 commit intovyos:currentfrom
Bartosz-lab:current
Aug 19, 2025
Merged

feat(T7206): Fix vyos default config path#89
dmbaturin merged 1 commit intovyos:currentfrom
Bartosz-lab:current

Conversation

@Bartosz-lab
Copy link
Contributor

Proposed Commit Message

Resolves problem with wrong default config in cloud-init

Check https://vyos.dev/T7206 for mor info.

LP: #T7206

Additional Context

Test Steps

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

Hi,
I wanted to test this changes, but i don't have idea how to duild this package myself.
Could you provide me instructions how to build it as .deb package?

@github-actions
Copy link

github-actions bot commented Aug 6, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Bartosz-lab
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

vyosbot added a commit to vyos/vyos-cla-signatures that referenced this pull request Aug 6, 2025
@sever-sever sever-sever requested review from Copilot and zdc August 11, 2025 11:10
Copy link

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 refactors VyOS configuration file handling by introducing a shared utility module to standardize config file path resolution and saving operations. It addresses an issue with wrong default config paths in cloud-init for VyOS systems.

  • Extracts common VyOS configuration file logic into a dedicated vyos_util.py module
  • Centralizes config file path resolution with fallback to multiple default locations
  • Standardizes configuration saving across VyOS-related modules

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
cloudinit/config/vyos_util.py New utility module providing centralized VyOS config file path resolution and saving functions
cloudinit/config/cc_vyos_userdata.py Refactored to use the new utility module for config file operations
cloudinit/config/cc_vyos.py Updated to use centralized config file handling from the utility module

Copy link
Contributor

@zdc zdc left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

The solution you propose seems overly complicated for the issue it addresses. While it could work with some adjustments, the Cloud‑Init structure limits us here and forces us to keep modules self‑sustainable - so we should avoid introducing vyos_util.py.

I would recommend simplifying it as follows:

  1. Update the path to the default config file in both modules: change /opt/vyatta/etc/config.boot.default/usr/share/vyos/config.boot.default.
  2. Add a new variable in both modules: CFG_FILE_SAVEPATH = '/opt/vyatta/etc/config/config.boot'
  3. Use that variable in the save operation:
    with open(CFG_FILE_SAVEPATH, 'w') as f:

That resolves the problem with just three lines of code changes per module.

Please also reformat the commit description according to the requirements (header + description of the problem solved or changes made).

@Bartosz-lab
Copy link
Contributor Author

Hi, code updated.

@sever-sever sever-sever requested a review from zdc August 17, 2025 11:30
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.

Now it's much simpler, should be good to go.

Copy link
Contributor

@zdc zdc left a comment

Choose a reason for hiding this comment

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

Thank you for the update! It looks good to go.

@dmbaturin dmbaturin merged commit 11d4c47 into vyos:current Aug 19, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants