salt.outputter.highstate: recursion for orch #46022

mattp- · 2018-02-14T00:45:33Z

What does this PR do?

orchestration / nested runners emitting to highstate were not rendered as
highstate like a normal state apply. With this change, recurse and indent
accordingly based on nested orchestrations.

Previous Behavior

nested orch / states were just rendered as raw data.

          ID: sleep 2
    Function: cmd.run
      Result: True
     Comment: Command "sleep 2" run
     Started: 15:33:47.059209
    Duration: 2022.743 ms
     Changes:
              ----------
              pid:
                  22643
              retcode:
                  0
              stderr:
              stdout:
----------
          ID: par2
    Function: salt.runner
        Name: state.orchestrate
      Result: True
     Comment: Runner function 'state.orchestrate' executed.
     Started: 15:33:49.084557
    Duration: 3026.912 ms
     Changes:
              ----------
              data:
                  ----------
                  saltlab-syndic02:
                      ----------
                      test_|-always-passes-with-any-kwarg_|-aaaa_|-nop:
                          ----------
                          __id__:
                              always-passes-with-any-kwarg
                          __run_num__:
                              0
                          __sls__:
                              orch.test.par2
                          changes:
                              ----------
                          comment:
                              Success!
                          duration:
                              0.881
                          name:
                              aaaa
                          result:
                              True
                          start_time:
                              15:33:52.090657
                      test_|-always-passes-with-changes_|-oinaosf_|-succeed_with_changes:
                          ----------
                          __id__:
                              always-passes-with-changes
                          __run_num__:
                              2
                          __sls__:
                              orch.test.par2
                          changes:
                              ----------
                              testing:
                                  ----------
                                  new:
                                      Something pretended to change
                                  old:
                                      Unchanged
                          comment:
                              Success!
                          duration:
                              0.853
                          name:
                              oinaosf
                          result:
                              True
                          start_time:
                              15:33:52.093029
                      test_|-always-passes_|-fasdfasddfasdfoo_|-succeed_without_changes:
                          ----------
                          __id__:
                              always-passes
                          __run_num__:
                              1
                          __sls__:
                              orch.test.par2
                          changes:
                              ----------
                          comment:
                              Success!
                          duration:
                              0.897
                          name:
                              fasdfasddfasdfoo
                          result:
                              True
                          start_time:
                              15:33:52.091850
              outputter:
                  highstate
              retcode:
                  0
----------
          ID: par4
    Function: salt.runner
        Name: test.arg
      Result: True
     Comment: Runner function 'test.arg' executed.
     Started: 15:33:52.111810
    Duration: 1444.039 ms
     Changes:
              ----------
              args:
                  - 1
                  - 2
              kwargs:
                  ----------

Summary for saltlab-syndic02
------------
Succeeded: 3 (changed=3)
Failed:    0
------------
Total states run:     3
Total run time:   6.494 s

New Behavior

orch/state output is now rendered as nested inner highstates:

local_master:
----------
          ID: sleep 2
    Function: cmd.run
      Result: True
     Comment: Command "sleep 2" run
     Started: 15:23:59.088329
    Duration: 5.811 ms
     Changes:
              ----------
              pid:
                  30087
              retcode:
                  0
              stderr:
              stdout:
 ----------
          ID: par2
    Function: salt.runner
        Name: state.orchestrate
      Result: True
     Comment: Runner function 'state.orchestrate' executed.
     Started: 15:24:01.115448
    Duration: 2.442 ms
     Changes:
              local_master:
              ----------
                        ID: always-passes-with-any-kwarg
                  Function: test.nop
                      Name: aaaa
                    Result: True
                   Comment: Success!
                   Started: 15:24:02.741617
                  Duration: 3.316 ms
                   Changes:
              ----------
                        ID: always-passes
                  Function: test.succeed_without_changes
                      Name: fasdfasddfasdfoo
                    Result: True
                   Comment: Success!
                   Started: 15:24:02.745295
                  Duration: 0.717 ms
                   Changes:
              ----------
                        ID: always-passes-with-changes
                  Function: test.succeed_with_changes
                      Name: oinaosf
                    Result: True
                   Comment: Success!
                   Started: 15:24:02.746166
                  Duration: 0.421 ms
                   Changes:
                            ----------
                            testing:
                                ----------
                                new:
                                    Something pretended to change
                                old:
                                    Unchanged

              Summary for local_master
              ------------
              Succeeded: 3 (changed=1)
              Failed:    0
              ------------
              Total states run:     3
              Total run time:   4.454 ms
----------
          ID: par4
    Function: salt.runner
        Name: test.arg
      Result: True
     Comment: Runner function 'test.arg' executed.
     Started: 15:24:01.118253
    Duration: 3.994 ms
     Changes:
              ----------
              return:
                  ----------
                  args:
                      - 1
                      - 2
                  kwargs:
                      ----------

Summary for local_master
------------
Succeeded: 3 (changed=3)
Failed:    0
------------
Total states run:     3
Total run time:  12.247 ms

Tests written?

No

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

rallytime · 2018-02-15T19:34:45Z

@mattp- Thanks for submitting this! Offhand, there's a lint error here: https://jenkins.saltstack.com/job/PR/job/salt-pr-lint-n/19299/violations/file/salt/output/highstate.py/

rallytime

Tests need to be written to make sure this change doesn't regress, too.

rallytime · 2018-02-15T19:35:49Z

salt/output/highstate.py

-            nchanges += 1 if schanged else 0
+            if ret.get('name') in ['state.orch', 'state.orchestrate', 'state.sls']:
+                nested = output(ret['changes']['return'], indent_level=indent_level+1)
+                ctext = re.sub('^',u' ' * 14 * indent_level, '\n'+nested, flags=re.MULTILINE)


We shouldn't be using the u'' syntax here. That should be using salt.utils.stringutils.to_unicode instead.

Ah my bad, originally wrote this on a release branch pre Unicode change. Will update and add a regression test

@mattp- All good! Thank you :)

The u is still there. Also, there need to be spaces after commas and between operators, or else the linter will complain.

I don't think we'll need to use salt.utils.stringutils.to_unicode() here, due to unicode_literals being imported. The nested variable should contain a unicode string because the output() function returns unicode strings.

cachedout

Can we get tests written for this?

terminalmage · 2018-03-04T17:49:51Z

salt/output/highstate.py

-            nchanges += 1 if schanged else 0
+            if ret.get('name') in ['state.orch', 'state.orchestrate', 'state.sls']:
+                nested = output(ret['changes']['return'], indent_level=indent_level+1)
+                ctext = re.sub('^',u' ' * 14 * indent_level, '\n'+nested, flags=re.MULTILINE)


The u is still there. Also, there need to be spaces after commas and between operators, or else the linter will complain.

I don't think we'll need to use salt.utils.stringutils.to_unicode() here, due to unicode_literals being imported. The nested variable should contain a unicode string because the output() function returns unicode strings.

mattp- · 2018-03-26T13:36:52Z

@rallytime / @terminalmage I've added a regression test and updated to correct the lint errors. can you please take another gander? Thanks

terminalmage

There is still a single lint issue that needs to be fixed. The ones in test_error.py are unrelated and already fixed upstream, if you rebase against the head of our develop branch after fixing the lint error I pointed out below, then it should get the lint check to pass.

terminalmage · 2018-03-28T19:33:22Z

tests/unit/output/test_highstate_out.py

+            'retcode': 0
+        }
+
+


This line needs to be removed to satisfy the linter.

Line 186 in case the above comment is ambiguous.

mattp- · 2018-03-30T17:02:15Z

Lint issues should now be resolved

orchestration / nested runners emitting to highstate were not rendered as highstate like a normal state apply. With this change, recurse and indent accordingly based on nested orchestrations.

mattp- force-pushed the nested-orch-highstate branch from 6137184 to 3ed7b48 Compare February 14, 2018 19:28

rallytime suggested changes Feb 15, 2018

View reviewed changes

rallytime added the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Feb 15, 2018

rallytime requested a review from a team February 15, 2018 19:36

cachedout suggested changes Mar 1, 2018

View reviewed changes

terminalmage suggested changes Mar 4, 2018

View reviewed changes

mattp- force-pushed the nested-orch-highstate branch 4 times, most recently from d960c6a to 1556338 Compare March 26, 2018 13:36

terminalmage suggested changes Mar 28, 2018

View reviewed changes

mattp- force-pushed the nested-orch-highstate branch 2 times, most recently from 1fec871 to 2928817 Compare March 29, 2018 17:03

mattp- force-pushed the nested-orch-highstate branch 2 times, most recently from 8b4f76a to 6966347 Compare April 5, 2018 15:52

salt.outputter.highstate: recursion for orch

98cfa1f

orchestration / nested runners emitting to highstate were not rendered as highstate like a normal state apply. With this change, recurse and indent accordingly based on nested orchestrations.

mattp- force-pushed the nested-orch-highstate branch from 6966347 to 98cfa1f Compare April 5, 2018 15:55

Merge branch 'develop' into nested-orch-highstate

13f72bb

rallytime removed the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Apr 6, 2018

rallytime requested review from cachedout, terminalmage and rallytime April 10, 2018 20:55

Merge branch 'develop' into nested-orch-highstate

d61fdd9

terminalmage approved these changes Apr 11, 2018

View reviewed changes

rallytime approved these changes Apr 11, 2018

View reviewed changes

cachedout approved these changes Apr 11, 2018

View reviewed changes

cachedout merged commit 4e44fee into saltstack:develop Apr 11, 2018

aneeshusa mentioned this pull request Apr 9, 2019

Restore pchanges in highstate outputter #52451

Closed

DmitryKuzmenko mentioned this pull request Sep 10, 2019

salt.runner test mode not working #54382

Closed

max-arnold mentioned this pull request Mar 27, 2021

Remove unused __orchestration__ ret key #59917

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

salt.outputter.highstate: recursion for orch #46022

salt.outputter.highstate: recursion for orch #46022

mattp- commented Feb 14, 2018 •

edited

Loading

rallytime commented Feb 15, 2018

rallytime left a comment

rallytime Feb 15, 2018

mattp- Feb 15, 2018

rallytime Feb 16, 2018

terminalmage Mar 4, 2018

cachedout left a comment

terminalmage Mar 4, 2018

mattp- commented Mar 26, 2018

terminalmage left a comment

terminalmage Mar 28, 2018

terminalmage Mar 28, 2018

mattp- commented Mar 30, 2018

salt.outputter.highstate: recursion for orch #46022

salt.outputter.highstate: recursion for orch #46022

Conversation

mattp- commented Feb 14, 2018 • edited Loading

What does this PR do?

Previous Behavior

New Behavior

Tests written?

Commits signed with GPG?

rallytime commented Feb 15, 2018

rallytime left a comment

Choose a reason for hiding this comment

rallytime Feb 15, 2018

Choose a reason for hiding this comment

mattp- Feb 15, 2018

Choose a reason for hiding this comment

rallytime Feb 16, 2018

Choose a reason for hiding this comment

terminalmage Mar 4, 2018

Choose a reason for hiding this comment

cachedout left a comment

Choose a reason for hiding this comment

terminalmage Mar 4, 2018

Choose a reason for hiding this comment

mattp- commented Mar 26, 2018

terminalmage left a comment

Choose a reason for hiding this comment

terminalmage Mar 28, 2018

Choose a reason for hiding this comment

terminalmage Mar 28, 2018

Choose a reason for hiding this comment

mattp- commented Mar 30, 2018

mattp- commented Feb 14, 2018 •

edited

Loading