Skip to content

Conversation

benank
Copy link
Contributor

@benank benank commented Apr 11, 2022

If the package manager returns code 102 (REBOOT_REQUIRED) or 103 (ZYPPER_UPDATED, will need to repeat last patch installation) from a dry run command, it will get stuck in a loop. It gets stuck in a loop because although the dry run command returns a non-zero exit code indicating that a specific action needs to be taken, this action will only need to be taken once the command is actually run and does not apply for the dry run command. The dry run command is a preview of what would happen when the command is run.

For code REBOOT_REQUIRED, it breaks out of the loop because the maintenance window is exceeded.

For code ZYPPER_UPDATED, it breaks out of the loop on the second attempt because of this code:

if package_manager.get_package_manager_setting(Constants.PACKAGE_MGR_SETTING_REPEAT_PATCH_OPERATION, False):  # We should not see this again
    error_msg = "Unexpected repeated package manager update occurred. Please re-run the update deployment."
    self.status_handler.add_error_to_status(error_msg, Constants.PatchOperationErrorCodes.PACKAGE_MANAGER_FAILURE)
    raise Exception(error_msg, "[{0}]".format(Constants.ERROR_ADDED_TO_STATUS))

This PR fixes this problem by not taking any specific actions when these exit codes are returned. If one of these exit codes is returned on a dry run command, it's logged, and execution continues normally without repeating installation/rebooting.

@benank benank requested review from a team, kjohn-msft and rane-rajasi and removed request for a team April 11, 2022 17:06
@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Merging #135 (6dd9d95) into master (3e34573) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #135      +/-   ##
==========================================
+ Coverage   86.95%   87.04%   +0.09%     
==========================================
  Files          48       48              
  Lines        8230     8262      +32     
==========================================
+ Hits         7156     7192      +36     
+ Misses       1074     1070       -4     
Flag Coverage Δ
python27 85.87% <100.00%> (+0.10%) ⬆️
python39 87.00% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../core/src/package_managers/ZypperPackageManager.py 86.59% <100.00%> (+1.03%) ⬆️
src/core/tests/Test_ZypperPackageManager.py 96.53% <100.00%> (+0.18%) ⬆️
src/core/tests/library/LegacyEnvLayerExtensions.py 94.15% <100.00%> (+0.10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e34573...6dd9d95. Read the comment docs.

Copy link
Collaborator

@kjohn-msft kjohn-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good investigation.. Please address naming and get some basic UT coverage.

Copy link
Collaborator

@kjohn-msft kjohn-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidentally approved.. UT.

@benank benank requested a review from kjohn-msft April 11, 2022 17:30
@benank benank merged commit 386bba3 into master Apr 11, 2022
@benank benank deleted the bankiel-fix-repeated-op branch April 11, 2022 17:52
@benank benank mentioned this pull request Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants