[ZTP] Add __ztp_profile_loaded guard to __discoverOnly() - #5
Open
gord1306 wants to merge 1 commit into
Open
Conversation
__discoverOnly() has no __ztp_profile_loaded guard, unlike the otherwise equivalent __loadZTPProfile(). In ZTPEngine.executeLoop() the discovery loop calls __discoverOnly() on the use_config_db path on every iteration (discovery-interval, 10s by default), and ztp-profile.sh discoverOnly unconditionally runs "systemctl restart rsyslog" and "systemctl restart interfaces-config". Running ztp-engine.py with -o/--use-config-db therefore restarts networking roughly every 10 seconds for the whole discovery phase, repeatedly killing the DHCP clients and tearing down the in-band interfaces instead of setting them up once. Guard __discoverOnly() with __ztp_profile_loaded, mirroring __loadZTPProfile(). Periodic re-discovery is unaffected: executeLoop() still restarts networking every restart-ztp-interval (300s default) and __forceRestartDiscovery() resets the flag so discovery setup is redone after a forced restart. All side effects of the discoverOnly branch (rsyslog configuration and restart, the static ZTP|mode CONFIG_DB fields, the rsyslog exit-hook symlink, dhcp_policy_create and the interfaces-config restart) are one time and persistent, so performing them once is sufficient. Signed-off-by: gord_chen <gord_chen@edge-core.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why I did it
__discoverOnly()has no__ztp_profile_loadedguard, unlike the otherwise equivalent__loadZTPProfile(). InZTPEngine.executeLoop()the discovery loop picks one of the two depending onuse_config_db:use_config_dbcomes from the-o/--use-config-dbcommand line flag (ztp-engine.py:940). On that path__discoverOnly()is called on every iteration of the discovery loop (discovery-interval, 10 s by default), andztp-profile.sh discoverOnly(ztp-profile.sh:232-258) unconditionally runssystemctl restart rsyslogandsystemctl restart interfaces-config.Purpose
Running
ztp-engine.py -otherefore restarts networking roughly every 10 seconds for the whole discovery phase, repeatedly killing the DHCP clients and tearing down the in-band interfaces instead of setting them up once. Eachinterfaces-configrestart drops the in-band netdevs, which breaks anything that runs on them.Guard
__discoverOnly()with__ztp_profile_loaded, mirroring__loadZTPProfile().Possible concerns and why they do not apply
executeLoop()still restarts networking everyrestart-ztp-interval(300 s default); that is an independent code path and is untouched.__forceRestartDiscovery()resets__ztp_profile_loadedtoFalse, so the setup is redone, exactly as it is for__loadZTPProfile().discoverOnlybranch were checked individually and are one-time and persistent: the rsyslog configuration file andsystemctl restart rsyslog, the four staticZTP|modeCONFIG_DB fields, the rsyslog exit-hook symlink,dhcp_policy_create, and theinterfaces-configrestart. Performing them once is sufficient.use_config_dbis fixed for the lifetime ofexecuteLoop(), decided once from the command line inmain(), so the__loadZTPProfile()and__discoverOnly()paths are mutually exclusive.How to verify it
Run ZTP discovery with
-o/--use-config-dband count the networking restarts during the discovery phase:Before this change
interfaces-configis restarted about every 10 seconds. After it, once during discovery setup, then only on therestart-ztp-intervalcadence or after a forced restart.Notes
tests/test_ztp_engine.pyexercises the__loadZTPProfile()path, so the existing tests are unaffected but this change is not covered by them. The test suite could not be executed in the environment used here (ztp/swsscommonnot importable), so this change was verified by code inspection only.use_config_dbfalse) is unaffected.