Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
dcnm_image_upgrade (ready to merge). (#269)
* More robust handling of response_data
* Let the user know which device is failing.
* More unit tests
* run thru black/isort
* Convert user input to boolean if it's boolean-like
* Add testcase, standardize testcase names, more...
Run thru black/isort
* standardize test names
* Fix docstring
* Rename "module" to something more descriptive
* Consider the upgrade status when determining idempotence
If have upgrade == "Failed" we should try to upgrade again, else the user would have to use the GUI (or policy management playbook) to reset the upgrade status.
* Determine valid EPLD module differently, more...
If options.epld.golden is True, then upgrade.nxos must be false or NX-OS throws an error which doesn't appear in any CLI output after the event. So, catch this combination, and fail_json with an explanation.
* Remove log messages
* More robust handling of missing keys
* Remove log messages
* method_name should be local to each method
If method_name is global to a class, the wrong method name is logged if a method calls another method.
* Remove fail_json from validate_devices.
* Update test_image_mgmt_upgrade_00005 based on last commit
* Add testcases for ImageUpgrade.commit(), more...
Add logging to image_upgrade.py to obtain controller responses for future tests. Remove these later...
* Add unit tests, more...
epld_module - Accept a module number in str() form and silently convert to int()
* Add unit tests for _merge_defaults_to_switch_config
* build_payload: Add another unit test, more...
ImageUpgrade.nxos_mode: sort valid_nxos_mode so that the match in corresponding unit-test works consistently.
* Remove check for install_options.success, more...
Removing since install_options will fail_json in the event of failure.
* ImageUpgrade.commit - add more unit tests
* run through black/isort
* build_payload: convert boolean-like strings to boolean
Give the user some leeway in setting boolean-like items e.g. instead of True, let the user enter "true" or "TRUE"
* Fix bad method name make_bool -> make_boolean
* Remove duplicate testcase 00004
This testcase didn't used to be a duplicate, but we changed the code and then changed this testcase without realizing that it's now a duplicate of 00005.
Renumber all subsequent testcases to make room for one additional testcase (next commit)
* ImageUpgrade: verify upgrade.nxos/upgrade.epld, more...
test_image_upgrade_ImageUpgrade.py
- Add summary for all tests
- Add tests for:
- upgrade.nxos invalid value (00018)
- upgrade.epld invalid value (00033)
- Fix 00015 (options.reboot.config_reload) test description
* upgrade.epld - remove redundant assignment/check
* Add unit tests, more...
- ImageUpgrade._wait_for_current_actions_to_complete: Add unit tests 00080 and 00081
- ImageUpgrade._wait_for_current_actions_to_complete: self.ipv4_todo should be a set()
- ImageUpgrade._wait_for_current_actions_to_complete: ipv4_todo/ipv4_done should be sorted for unit tests
- Run class and test .py through black/isort
- ImageUpgrade.check_interval - add setter for unit test
- ImageUpgrade.check_timeout - add setter for unit test
* Add unit test for _wait_for_image_upgrade_to_complete
Add unit test test_image_mgmt_upgrade_00090
* Add unit test for _wait_for_image_upgrade_to_complete
Add unit test test_image_mgmt_upgrade_00091
* Sort set difference for unit tests, more...
Also remove unused var status. This was used in log_msg() previously but these log_msg() have since been removed.
* _get() run return value thru make_boolean()
* test_image_upgrade_ImagePolicyAction: Standardize test names, more...
Rename mock_ image_policy_action to image_policy_action
Rename mock_issu_details to issu_details
Rename mock_image_policies to image_policies
Rename match, if if is defined globally, to match_<testcase_ID>
Add more consistent docstrings
* image_policy_action: rename build_attach_payload to build_payload
* Run thru black/isort
* Don't hardcode method names
* Provide user's value in fail_json message
* Add unit tests, run thru black/isort, more...
Add unit tests for setters:
- action
- serial_numbers
* Standardize test names
* Run thru black/isort
* Standardize test names, more...
image_upgrade_responsees_SwitchDetails.json - Remove key/values which are not used in tests.
* Run thru black/isort, doctstring cleanup
* Add property vpc_role2, more...
Two properties exist for vpc_role in the controller response.
- vpc_role2 corresponds to vpc_role.
- vpc_role corresponds to vpcRole
method_name made local in both __init__ and _init_properties
* Standardize test names
* Standardize test names, run thru black/isort
* Standardize docstrings
* Run thru black/isort
* Add TEST_NOTES and remove unused entries
* Make global var name unique
* Add copyright and attribution
* Remove unused import
* Standardize test names, remove unused fixtures, more...
image_upgrade_common.py
- Changed self.debug to False
- Changed logfile name to match collection name
* Add TEST_NOTES to _handle_delete_post_put_response fixtures
* Remove unused file
* Detach policy only if have policy matches want policy
* Run thru pylint
* Run thru black/isort
* Run thru black/isort/pylint
* Run thru black, isort, pylint, make method_name local to functions
* Update docstrings
* Removed unused fixtures file
* Remove unused payloads file
* Remove unused configs file
* Add a TODO for a future test
* Update docstrings
* Update docstrings, rename error_message to match
* Update docstrings
* Run thru black, isort, pylint
* Add docstrings
* Add docstrings
* Modify boolean asserts to satisfy pylint
* Appease linters and ansible sanity (part 1)
* Fix imports
* Fixes, hardening, reduce unit test code duplication, more...
- Consolidate fixtures and file loaders into image_upgrade_utils.py
- Harden fixture.py
- Fix EPLD idempotence issue
- Fix merge of switch and global configs
- Add initial unit tests for ImageUpgradeTask (to verify above fix for merging configs)
- Modifications to appease pylint
- More work toward passing Ansible sanity (more TODO though)
- Temporarily add debugging statements throughout
* Use load_playbook_config for all inputs, more...
- More cleanup for pylint
- image_upgrade.py - Add comment about key misspelling
- Update TEST_NOTES for some inputs
- Run thru black, isort, pylint
* merge_dicts() handle case where no suboptions are specified in options dict
There's a case where the user specifies an options dict (e.g. "upgrade"), but does not include the suboptions. This results in the options dict given a value of null when converting from YAML to a Python dict. Rewrite merge_dicts() to handle this case.
Also, moved merge_dicts to image_upgrade_common.py so it can be shared in the future.
Lastly, so that all switch config processing is performed in one place, moved _merge_defaults_to_switch_config() from image_upgrade.py (ImageUpgrade class) to dcnm_image_upgrade.py (ImageUpgradeTask class). These changes required modifying/moving several unit tests.
* ansible-sanity related cleanup
* Ignore missing-gplv3-license for dcnm_image_upgrade.py
* Move all parameter validation to ParamsValidator class
The goal here is to separate parameter validation from things like applying defaults and to isolate it so that it's separately testable.
A few testcases need to be moved/rewritten as a result of this commit. These are now commented out:
test_image_mgmt_upgrade_task_00003
test_image_mgmt_upgrade_task_00004
test_image_mgmt_upgrade_task_00005
* Decouple _merge_defaults_to_switch_configs from _merge_global_and_switch_configs
Make _merge_global_and_switch_configs self-contained
ParamsValidator: Add ability for a param to match more than one type
These changes required changes to the unit tests associated with _merge_defaults_to_switch_configs
* Add integer range validation
* Use copy.copy() when assigning need to self.need
* sentence should be on a single line
Addressing mikeweibe review comment
* Expanded on meaning of validate
Addressing mikewiebe review comment.
* Fix copywrite
Addressing mikewiebe review comment
* Align epld description with current playbook structure
Addressing mikewiebe review comments.
* Fix nxos options in two places in example playbook
Addressing mikewiebe review comments.
* Leverage ansible.module_utils.common validation
* Decouple ParamsValidate from ImageUpgradeCommon
For now, copy log_msg() into ParamsValidate, along with self.debug and self.file_handle.
Eventually, we should create a common logging infra that contains log_msg(), a var to enable/disable logging, and a common destination log file.
* Run thru black/isort
* isolate self.need from need using copy.copy()
* rename test file (remove extraneous "image_upgrade")
* Initial set of unit tests for ParamsValidate, more...
Also, hardening of ParamsValidate class:
ParamsValidate.params_spec - Add basic verifications
ParamsValidate.parameters - Add basic verifications
ParamsValidate.verify_type() - fix unbound local variable error
ParamsValidate.verify_multitype() - no need for var 'error', removed it
* add assert for mandatory_param_spec_keys
* Update or add copyright header, more...
Also:
dcnm_image_upgrade/fixture.py
- change "f" to "file_handle" to appease pylint.
All unit tests:
- Run thru black / isort / pylint linters
* Align fail_json messages to suggest similar debugging steps.
* Add unit tests for multitype and integer range, more...
Run through black/isort/pylint and address some pylint issues.
* Fix assert expectation
After modifying ParamsValidate to leverage validate's conversions, the assert needed to check for int 1 rather than str "1"
* Add required 'preferred_type' param for multi-type, more...
1. params_validate.py - If 'type' param is a list of types (i.e. multi-type), require that the caller has set 'preferred_type' to indicate what type they prefer param to be. We then do a best-effort attempt to conver param to that type. If this conversion fails, we call _verify_multitype() so that param will be converted to whatever type succeeds first. If no types succeed, then we fail_json.
2. params_validate.py - Added function ipaddress_guard() to guard against the ipaddress module doing undesired conversions of int and bool types.
3. dcnm_image_upgrade.py - added 'preferred_type' to params_spec, per addtions in item 1 above.
4. modified some unit-tests and fixtures, per additions in item 1 above.
* Harden ParamsValidate and add more unit tests, more...
This should be the last modification to ParamsValidate. It turns out that, while Github Co-Pilot generated a "correct" version of self._verify_multitype(), it was anything but elegant. This has been rewritten to (hopefully) be more understandable and maintainable. Same goes for self._verify_type()
* Run thru black/isort.
* ParamsValidate hardening, add type check, more...
ParamsValidate:
- verify that range_min and range_max are integers before using them.
- update example usage to include range_min and range_max.
- privatize a few formerly-public methods.
test_image_upgrade_params_validate.py
- Add negative test to verify non-integer handling for range_min and range_max.
- Modify match for a few tests to match private method names
- Modify a couple mark.parametrize structures to include range_min and range_max
* Add properties to enable/disable debugging and set the logfile name, more...
ParamsValidate:
- Add ability to enable/disable logging on the fly
- Add more reserved parameters to self.reserved_params (length_max and no_log)
- Refactor __init__()
test_image_upgrade_params_validate.py
- Modify asserts in test_params_validate_00001 to reflect the above changes
* Run thru black and isort
* Appease ansible sanity tests, more...
Fix pep8 and missing __metaclass__ Ansible sanity errors.
I guess there's a disagreement between Ansible sanity and pylint regarding __metaclass__ = type. To appease Ansible sanity, I'll ignore local pylint errors regarding this and modify my pylint config to ignore the error.
Also, remove all module docstrings since these seem not to have a good place to go without causing Ansible sanity errors (per pylint, a module docstring is supposed to go at the very top of the file, but this causes Ansible sanity to complain about boilerplate).
* ImageValidate: add unit tests for instance.commit()
* ImageValidate: additional unit tests, more...
ImageValidate
- isinstance(True, int) ends up being True, so we needed to add a check for isinstance(value, bool) to a couple properties.
- Added unit tests for commit() and for several properties
* ImageUpgradeTask: leverage ParamsMergeDefault class
1. A few boilerplate lines were updated.
2. ImageUpgradeTask
- Previously, we were creating a defaults dictionary which we used to update the merged config. This was not optimal since we were, in essence, duplicating information about default values for parameters that is already in a properly-written params_spec.
Wrote ParamsMergeDefaults class which takes a params_spec, and a playbook config and updates the playbook config to add any missing parameters that have a default value.
This required re-writing some unit tests. And, as a result of using this, a errors were found in the params_spec for image_upgrade (the following were not specified: reboot, upgrade.epld, upgrade.nxos).
* Copy dcnm_inventory action plugin and modify for dcnm_image_upgrade
* Appease pycodestyle pep8 issues in sanity run
* Fix pylint disallowed-name error in sanity tests
* Fixes for Undefined variable '__class__' pylint errors
* Remove pylint disable=consider-using-with which doesn't seem compatible with earlier pylint
* Add docs for dcnm_image_uupgrade
* Fix typo in validate() fail_json message, add related unit tests
* Use a common class for log messages
* Move merge_dicts() to a common class MergeDicts, more...
Move merge_dicts() out of ImageUpgradeCommon() and into its own class that can be shared with other modules.
ImageUpgradeTask now leverages MergeDicts()
Also:
- ImageUpgradeTask(): remove unused self.mandatory_global_keys
- ImageUpgradeCommon(): remove self.method_name from a couple functions (these should have been non-global assignments and were obscuring the method names of functions that called these).
- ImageValidate: Update unit tests to expect the proper method names (per above change)
- test_image_mgmt_upgrade_task_00001: remove verifications for items no longer present in __init__.
- Cleanup a few comments here and there.
* Move unit tests for common classes out of image_upgrade
ControllerVersion and Log were initially written as specific to image_upgrade, and their unit tests lived in:
tests/unit/modules/dcnm/dcnm_image_upgrade
These classes have moved to plugins/module_utils/common and so we've moved their unit test scripts, fixtures, and other supporting utilities to:
tests/unit/module_utils/common
* Don't abbreviate import
* MergeDicts: add unit tests, simplify logic
* Additional unit test
* ImageValidaate: move refresh() out of for loop
ImageValidate.prune_serial_numbers: issu_detail.refresh() should be called only once outside the for loop.
* ParamsMergeDefaults: use dict.get() rather than "not in"
For reasons I don't understand, using:
"if <key> not in <dict>"
was failing idempotence for keys with null values that have a default value of dict i.e. key["default"] = {}. After changing this to use:
"if dict.get(<key>, None) is None"
things are working for this specific case.
* ImageInstallOptions: Avoid NDFC error
NDFC returns an error when all of issu, epld, and package_install are False.
Avoid this by returning a mocked NDFC response indicating that nothing needs to be done. This makes the change transparant to anything outside this class.
While we're modifying this class, fix a couple pylint errors and inappropriate assignments of self.method_name.
* Add unit test to cover the changes in the last commit
* rename ParamsValidate.validate() to ParamsValidate.commit()
For consistency with other classes...
* Changing behavior of ImagePolicies, more...
ImagePolicies() called fail_json() when the caller asked for a policy_name that did not exist on the controller.
During initial work on dcnm_image_policy module, we need this to return None instead. Hence, changing the behavior.
This does not require any other changes to dcnm_image_upgrade, other than modifying one of the unit tests to expect the new behavior.
* Add a failed_result to all fail_json calls, more...
Add failed_result property to ImageUpgradeCommon. Modify all fail_json() calls to add this result. This is the first phase of adding proper results to the dcnm_image_upgrade module. Phase 2 will add a Result() class and the needed infra to return proper success results.
image_policies.py in class ImagePolicies, add property all_policies which return the current image policies on the controller. This was needed for dcnm_image_policy module.
* Fix mocked fail_json() to add **kwargs required by last commit.
* Fix ansible-sanity complaints
* Fix ansible-sanity errors
* SwitchIssuDetails: change filtering key name, more...
In preparation for refactoring, we are changing the key name used to set which device's information we retrieve.
Previously, each of the subclasses used a different key name:
SwitchIssuDetailsByIpAddress used ip_address
SwitchIssuDetailsBySerial_number used serial_number
SwitchIssuDetailsByDeviceName used device_name
We have changed the above such that all of them now use "retrieval_key" to set the filter. This will allow us to refactor now only this class, but also we'll be able to refactor _wait_for_current_actions_to_complete() from ImageStage(), ImageUpgrade() and ImageValidate() into a single function in ImageUpgradeCommon()
* Change "retrieval_key" to "filter", more...
Since these classes already have a "filtered_data" property which is used to return the data filtered by "retrieval_key", it's more intuitive to change the name of "retrieval_key" to "filter".
* New logging system. New result system, more...
1. Added a common logging facility leveraging Ansible's logging.config.dictConfig so that logging can be changed based on the contents of a YAML file. Default (log.config = None) is not to log (i.e. the root logger is disabled). An example log.yaml config file is provided in module_utils/common/log.yaml.
2. Added a Result() class (using this also for the dcnm_image_policy module) and made the necessary modifications to leverage this.
3. Various other cleanup.
* Forgot to add module_utils/common/result.py
* Switching to JSON for the logging config, more...
Opps. YAML is not natively supported by Python. While it would be nicer to be able to use a YAML file for configuring the logging system, we'd have to update requirements.txt to include PyYAML, which wouldn't be good.
Using JSON instead since it's build-in.
* Appease the linters
* Fix pylint error for a couple earlier python versions
Looks like bare __class__.__name__ doesn't work for some python versions.
In all classes, changed:
self.class_name = __class__.__name__
To:
self.class_name = type(self).__name__
* Fix a few things found with local linter, more...
Also:
1. After some testing, it appears that the following are equivalent
self.class_name = type(self).__name__
self.class_name = self.__class__.__name__
The second one seems more readable to me, so I changed things back.
The issue I was trying to fix was that, when subclasses call super() on their parent class(es), the parent logs the subclass's class name. After a lot of testing, I think this is just the way things work. To make the logs more clear, I added an explicit (hardcoded) class name in the "ENTERED" debug log each class's __init__() method. This way, it's much clearer when a subclass calls super() on it's parent class.
2. Added encoding="utf-8" to the open() call in log.py to satisfy (local) pylint.
3. Added better docstring to the Log().commit() method.
4. Added logging to some classes that I'd missed (mostly in module_utils/common/).
* Remove unit test for MergeDicts.debug @Property
* Modify unit tests for the new logger implementation
1. Remove log tests from test_params_validate.py
2. Rewrite test in test_log.py
* Fix pylint f-string errors
* Enhance playbook results
1. Enhance result for "deleted" state. Per-switch result for "deleted" state now includes action, ip_address, logical_name, policy, and serial_number.
2. Add response list to Result class and update "deleted" state responses to include per-policy response.
3. Update "query" state results to include controller responses.
TODO: Need to update merged state results...
* Fix pylint error
* Result handling for merged state
* Fix unit tests to reflect changes in the last commit
* Remove one call to issu_details.refresh() in _build_payload(), more...
Also:
1. Add debug logs in for all refresh() calls (for all instances that touch the controller) in image_upgrade.py
2. Fix typo in docstring for filtered_data property
3. Update key for unit test: test_image_mgmt_upgrade_00004
4. Comment out log.config assignment in dcnm_image_upgrade.py to disable logging
* Remove a TODO that was harder than I thought
1. Getting rid of one extra call to issu_detail.refresh() isn't worth the effort given that we call issu_detail.refresh() many times within the for loops (by nesessity) in both _wait_for_current_actions_to_complete() and _wait_for_image_upgrade_to_complete().
2. Rearranged lines in test_image_mgmt_upgrade_00004 (cosmetic change).
* Modified returned result not to use ansible state names
In preparation for writing integration tests, I realized that the returned results weren't very good.
There wasn't a clean way to categorize the result sections by Ansible state (at least I couldn't think of one).
1. Deleted state detaches image policies from switches
2. Merged state attaches policies, stages and validates images, and upgrades switches.
3. Query state simply returns the current ISSU state of the switch(es).
Given that the above don't really fix neatly into similar per-state actions, I removed Ansible state names from result section names and, instead, used names that describe what is being returned.
So, we have the following subsections in both the "diff" and "response" sections:
attach_policy
detach_policy
issu_status
stage
upgrade
validate
* Fix linter shebang error
* Fix trailing whitespace linter error
* Improve ImageStage results and logging, more...
1. Sort todo and done sets when logging them so that serial_numbers/ip_addresses line up within the log.
2. Improve ImageStage diff to include the same info as other diffs (ip, serial, logical name, action).
3. TODO: need the same diff mods for ImageValidate...next commit...
4. Remove failed_result propery from ImageUpgradeCommon since we're using ImageUpgradeTaskResult() for that now.
* Consistent diff between ImageStage and ImageValidate
1. Align the diff returned by ImageStage and ImageValidate
2. ImageStage, improve the response when no serial numbers need to be processed.
3. ImageValidate, return a response when no serial numbers need to be processed (similar code to 2, above).
4. ImageValidate, update unit test to reflect different response from ImageValidate in the case where no serial_numbers need to be processed.
* Refactor, more...
1. ImageUpgradeTask: refactor _build_policy_attach_payload(), _self_policy_attach_payload(), and handle_deleted_state() into a single method _attach_or_detach_policy()
2. ImageUpgradeTask: Fix _failure() and remove docstring contents that described the issue.
3. Modify test_image_mgmt_upgrade_task_00001 to remove assert for instance.payloads as this is no longer needed
4. test_image_upgrade_validate.py: run through black and isort
5. ImagePolicyAction() : modify to store list of diffs and leverage this in ImageUpgradeTask() when generating results.
6. ImagePolicyAction().validate_request() - add check for image policy does not exist on the controller.
7. ImagePolicyAction() - Add typing for all dict assignments.
* Add type defininitions for dicts, more...
1. ImagePolicyAction: Add type definitions for dicts
2. ImageStage, ImageValidate, ImageUpgrade: Add clarifying comments
* Fix linter pycodestyle linter error
* Initial integration test for "merged" state
* Appease YAML linter
* IT: Fix jinja2 warning, add "deleted" state test, more...
1. Fixed the following with the merged.yaml role:
[WARNING]: conditional statements should not include jinja2 templating delimiters ...
2. Adding deleted.yaml role
3. Fix ImageUpgradeTask detach_policy result (found with integration test for "deleted" state)
* skip query state diffs when determining if anything changed
* Add "query" state integration test
* Add loop around dcnm_send to wait for successful response.
We've seen cases were a 500 response is returned if install-options is called too early. Add a loop to try for self.timeout seconds with 5 second poll time. Update unit tests to bypass the sleep so that the tests run faster.
* Remove test_image_mgmt_validate_00020
We''re now initializing self.serial_numbers to [] instead of None and silently returning if self.serial_numbers is [] in commit(). Previously, we would fail_json if self.serial_numbers was still set to None in commit(). We no longer need the unit tests that verifies fail_json is called.
* ImageUpgradeTaskResult: Remove redundant class definition.
* Break "merged" state integration tests into two files, more...
Rename ansible_switchX to ansible_switch_X
Update comments and task names to include test stage
* ImageUpgradeTask: Fix idempotence for merged state
Fix an intermittent idempotence issue found by the integration tests.
have.status was sometimes reported as "Not-In-Sync" which caused upgrade.nxos to be set to True, which cause unnecessary upgrade. The fix was to remove have.status from consideration. We now consider only have.reason, have.policy, and have.upgrade.
* Move timeout and unit_test properties to ImageUpgradeCommon
As a first step, in preparation for moving safe_send() to ImageUpgradeCommon, move two properties that safe_send() relies on.
* Add safe_send() method to call dcnm_send with a retry and timeout mechanism
We were experiencing timeouts and intermittent 500 responses when using dcnm_send() directly. Hence, adding a safe_send() method to retry dcnm_send() until we get a 200 response or until a timeout is exceeded.
Will move this to ImageUpgradeCommon later.
* Add recent runtimes to integration tests
* Forgot to commit changes to unit test.
* Fix PEP8 issue with inline comment
* Cleanup result names
Be explicit with result names.
1. ImageUpgradeTask: rename self.result to self.task_result and update related unit test.
2. ImageUpgradeCommon.failed_result: No need to instantiate ImageUpgradeTaskResult since we're only using one of its properties.
3. Move module_utils/common/result.py to ImagePolicy module. It wasn't really a good idea to try to develop a generic result class given the differences in results between the different modules. common/result.py is currently specific to ImagePolicy
* Move ImagePolicyAction.safe_send to ImageUpgradeCommon.dcnm_send_with_retry
Moving to dcnm_send_with_retry to ImageUpgradeCommon so that other classes can leverage it.
* ImagePolicies() - initialize response data to {}, more...
Also, update some comments throughout.
* Use dcnm_send_with_retry in more classes
Use ImageUpgradeCommon().dcnm_send_with_retry() in two more classes.
1. ImageUpgrade()
2. ImagePolicyAction()
- Update ImageUpgradeTask() to use ImagePolicyAction().response_current
- Update unit tests for ImageUpgrade() and ImagePolicyAction
- ImageUpgrade: remove result and response properties and inherit from ImageUpgradeCommon.
- ImageUpgrade: move devices verification from commit() to _validate_devices()
- ImageUpgradeTaskResult: minor docstring edit to fix typo
- ImageUpgradeCommon: add response_current and result_current properties.
- ImagePolicyAction: Remove local response and result property definitions and inherit from ImageUpgradeCommmon
- ImagePolicyAction: Use response_current and result_current as appropriate
- ImagePolicyAction: Add a setter for query_result property
* Fix PEP8 error (need two black lines)
* Remove unused dcnm_send import
* Add class RestSend(), more...
Replacing ImageUpgradeCommon.dcnm_send_with_retry() with RestSend() class. The problem with the former has to do with pytest patching. For some tests, we need to patch dcnm_send() to mock different responses for each of the subclasses of ImageUpgradeCommon. Since the patch path was identical, there was no way to do this.
The solution is to move this to a separate class (RestSend) which is then imported into subclasses of ImageUpgradeCommon. This provides the different patch paths that we need to provide appropriate mock responses to each subclass.
Other changes:
1. The above required extensive changes for unit tests of ImageUpgrade (the first subclass we've modified to use RestSend). Other subclasses will be similarly modified later.
2. Run test_image_upgrade_image_policy_action.py through black/isort
3. InstallOptions: Move validation of refresh parameters out of refresh() and into a separate method. Modify unit tests appropriately.
4. InstallOptions: Fix use of self.result, self.result_current, self.response, self.response_current
5. InstallOptions: Remove timeout and unit_test properties (these are moved to RestSend). Remove response and result properties (these are inherited from ImageUpgradeCommon). Fix raw_response property to alias response_current.
6. ImageUprade: Use RestSend.
7. ImageUpgradeCommon: Added a lot of debugging to dcnm_send_with_retry()
* Fix PEP8 whitespace error
* Fix PEP8 import handling error
* Fix PEP8 linespace error, move MockRestSend to image_mgmt for now
* Move MockRestSend into the image management unit test directory
* Fix f-string error
* Remove need for MockRestSend
Figured out a working patch path.
* ImageUpgrade: instantiate RestSend in __init__, more...
ImageUpgrade.commit(): populate self.result, self.result_current, self.response, self.response_current, self.response_data with data received from RestSend.
* ImageStage: Leverage RestSend(), more...
- ImageStage: remove response, response_data, result properties (inherit from ImageUpgradeCommon)
- ImageStage: modify debug statements to use json.dumps for prettier output
- ImageStage: Use RestSend() class
- ImageUpgradeCommon: Add response_data getter/setter properties
- ImageUpgradeTask: harden handling of ImageStage.response
- test_image_upgrade_image_stage.py: modify unit tests to reflect the above changes
* Add unit test for ImageStage for len(serial_num) == 0
* Fix PEP8 errors in sanity tests
* ImageStage: Add unit test for 500 response
* ImageStage: add unit tests, more...
Add unit tests for the following properties:
- check_interval
- check_timeout
- serial_numbers
* ImageValidate: leverage RestSend, more...
Also:
ImageValidate: hardening for check_interval and check_timeout setters.
ImageValidate: Remove response_data, result, and response propoerties (these are now inherited from ImageUpgradeCommon)
test_image_upgrade_image_validate.py: modifications for changes made above.
* SwitchDetails: Leverage RestSend
Also:
RestSend: Add getter/setter property for payload
RestSend: Remove a couple debug statements
ImageStage: improve fail_json messages for check_interval and check_timeout properties
Update the following unit test files to reflect the above changes:
- test_image_upgrade_switch_details.py
- test_image_upgrade_image_policy_action.py
Minor update to docstring for test_image_mgmt_stasge_00010 in:
- test_image_upgrade_image_stage.py
* response and result properties should be inherited
- SwitchIssuDetails: inherit response* and result* properties from ImageUpgradeCommon
- ImageUpgrade: inherit response_data from ImageUpgradeCommon
- ImageUpgrade: reorder some items in __init__()
- ImageUpgrade: add some debug statements for result, response, response_data, etc
- ImageUpgrade: Modify fail_json conditional to read self.result_current rather than rest_send.result_current
- ImageUpgradeTask: pop "DATA" only if present in response_current
- ImageStage: move debug message
- ImagePolicies: fix typo in fail_json message
- Update unit tests to reflect above changes
- Run everthing through black/isort
* Fix ansible-sanity PEP8 issues
* Increase example logging_config.json maxBytes from 500K to 50M
* Update comments to include correct testcase name
* Fix reference to non-existing property
* Disable logging
* SwitchIssuDetails: rename refresh() to refresh_super(), more...
SwitchIssuDetails: rename refresh() to refresh_supper() so that subclasses can call it from within their local refresh() methods (previously, they called super().refresh()).
ImageValidate.commit(): don't upgrade property values directly, use inherited properties instead.
Update unit tests to reflect the above changes.
* ImagePolicies: do not fail_json if length of DATA.lastOperDataObject == 0
While writing unit tests for dcnm_image_policy module, found that we cannot fail_json if no policies are present on the controller. Modified ImagePolicies() class accordingly, as well as unit test for this case.
* Don't call refresh() more than needed.
- ImageStage.prune_serial_numbers: Call refresh() outside of for loop.
- ImageStage.validate_serial_numbers: Call refresh() outside of for loop.
The above could be made even more efficient by calling refresh() in commit(). This would require rethinking a couple unit tests, so will leave this for a future commit.
Also:
- test_image_upgrade_image_stage: Add a Summary section which provides the "gist" of a test case.
* Fix pep8 too many blank lines
* rename image_mgmt directory to image_upgrade
Background:
Initially, image_mgmt was going to hold the module_utils for image_policy, image_upgrade, and image_upload. I never coordinated with Mallik on this, and later decided it was a bit of organizational overkill.
This commit comprises the following:
1. Renaming directory module_utils/image_mgmt to module_utils/image_upgrade.
2. Modifying all python imports that point to module_utils/image_mgmt, to point to module_utils/image_upgrade.
3. Modifying all unit test names to replace image_mgmt with image_upgrade
4. Changing key names for all the unit tests to replace image_mgmt with image_upgrade.
* ImageStage: Add unit test for successful commit, more...
This commit brings ImageStage to 98% coverage.
Added unit test "test_image_stage_00075"
Also:
- Run unit test scripts thru black/isort/pylint
- Renumber ImageStage.commit() unit tests to be in range 00070 - 00079
- SwitchIssuDetails: Add debug message to .refresh() in all subclasses.
- ImageStage.commit: modify debug message
* Remove unused mocks
* Remove unused mocks
* Remove unused mocks, more...
Fix a couple pylint errors that were not showing up in sanity tests (redefined-builtin and invalid-name).
* Add unit test for bad result due to 500 response
* Fix pylint invalid-name errors
* Remove unused fixture, more...
controller_version_fixture was moved to tests/unit/module_utils/common many commits ago, and I forgot to remove the original.
* Fix test_log.py import of MockAnsibleModule
* Expand unit test execution to all files under tests/unit/.
.github/workflows/main.yml is currently pointing to tests/unit/modules/dcnm/. Modified this to point to tests/unit/. instead so that unit tests in tests/unit/module_utils/common will be executed.
* ImageStage.check_interval unit test: parameter for bool case
This brings us to 99% coverage for image_stage.py
* ImageStage.check_timeout unit test: parameter for bool case
* Fix comment typos
* ImageUpgradeCommon: Add unit tests, more...
ImageUpgradeCommon: Add unit tests for dcnm_send_with_retry.
To facilitate the above, we've modified ImageUpgradeCommon.__init__() to add self.dcnm_send. This required also modifying the patch for ImageUpgradeCommon.dcnm_send in unit test test_image_upgrade_image_policy_action_00021 .
Also:
- Added Summary to a couple test cases and other docstring improvements.
- Updated test_image_upgrade_image_upgrade_common_00001 with more asserts
- Removed old commented logging unit test code that is no longer relevant.
* Update version_added to "3.5.0"
* image_upgrade.py: 100% unit test coverage
test_image_upgrade_image_upgrade.py: Rewrote some unit tests to simplify logic and provide greater coverage.
test_image_upgrade_image_upgrade.py: Renumbered many unit tests.
image_upgrade.py: Move issu_detail.refresh() out of tight for loops in:
- ImageUpgrade()._wait_for_image_upgrade_to_complete
- ImageUpgrade()_wait_for_current_actions_to_complete
- ImageUpgrade().check_interval: Add check for bool due to isinstance() treating bool as int.
- ImageUpgrade().check_timeout: Add check for bool due to isinstance() treating bool as int.
Ran all module_utils/image_upgrade files through black/isort
* Move issu_detail.refresh() out of inner for loops
* Remove caplog from unit tests, update docstrings.
Update unit test docstrings to include class name.
Removing caplog from all unit tests where this was present (two tests).
* ImageUpgradeCommon: add unit tests
Brings ImageUpgradeCommon unit test coverage to 99%
- Updated setters which expect an int() to test first for bool().
- Added unit tests for:
- changed
- diff
- failed
- response_current
- response
- result_current
- result
- send_interval
- timeout
- unit_test
* ImagePolicies: Add unit tests, update docstrings, remove inherited properties
Removed the following properties which are inherited from ImageUpgradeCommon:
- response_data
- response
- result
Add unit tests for:
- ImagePolicies._get (two tests)
- ImagePolicies.all_policies (two tests)
Improve docstrings for existing tests.
* Replace (pylint) disallowed name "foo"
* Move rest_send.py to module_utils/common
RestSend will be used by both dcnm_image_upgrade and dcnm_image_policy in the future, so moving it into module_utils/common for sharing between these modules.
* Add path and verb properties, fix whitespace issue.
Also fix some debug lines that were too long.
* Use do_not_raise() to verify that fail_json isn't called.
Also:
test_image_upgrade_switch_details.py:
- Improve test docstrings
switch_details.py (SwitchDetails class):
- initialize self.path and self.verb in __init__() and use these in refresh()
* ImageUpgradeTaskResult: 100% unit test coverage
Also:
test_image_upgrade_switch_details.py: Run thru black
* Fix PEP8 sanity issue
* ImagePolicyAction: 100% unit test coverage
Also:
ImagePolicyAction._attach_policy: fix KeyError in fail_json message.
ImagePolicyAction.query_result: remove checking for value == dict since we don't know 100% what the controller will return.
ImagePolicyAction.serial_numbers: enforce list must be at least one element.
* ImageValidate: remove unused unit test
We moved initialization of self.path and self.verb to __init__(), which is covered by test_image_upgrade_validate_00001
Hence, we no longer need test_image_upgrade_validate_00021
* SwitchDetails: 100% unit test coverage
Also:
SwitchDetails.info: self.propertiies should be self.properties
test_image_upgrade_switch_details.py: Fix a few docstring typos
* SwitchIssuDetailsBy*: 100% unit test coverage
* Shaved a couple seconds off unit test runtime
* Fix PEP8 whitespace errors
* Fix PEP8 whitespace error
* ImageStage: Instantiate path and verb in __init__()
test_image_upgrade_stage_00001: add path,verb asserts
test_image_upgrade_stage_00071: remove this test case now that it's covered in 00001 above
* ImageValidate: 98% unit test coverage
ImageValidate.commit: Add unit test to verify diff
ImageValidate: Improve unit test docstrings
image_upgrade_responses_imageStage.json: Fix unit test 00075 key
ImageStage.commit: remove self.path and self.verb initializations (these are initialized in __init__()
ImageStage.commit: add return value type hint
* ImageInstallOptions: 99% unit test coverage
InstallOptions.policy_name: minor edit to fail_json message.
ImageInstallOptions.policy_name: Add unit test
test_image_upgrade_image_install_options.py:
- rename unit test test_image_upgrade_install_options_00003 to test_image_upgrade_install_options_00070
Run all other test_image_*.py files through black/isort
* Disable logging
* Update integration tests with new include syntax
'include' is deprecated in Ansible roles and is now 'include_tasks'
* Initial check_mode changes (needs more work)
* Fix PEP8 too many blank lines
* Add support for check_mode
Also:
dcnm_image_upgrade.py: Add a boolean, enable_logging, so that users don't have to comment/uncomment lines to disable/enable logging.
Rename image_upgrade_utils.py to utils.py to align with the convention in dcnm_image_policy
Modify the match regex in several unit test cases to align with the check_mode changes.
* Fix pylint f-string interpolation errors.
* Pull in changes from dcnm_image_policy
Since dcnm_image_upgrade is the single source of truth (prior to commiting these new modules) for everything in module_utils/common, we need to update a couple files that were enhanced during development of dcnm_image_policy.
This commit removes the dependency that rest_send.py had on module_utils/image_upgrade/image_upgrade_task_result, since this file will go away in a future version of this repo. rest_send.py now imports failed_result from module_utils/common/results.py
Also:
module_utils/params_validate.py : fix a sanity test error with type hinting
* Apply changes to results.py from dcnm_image_policy
This adds an "ok_result" property to results.py Results() class.
* Align result output with existing DCNM collection modules
This aligns the result output with existing modules. However, it doesn't leverage the Result() class.
For expediency's sake, I'll save those changes to a later version of this module since it'll be a larger effort that would require changes to many unit tests.
* Fix yamllint "too many blank lines"- Loading branch information
1 parent
0b0dc30
commit 43459d1