[ZTP] Improvements to allow in-band zero touch provisioning #39
Conversation
|
This pull request introduces 1 alert when merging c8a8eac into f7dd3c5 - view on LGTM.com new alerts:
|
|
@liorghub Can you please review? |
src/usr/lib/ztp/ztp-engine.py
Outdated
|
|
||
| except Exception as e: | ||
| logger.debug('Exception [%s] encountered while verifying plugin for configuration section %s.' % (str(e), sec)) | ||
| logger.info('Exception encountered while verifying plugin for configuration section %s. Marking it as FAILED.' % sec) |
There was a problem hiding this comment.
Should these be logged as errors?
There was a problem hiding this comment.
Agreed changed it to a logger.error.
| @@ -191,6 +191,7 @@ def test_ztp_dynamic_url_invalid_arg_type(self, tmpdir): | |||
| "source": "/tmp/test_firmware_%s.json" | |||
| } | |||
| }, | |||
| "pre-ztp-plugin-download": true, | |||
There was a problem hiding this comment.
Since default for pre-ztp-plugin-download is defined as true I believe this need not be updated on all the test cases.
There was a problem hiding this comment.
The way test cases are written, default setting also needs to be set.
| } | ||
| } | ||
| }""" | ||
| expected_result = """0001-test-plugin |
There was a problem hiding this comment.
Can we add another case with ignore_result=true for section 0002?
There was a problem hiding this comment.
Added a new test case test_ztp_json_invalid_plugins_ignore_result.
| # Obtain a copy of the list of configuration sections | ||
| section_names = list(self.objztpJson.section_names) | ||
| abort = False | ||
| logger.debug('Verifying and downloading plugins user by configuration sections: %s' % ', '.join(section_names)) |
src/usr/lib/ztp/ztp-engine.py
Outdated
| abort = True | ||
|
|
||
| except Exception as e: | ||
| logger.error('Exception [%s] encountered while verifying plugin for configuration section %s. Marking it as FAILED.' % (str(e), sec)) |
There was a problem hiding this comment.
Consider change from verifying to downloading.
src/usr/lib/ztp/dhcp/inband-ztp-ip
Outdated
| @@ -38,7 +38,7 @@ case $reason in | |||
| if inband_interface_check ${interface} ; then | |||
| if [ -n "$old_ip_address" ] && [ -n "$old_subnet_mask" ]; then | |||
| prefix=$(IPprefix_by_netmask "${old_subnet_mask}") | |||
| config interface ip remove ${interface} ${old_ip_address}${prefix} | |||
| /usr/local/bin/config interface ip remove ${interface} ${old_ip_address}${prefix} | |||
There was a problem hiding this comment.
Instead of changing each line, you can add /usr/local/bin to PATH by adding the following line at start of file.
export PATH=/usr/local/bin:$PATH
There was a problem hiding this comment.
The dhclient hooks are executed in the context of the dhclient process. I suggest we don't change the PATH variable unless it is really required.
rajendra-dendukuri
force-pushed
the
inband_ztp_improvements
branch
from
b981042 to
cbfc1ae
Compare
- Download all plugins before processing each configuration sections. This ensures that configuration section plugins are available at all times. If the plugin of a configuration section needs to be downloaded at a later time when ZTP is in-progress and the configuration section is actually processed, set the field "pre-ztp-plugin-download" : false in the corresponding configuration section data in ztp.json. - Additional unit test cases added
rajendra-dendukuri
force-pushed
the
inband_ztp_improvements
branch
from
cbfc1ae to
01d1777
Compare
Download all plugins before processing each configuration sections. This ensures that configuration section plugins are available at all times. If the plugin of a configuration section needs to be downloaded at a later time when ZTP is in-progress and the configuration section is actually processed, set the field "pre-ztp-plugin-download" : false in the corresponding configuration section data in ztp.json.
Additional unit test cases added