-
Notifications
You must be signed in to change notification settings - Fork 46
dcnm_bootflash module #311
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
Merged
Merged
Conversation
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
bootflash_info.py - Initial work to retrieve bootflash info. dcnm_bootflash.py - initial work on skeleton - Query(): sort of working. playbooks/roles/dcnm_bootflash - base files added tests/integration/targets/dcnm_bootflash/* - base files added module_utils/common/api/v1/... /bootflash/bootflash.py - Initial endpoints done
The original requested name was dcnm_switch_bootflash, but I was using dcnm_bootflash. Have changed the name in all relevamt places.
After discussing with Shangxin, renaming the module to dcnm_bootflash. Have changed the name in all relevant places.
1. bootflash_info.py - build_matches(): strip leading space from ipAddr value in NDFC response. - Update and/or add docstrings throughout. - Add match property to return the current matching switch+file information. - Remove unused @Properties.add_params class decorator. 2. dcnm_bootflash.py - Query(): Rewrite to use self.have. 3. tests/integration/targets/dcnm_bootflash/tests/dcnm_bootflash_query.yaml - Rewrite assertions to avoid jinja2 warnings. - Update some of the REQUIREMENTS section.
Add infrastructure to easily determine whether files can be deleted on each switch and raise ValueError with appropriate error message if any files cannot be deleted. I suspect there will be more reasons than Migration mode so add capability to handle additional scenarios in the future.
This requires that files be manually created on switch-under-test bootflash. e.g. echo foo > foo.txt There's just no good, low-overhead, way to add files to a switch bootflash. dcnm_file_upload + dcnm_image_upgrade is (IMHO) way too slow for an integration test. Better just to get on the switch CLI and use echo...
1. Add partition property 2. Fix payload to use partition property (rather than file_path) for "partition" value.
BootflashFiles().__init__() - Remove self.response_dict and self.result_dict. - Alphabetize remaining attributes.
Also implement idempotence for files in root directory of flash devices. Idempotence cannot currently be supported for files in directories since the bootflash-info endpoint does not support listing files within directories.
1. EpBootflashDiscovery(): new endpoint class. 2. BootflashInfo(): To ensure we have current bootflash information, send EpBootflashDiscovery request before sending EpBootflashInfo request. 3. BootflashInfo(): Refactor stripping of ipAddr in build_matches() info strip_ipaddr_leading_space() 4. BootflashInfo(): change all occurances of bootflash_type to supervisor (new supervisor property returns associated key bootflash_type). 5. BootflashInfo(): update docstrings. 6. imagemgnt.py: Remove EpBootflashInfo() from this file. It already exists in imagemgnt/bootflash/bootflash.py
1. BootflashFiles(): Rename all occurances of bootflash_type to supervisor. 2. BootflashFiles(): Run through linters.
1. Update DOCUMENTATION to note that currently we support only files in a partition's root directory. 2. Update EXAMPLES section to remove subdirectories. 3. Update docstrings throughout. 4. Common(): fix error messages to reference the correct vars. 3.
Remove directories from all filepath values since we do not support deleting files in directories.
1. Rename EpBootFlashInfo() everywhere to EpBootflashInfo() 2. Rename test file to reflect that it contains tests for the bootflash endpoints.
1. Deleted().parse_target() move to Common() 2. Query(): leverage parse_target()
1. ParseTarget(): new class to parse a target dict. 2. dcnm_bootflash.py: Query() and Delete(): Leverage new ParseTargets() class. 3. BootflashInfo(): Only set results.action, results.check_mode, and results.state. Leave it to the caller to set results.current_*. 4. BootflashInfo().populate_property(): Refactor out build_match() 5. BootflashInfo(): Run through linters. 6. ParseTarget(): new class to parse a single target e.g. bootflash:/my.txt into its constituent bootflash-files API parameters. 7. dcnm_bootflash.py: leverage ParseTarget() 8. dcnm_bootflash.py: Remove Common().get_have() in favor of using BootflashInfo() in Query() and Deleted() 9. dcnm_bootflash_query.py - Update integration test expectations based on above changes.
BootflashInfo().populate_property() return None if property is not found in self.match. Previously we raised ValueError().
1. test_bootflash_common.py - 100% coverage of Common() in dcnm_bootflash.py - Remove unused commented imports.
1. dcnm_bootflash.py - Common().get_want(): Use copy.deepcopy() 2. test_bootflash_deleted.py - Deleted(): Initial unit tests
1. test_bootflash_deleted.py - test_bootflash_deleted_02000 - populate_files_to_delete() negative test
1. BootflashFiles().target should raise TypeError if value is not a dictionary. 2. test_bootflash_files.py - Update corresponding unit tests.
1. dcnm_bootflash.py - Deleted().update_bootflash_files(): add try/except block around - convert_target_to_params - bootflash_files property setters 2. test_bootflash_deleted.py - Add unit tests for the above
1. test_bootflash_deleted.py - Deleted(): 100% coverage
1. test_bootflash_query.py - 87% coverage for dcnm_bootlfash.py - test_bootflash_query_00000 - test_bootflash_query_00010 - test_bootflash_query_01000
1. dcnm_bootflash.py - Common()__init__() - Fix crash when users doesn't include global target. - Query().register_null_result() - new method. 2. Update unit tests to align with changes to Common() above. 3. test_bootflash_query.py - test_bootflash_query_01010 - Add test 4. Run everything through black/isort.
1. Move query-state tests into separate files to match deleted-state tests.
1. result.response was changed a couple commits ago. Modifying integration tests to align with those changes.
1. tests/integration/targets/dcnm_bootflash/defaults/main.yaml - Add default filenames here. User can override in dcnm_tests.yaml - Add default wildcard_filepath here. User can override in dcnm_tests.yaml 2. Update inventory to use switch1 and switch2 instead of spine1 and spine2 3. tests/integration/targets/dcnm_bootflash/tests/*.yaml - Change spine1 and spine2 to switch1 and switch2 everywhere - Update REQUIREMENTS section with current set of vars and mention that the default vars can be overridden by uncommenting in dcnm_tests.yaml 4. playbooks/roles/dcnm_bootflash/*.yaml - Update to reflect the above changes. - Remove all vars not used in the dcnm_bootflash integration tests.
There doesn't appear to be a way to add create_files as a testcase within dcnm_bootflash role. For now, create_files.yaml is a separate playbook that uses ansible.builtin.include_vars to read nxos_vars.yaml to determine what files to create. 1. playbooks/roles/dcnm_bootflash/create_files.yaml - modify to read vars from nxos_vars.yaml 2. playbooks/roles/dcnm_bootflash/nxos_vars.yaml - contains dictionary nxos_vars defining files to create.
@allenrobel I generated and added the docs for this module to the PR |
mikewiebe
approved these changes
Oct 30, 2024
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.
Summary
This PR provides the following:
dcnm_bootflash
modulePurePosixPath
for listing/deleting matching files so users do not need to explicitly specify each file.dcnm_bootflash
module.Branch
dcnm_bootflash
created off of branchdcnm_fabric_v2
Unit test coverage
99%
bootflash_files.py100%
bootflash_info.py100%
convert_file_info_to_target.py100%
convert_target_to_params.py89%
dcnm_bootflash.pyIntegration test coverage
Roles
Playbooks
There didn't seem to be a good way to use a Role within
dcnm_bootflash
to create files, since that role would use ndfc as the host rather than the switches themselves (create_files.yaml
usescisco.nxos.nxos_command
to create the test files directly on the switches, since NDFC doesn't offer a way to create files on a switch, other than to create an image_policy and upload large images, which is way too heavy-weight for this task).Instead, we are using a separate playbook to create the test files. Or the person running the integration tests can manually create the files on the switch using switch CLI e.g.:
switch# echo foo > bootflash:/air.ndfc_ut
.New files
deleted
andquery
states.file_info
dict (retrieved from NDFC) into atarget
dict (which fixes the leading space in ipAddr in NDFC'sfile_info
dict, and constructs afilepath
from e.g.partition
,fileName
, andfilePath
in theinfo_dict
which can be easily matched usingPurePosixPath()
).target
also includesip_address
andserial_number
to give users complete information about files matched and/or deleted.Example Playbooks
Query all text files on bootflash on the active supervisor of switches 192.168.1.1 and 192.168.1.2
deleted state