Merge pull request #52601 from Ch3LL/cp-52415

Cherry-Pick #52415 into 2019.2.1

doc/ref/states/writing.rst

@@ -259,10 +259,6 @@ A State Module must return a dict containing the following keys/values:
   Prefer to keep line lengths short (use multiple lines as needed),
   and end with punctuation (e.g. a period) to delimit multiple comments.
 
-The return data can also, include the **pchanges** key, this stands for
-`predictive changes`. The **pchanges** key informs the State system what
-changes are predicted to occur.
-
 .. note::
 
     States should not return data which cannot be serialized such as frozensets.
@@ -448,7 +444,6 @@ Example state module
             'changes': {},
             'result': False,
             'comment': '',
-            'pchanges': {},
             }
 
         # Start with basic error-checking. Do all the passed parameters make sense
@@ -469,7 +464,7 @@ Example state module
         # in ``test=true`` mode.
         if __opts__['test'] == True:
             ret['comment'] = 'The state of "{0}" will be changed.'.format(name)
-            ret['pchanges'] = {
+            ret['changes'] = {
                 'old': current_state,
                 'new': 'Description, diff, whatever of the new state',
             }

salt/states/archive.py

@@ -1436,25 +1436,19 @@ def extracted(name,
                 dir_result = __states__['file.directory'](full_path,
                                                           user=user,
                                                           group=group,
-                                                          recurse=recurse,
-                                                          test=__opts__['test'])
+                                                          recurse=recurse)
                 log.debug('file.directory: %s', dir_result)
 
-                if __opts__['test']:
-                    if dir_result.get('pchanges'):
-                        ret['changes']['updated ownership'] = True
-                else:
-                    try:
-                        if dir_result['result']:
-                            if dir_result['changes']:
-                                ret['changes']['updated ownership'] = True
-                        else:
-                            enforce_failed.append(full_path)
-                    except (KeyError, TypeError):
-                        log.warning(
-                            'Bad state return %s for file.directory state on %s',
-                            dir_result, dirname
-                        )
+                if dir_result.get('changes'):
+                    ret['changes']['updated ownership'] = True
+                try:
+                    if not dir_result['result']:
+                        enforce_failed.append(full_path)
+                except (KeyError, TypeError):
+                    log.warning(
+                        'Bad state return %s for file.directory state on %s',
+                        dir_result, dirname
+                    )
 
         for filename in enforce_files + enforce_links:
             full_path = os.path.join(name, filename)

salt/states/boto_cloudfront.py

@@ -135,7 +135,7 @@ def present(
         if __opts__['test']:
             ret['result'] = None
             ret['comment'] = 'Distribution {0} set for creation.'.format(name)
-            ret['pchanges'] = {'old': None, 'new': name}
+            ret['changes'] = {'old': None, 'new': name}
             return ret
 
         res = __salt__['boto_cloudfront.create_distribution'](
@@ -203,7 +203,7 @@ def _yaml_safe_dump(attrs):
                 'Distribution {0} set for new config:'.format(name),
                 changes_diff,
             ])
-            ret['pchanges'] = {'diff': changes_diff}
+            ret['changes'] = {'diff': changes_diff}
             return ret
 
         res = __salt__['boto_cloudfront.update_distribution'](

salt/states/boto_s3.py

@@ -282,7 +282,7 @@ def _yaml_safe_dump(attrs):
         ret['result'] = None
         ret['comment'] = 'S3 object {0} set to be {1}d.'.format(name, action)
         ret['comment'] += '\nChanges:\n{0}'.format(changes_diff)
-        ret['pchanges'] = {'diff': changes_diff}
+        ret['changes'] = {'diff': changes_diff}
         return ret
 
     r = __salt__['boto_s3.upload_file'](

salt/states/boto_sqs.py

@@ -136,7 +136,7 @@ def present(
             ret['comment'].append(
                 'SQS queue {0} is set to be created.'.format(name),
             )
-            ret['pchanges'] = {'old': None, 'new': name}
+            ret['changes'] = {'old': None, 'new': name}
             return ret
 
         r = __salt__['boto_sqs.create'](
@@ -225,7 +225,7 @@ def _yaml_safe_dump(attrs):
                 attributes_diff,
             )
         )
-        ret['pchanges'] = {'attributes': {'diff': attributes_diff}}
+        ret['changes'] = {'attributes': {'diff': attributes_diff}}
         return ret
 
     r = __salt__['boto_sqs.set_attributes'](
@@ -300,7 +300,7 @@ def absent(
     if __opts__['test']:
         ret['result'] = None
         ret['comment'] = 'SQS queue {0} is set to be removed.'.format(name)
-        ret['pchanges'] = {'old': name, 'new': None}
+        ret['changes'] = {'old': name, 'new': None}
         return ret
 
     r = __salt__['boto_sqs.delete'](

salt/states/chocolatey.py

@@ -336,7 +336,6 @@ def upgraded(name,
     ret = {'name': name,
            'result': True,
            'changes': {},
-           'pchanges': {},
            'comment': ''}
 
     # Get list of currently installed packages
@@ -346,12 +345,10 @@ def upgraded(name,
     # Package not installed
     if name.lower() not in [package.lower() for package in pre_install.keys()]:
         if version:
-            ret['pchanges'] = {
-                name: 'Version {0} will be installed'.format(version)
-            }
+            ret['changes'][name] = 'Version {0} will be installed'.format(version)
             ret['comment'] = 'Install version {0}'.format(version)
         else:
-            ret['pchanges'] = {name: 'Latest version will be installed'}
+            ret['changes'][name] = 'Latest version will be installed'
             ret['comment'] = 'Install latest version'
 
     # Package installed
@@ -378,8 +375,7 @@ def upgraded(name,
                     oper="==",
                     ver2=version):
                 if force:
-                    ret['pchanges'] = {
-                        name: 'Version {0} will be reinstalled'.format(version)}
+                    ret['changes'][name] = 'Version {0} will be reinstalled'.format(version)
                     ret['comment'] = 'Reinstall {0} {1}'.format(full_name, version)
                 else:
                     ret['comment'] = '{0} {1} is already installed'.format(
@@ -389,11 +385,9 @@ def upgraded(name,
                 # If installed version is older than new version
                 if salt.utils.versions.compare(
                         ver1=installed_version, oper="<", ver2=version):
-                    ret['pchanges'] = {
-                        name: 'Version {0} will be upgraded to Version {1}'.format(
-                            installed_version, version
-                        )
-                    }
+                    ret['changes'][name] = 'Version {0} will be upgraded to Version {1}'.format(
+                        installed_version, version
+                    )
                     ret['comment'] = 'Upgrade {0} {1} to {2}'.format(
                         full_name, installed_version, version
                     )
@@ -409,13 +403,13 @@ def upgraded(name,
         else:
             ret['comment'] = 'No version found to install'
 
-    # Return if `test=True`
-    if __opts__['test']:
-        ret['result'] = None
+    # Return if there are no changes to be made
+    if not ret['changes']:
         return ret
 
-    # Return if there are no changes to be made
-    if not ret['pchanges']:
+    # Return if running in test mode
+    if __opts__['test']:
+        ret['result'] = None
         return ret
 
     # Install the package
@@ -439,6 +433,9 @@ def upgraded(name,
     # Get list of installed packages after 'chocolatey.install'
     post_install = __salt__['chocolatey.list'](local_only=True)
 
+    # Prior to this, ret['changes'] would have contained expected changes,
+    # replace them with the actual changes now that we have completed the
+    # installation.
     ret['changes'] = salt.utils.data.compare_dicts(pre_install, post_install)
 
     return ret

salt/states/dvs.py

@@ -401,13 +401,11 @@ def dvs_configured(name, dvs):
                                 ''.format(dvs_name, datacenter_name)),
                     'result': True})
     else:
-        ret.update({'comment': '\n'.join(comments)})
-        if __opts__['test']:
-            ret.update({'pchanges': changes,
-                        'result': None})
-        else:
-            ret.update({'changes': changes,
-                        'result': True})
+        ret.update({
+            'comment': '\n'.join(comments),
+            'changes': changes,
+            'result': None if __opts__['test'] else True,
+        })
     return ret
 
 
@@ -512,8 +510,10 @@ def portgroups_configured(name, dvs, portgroups):
     log.info('Running state {0} on DVS \'{1}\', datacenter '
              '\'{2}\''.format(name, dvs, datacenter))
     changes_required = False
-    ret = {'name': name, 'changes': {}, 'result': None, 'comment': None,
-           'pchanges': {}}
+    ret = {'name': name,
+           'changes': {},
+           'result': None,
+           'comment': None}
     comments = []
     changes = {}
     changes_required = False
@@ -623,13 +623,11 @@ def portgroups_configured(name, dvs, portgroups):
                                 'Nothing to be done.'.format(dvs, datacenter)),
                     'result': True})
     else:
-        ret.update({'comment': '\n'.join(comments)})
-        if __opts__['test']:
-            ret.update({'pchanges': changes,
-                        'result': None})
-        else:
-            ret.update({'changes': changes,
-                        'result': True})
+        ret.update({
+            'comment': '\n'.join(comments),
+            'changes': changes,
+            'result': None if __opts__['test'] else True,
+        })
     return ret
 
 
@@ -649,8 +647,10 @@ def uplink_portgroup_configured(name, dvs, uplink_portgroup):
     log.info('Running {0} on DVS \'{1}\', datacenter \'{2}\''
              ''.format(name, dvs, datacenter))
     changes_required = False
-    ret = {'name': name, 'changes': {}, 'result': None, 'comment': None,
-           'pchanges': {}}
+    ret = {'name': name,
+           'changes': {},
+           'result': None,
+           'comment': None}
     comments = []
     changes = {}
     changes_required = False
@@ -708,11 +708,9 @@ def uplink_portgroup_configured(name, dvs, uplink_portgroup):
                                 'Nothing to be done.'.format(dvs, datacenter)),
                     'result': True})
     else:
-        ret.update({'comment': '\n'.join(comments)})
-        if __opts__['test']:
-            ret.update({'pchanges': changes,
-                        'result': None})
-        else:
-            ret.update({'changes': changes,
-                        'result': True})
+        ret.update({
+            'comment': '\n'.join(comments),
+            'changes': changes,
+            'result': None if __opts__['test'] else True,
+        })
     return ret

salt/states/esxdatacenter.py

@@ -89,11 +89,11 @@ def datacenter_configured(name):
         dc_name = name
     log.info('Running datacenter_configured for datacenter \'{0}\''
              ''.format(dc_name))
-    ret = {'name': name, 'changes': {}, 'pchanges': {},
-           'result': None, 'comment': 'Default'}
+    ret = {'name': name,
+           'changes': {},
+           'result': None,
+           'comment': 'Default'}
     comments = []
-    changes = {}
-    pchanges = {}
     si = None
     try:
         si = __salt__['vsphere.get_service_instance_via_proxy']()
@@ -103,27 +103,19 @@ def datacenter_configured(name):
             if __opts__['test']:
                 comments.append('State will create '
                                 'datacenter \'{0}\'.'.format(dc_name))
-                log.info(comments[-1])
-                pchanges.update({'new': {'name': dc_name}})
             else:
                 log.debug('Creating datacenter \'{0}\'. '.format(dc_name))
                 __salt__['vsphere.create_datacenter'](dc_name, si)
                 comments.append('Created datacenter \'{0}\'.'.format(dc_name))
-                log.info(comments[-1])
-                changes.update({'new': {'name': dc_name}})
+            log.info(comments[-1])
+            ret['changes'].update({'new': {'name': dc_name}})
         else:
             comments.append('Datacenter \'{0}\' already exists. Nothing to be '
                             'done.'.format(dc_name))
             log.info(comments[-1])
         __salt__['vsphere.disconnect'](si)
-        if __opts__['test'] and pchanges:
-            ret_status = None
-        else:
-            ret_status = True
-        ret.update({'result': ret_status,
-                    'comment': '\n'.join(comments),
-                    'changes': changes,
-                    'pchanges': pchanges})
+        ret['comment'] = '\n'.join(comments)
+        ret['result'] = None if __opts__['test'] and ret['changes'] else True
         return ret
     except salt.exceptions.CommandExecutionError as exc:
         log.error('Error: {}'.format(exc))

salt/states/esxi.py

@@ -1070,8 +1070,10 @@ def diskgroups_configured(name, diskgroups, erase_disks=False):
                else proxy_details['esxi_host']
     log.info('Running state {0} for host \'{1}\''.format(name, hostname))
     # Variable used to return the result of the invocation
-    ret = {'name': name, 'result': None, 'changes': {},
-           'pchanges': {}, 'comments': None}
+    ret = {'name': name,
+           'result': None,
+           'changes': {},
+           'comments': None}
     # Signals if errors have been encountered
     errors = False
     # Signals if changes are required
@@ -1294,12 +1296,8 @@ def diskgroups_configured(name, diskgroups, erase_disks=False):
               None if __opts__['test'] else  # running in test mode
               False if errors else True)  # found errors; defaults to True
     ret.update({'result': result,
-                'comment': '\n'.join(comments)})
-    if changes:
-        if __opts__['test']:
-            ret['pchanges'] = diskgroup_changes
-        elif changes:
-            ret['changes'] = diskgroup_changes
+                'comment': '\n'.join(comments),
+                'changes': diskgroup_changes})
     return ret
 
 
@@ -1387,8 +1385,10 @@ def host_cache_configured(name, enabled, datastore, swap_size='100%',
                else proxy_details['esxi_host']
     log.trace('hostname = %s', hostname)
     log.info('Running host_cache_swap_configured for host \'%s\'', hostname)
-    ret = {'name': hostname, 'comment': 'Default comments',
-           'result': None, 'changes': {}, 'pchanges': {}}
+    ret = {'name': hostname,
+           'comment': 'Default comments',
+           'result': None,
+           'changes': {}}
     result = None if __opts__['test'] else True  # We assume success
     needs_setting = False
     comments = []
@@ -1582,11 +1582,8 @@ def host_cache_configured(name, enabled, datastore, swap_size='100%',
         __salt__['vsphere.disconnect'](si)
         log.info(comments[-1])
         ret.update({'comment': '\n'.join(comments),
-                    'result': result})
-        if __opts__['test']:
-            ret['pchanges'] = changes
-        else:
-            ret['changes'] = changes
+                    'result': result,
+                    'changes': changes})
         return ret
     except CommandExecutionError as err:
         log.error('Error: %s.', err)