Skip to content
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

Use consistent confd timeout value from all front-ends #605

Merged
merged 3 commits into from
Sep 5, 2024

Conversation

wkz
Copy link
Contributor

@wkz wkz commented Sep 5, 2024

Description

Fix an issue where the system would fallback to failure-config even though a legitimate startup-config was available, but took more than 10s to apply.

Accomplish this by making sure that there is only a single definition of confd's timeout value, and use it in all front-ends.

Checklist

Tick relevant boxes, this PR is-a or has-a:

  • Bugfix
    • Regression tests
    • ChangeLog updates (for next release)
  • Feature
    • YANG model change => revision updated?
    • Regression tests added?
    • ChangeLog updates (for next release)
    • Documentation added?
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

@wkz wkz added the ci:main Build default defconfig, not minimal label Sep 5, 2024
While sysrepo's architecture places the responsibility of choosing a
timeout on each front-end, we want to control it centrally since the
majority of the time is spend actually applying the config to the
system - which is front-end agnostic.

Therefore, create a single definition of the timeout, which can be
included by all front-ends.
This will enable us to align this timeout with the other front-ends in
the system.
Now that we have a unified timeout value that all front-ends should
honor, make sure to do so when loading startup-config and
failure-config during boot.

This fixes an issue where the system would fallback to failure-config
even though a legitimate startup-config was available, but took more
than 10s to apply.
@wkz wkz requested review from troglobit and mattiaswal and removed request for troglobit September 5, 2024 19:20
Copy link
Contributor

@mattiaswal mattiaswal left a comment

Choose a reason for hiding this comment

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

Awesome! I thought we had fixed this for long time ago. 🏒 💥

Copy link
Contributor

@troglobit troglobit left a comment

Choose a reason for hiding this comment

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

Amazing work on tracking down this one! 👍

@wkz wkz merged commit f77f585 into main Sep 5, 2024
4 checks passed
@wkz wkz deleted the startup-cfg-timeout branch September 5, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:main Build default defconfig, not minimal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants