-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
lib: fix outdated candidate configuration issue #4506
lib: fix outdated candidate configuration issue #4506
Conversation
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotest tests on Ubuntu 16.04 i386: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-8017/test Topology Tests failed for Topotest tests on Ubuntu 16.04 i386: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8017/artifact/TOPOI386/ErrorLog/log_topotests.txt Topology tests on Ubuntu 16.04 amd64: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1604-8017/test Topology Tests failed for Topology tests on Ubuntu 16.04 amd64:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8017/artifact/TOPOU1604/ErrorLog/log_topotests.txt Topology tests on Ubuntu 18.04 amd64: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1804-8017/test Topology Tests failed for Topology tests on Ubuntu 18.04 amd64:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8017/artifact/TOPOU1804/ErrorLog/log_topotests.txt Successful on other platforms
Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8017/artifact/TOPOI386/MemoryLeaks/
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change has been verified to work in the case it previously failed. Running thru our internal smoke tests now but all indications are it's a good fix.
Regarding the CI failure, the problem is that |
Even when using the classic CLI mode (i.e. when --tcli is not used), the northbound code still uses vty->candidate_config to perform configuration changes. From the perspective of the user, the running configuration is being edited directly, but under the hood the northbound layer does a full configuration transaction for each command. When the running configuration is edited by a northbound client other than the CLI (e.g. kernel, gRPC), vty->candidate_config might become outdated, and this can lead to lots of weird problems. To fix this, always regenerate vty->candidate_config before each configuration command when using the classic CLI mode. When using the transactional CLI, the user needs to update the candidate manually using the "update" command, otherwise the "commit" command will fail with this error: "% Candidate configuration needs to be updated before commit". Fixes some problems reported by Don after moving an interface from one VRF to another one while zebra is running. Reported-by: Don Slice <dslice@cumulusnetworks.com> Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
97e7c82
to
eaf6705
Compare
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8025/ This is a comment from an automated CI system. CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base4 Static Analyzer issues remaining.See details at |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Even when using the classic CLI mode (i.e. when --tcli is not
used), the northbound code still uses vty->candidate_config
to perform configuration changes. From the perspective of the
user, the running configuration is being edited directly, but
under the hood the northbound layer does a full configuration
transaction for each command. When the running configuration is
edited by a northbound client other than the CLI (e.g. kernel,
gRPC), vty->candidate_config might become outdated, and this can
lead to lots of weird problems. To fix this, always regenerate
vty->candidate_config before each configuration command when
using the classic CLI mode. When using the transactional CLI,
the user needs to update the candidate manually using the "update"
command, otherwise the "commit" command will fail with this error:
"% Candidate configuration needs to be updated before commit".
Fixes some problems reported by Don after moving an interface from
one VRF to another one while zebra is running.
Reported-by: Don Slice dslice@cumulusnetworks.com
Signed-off-by: Renato Westphal renato@opensourcerouting.org