From f60a932ebbbf017fb82d648543a196636aaf11d2 Mon Sep 17 00:00:00 2001 From: Tyler Johnson Date: Wed, 12 Aug 2020 16:44:16 -0600 Subject: [PATCH 1/4] add sub_state_return sep --- 0024-sub-state-returns.md | 155 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 0024-sub-state-returns.md diff --git a/0024-sub-state-returns.md b/0024-sub-state-returns.md new file mode 100644 index 0000000..2181bbf --- /dev/null +++ b/0024-sub-state-returns.md @@ -0,0 +1,155 @@ +- Feature Name: Sub State Returns +- Start Date: 2020-08-12 +- SEP Status: Draft +- SEP PR: github.com/saltstack/salt/pull/57993 +- Salt Issue: (leave this empty) + +# Summary +[summary]: #summary + +Add the ability for states to have sub state returns. +For example, if you are running a state that runs several external states under a different engine, +you can now add those external state runs individually to the "sub_state_run" key of the state return. +They will be parsed and printed alongside the salt state runs. + +# Motivation +[motivation]: #motivation + +Salt has the ability to run Ansible Playbooks, Idem SLSs, and Chef configurations. +These "external state engines" run multiple states in their own way and salt currently +only reports whether or not external state runs as a whole were run successfully +(I.E Return status of True if an entire Ansible Playbook was executed successfully). + +We want to add the ability for salt states to have a "sub_state_run", which gives +these external state engines the ability to report the status of each of the "sub states" they +ran independently, rather than reporting on the status of the entire group of sub states. + +The result is that state output will be very clear and verbose, users will be able to see +exactly which idem state failed in their idem SLS or exactly which configurations passed +or failed in Chef, and exactly which ansible plays were successful when statefully +executing a playbook. + + +# Design +[design]: #detailed-design + +Definitions +----------- +- External state engine: A salt module that has the ability to execute a group of configurations (I.E Ansible, Chef, Idem) +- Sub state run: A new key in the return of salt states that allows definitions of external state engine run details + +This is the bulk of the SEP. Explain the design in enough detail for somebody familiar +with the product to understand, and for somebody familiar with the internals to implement. It should include: + +How it is used +-------------- +Some states can return multiple state runs from an external engine. +State modules that extend tools like Ansible, Chef, and Idem can run multiple external +"states" and then return their results individually in the "sub_state_run" portion of their return +as long as their individual state runs are formatted like salt states with low and high data. + +For example, the idem state module can execute multiple idem states +via it's runtime and report the status of all those runs by attaching them to "sub_state_run" in it's state return. +These sub_state_runs will be formatted and printed alongside other salt states. + +Example +------- + +```python +state_return = { + "name": None, # The parent state name + "result": None, # The overall status of the external state engine run + "comment": None, # Comments on the overall external state engine run + "changes": {}, # An empty dictionary, each sub state run has it's own changes to report + "sub_state_run": [ + { + "changes": {}, # A dictionary describing the changes made in the external state run + "result": None, # The external state run name + "comment": None, # Comment on the external state run + "duration": None, # Optional, the duration in seconds of the external state run + "start_time": None, # Optional, the timestamp of the external state run's start time + "low": { + "name": None, # The name of the state from the external state run + "state": None, # Name of the external state run + "__id__": None, # ID of the external state run + "fun": None, # The Function name from the external state run + }, + } + ], +} +``` + +Code Changes +------------ + +salt/state.py would just need to be modified to look for the `sub_state_run` key in +a state return and then process that data the same way it would process a regular state +run, assigning a run_num (and incrementing the global counter), and formatting the low data. + +```python +for sub_state_data in running[tag].pop("sub_state_run", ()): + self.__run_num += 1 + sub_tag = _gen_tag(sub_state_data["low"]) + running[sub_tag] = { + "name": sub_state_data["low"]["name"], + "changes": sub_state_data["changes"], + "result": sub_state_data["result"], + "duration": sub_state_data.get("duration"), + "start_time": sub_state_data.get("start_time"), + "comment": sub_state_data.get("comment"), + "__state_ran__": True, + "__run_num__": self.__run_num, + "__sls__": low["__sls__"], + } +``` + +State Decorators would need to check for errors in these sub state runs. +```python +sub_state_run = None +if isinstance(result, dict): + sub_state_run = result.get("sub_state_run", ()) +result = self._run_policies(result) +if sub_state_run: + result["sub_state_run"] = [ + self._run_policies(sub_state_data) + for sub_state_data in sub_state_run + ] +``` + +The state content checker would need to be modified to verify sub states. +```python +for sub_state in result.get("sub_state_run", ()): + self.content_check(sub_state) +``` + +The tests for this feature would be an exact copy of the existing state tests, +only tweaked to verify `sub_state_run` returns rather than traditional state returns. +```python +def test_sub_state_output_check_changes_is_dict(self): + """ + Test that changes key contains a dictionary. + """ + data = {"changes": {}, "sub_state_run": [{"changes": []}]} + out = statedecorators.OutputUnifier("content_check")(lambda: data)() + assert "'Changes' should be a dictionary" in out["sub_state_run"][0]["comment"] + assert not out["sub_state_run"][0]["result"] +``` + +## Alternatives +[alternatives]: #alternatives + +The impact of NOT doing this is that when calling Idem SLS, Ansible playbooks, and Chef configurations, +The output of state runs will not be verbose and it will not be clear what individual changes +were made after executing a playbook/sls/configuration. + +## Unresolved questions +[unresolved]: #unresolved-questions + +None, the proof of concept is functional, we just to double check with everyone before +making a change to something as critical to salt as the state engine. + +# Drawbacks +[drawbacks]: #drawbacks + +Doing this modifies the state return code, a critical part of salt. +If it is done poorly then it could cause unintended grief down the road. From 62123b70f7f751d131721e2311a5a476ae837a7e Mon Sep 17 00:00:00 2001 From: Tyler Johnson Date: Wed, 12 Aug 2020 16:48:56 -0600 Subject: [PATCH 2/4] removed template sentance --- 0024-sub-state-returns.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/0024-sub-state-returns.md b/0024-sub-state-returns.md index 2181bbf..2111603 100644 --- a/0024-sub-state-returns.md +++ b/0024-sub-state-returns.md @@ -38,8 +38,6 @@ Definitions - External state engine: A salt module that has the ability to execute a group of configurations (I.E Ansible, Chef, Idem) - Sub state run: A new key in the return of salt states that allows definitions of external state engine run details -This is the bulk of the SEP. Explain the design in enough detail for somebody familiar -with the product to understand, and for somebody familiar with the internals to implement. It should include: How it is used -------------- From 9f2ce80fdefb4dc2566158d2b77746619c773d72 Mon Sep 17 00:00:00 2001 From: Tyler Johnson Date: Wed, 12 Aug 2020 16:55:58 -0600 Subject: [PATCH 3/4] formatting changes --- 0024-sub-state-returns.md | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/0024-sub-state-returns.md b/0024-sub-state-returns.md index 2111603..332da38 100644 --- a/0024-sub-state-returns.md +++ b/0024-sub-state-returns.md @@ -1,4 +1,4 @@ -- Feature Name: Sub State Returns +- Feature Name: Sub State Runs - Start Date: 2020-08-12 - SEP Status: Draft - SEP PR: github.com/saltstack/salt/pull/57993 @@ -7,25 +7,25 @@ # Summary [summary]: #summary -Add the ability for states to have sub state returns. +Add the ability for states to have sub state runs. For example, if you are running a state that runs several external states under a different engine, you can now add those external state runs individually to the "sub_state_run" key of the state return. -They will be parsed and printed alongside the salt state runs. +They will be parsed and printed alongside all the Salt state returns. # Motivation [motivation]: #motivation -Salt has the ability to run Ansible Playbooks, Idem SLSs, and Chef configurations. -These "external state engines" run multiple states in their own way and salt currently -only reports whether or not external state runs as a whole were run successfully -(I.E Return status of True if an entire Ansible Playbook was executed successfully). +Salt has the ability to run Ansible playbooks, Idem SLSs, and Chef cookbooks. +These "external state engines" run multiple states in their own way and Salt currently +only reports whether or not external state runs as a whole were run successfully. +I.E Return status of True if an entire Ansible playbook was executed successfully in an ansiblegate state -We want to add the ability for salt states to have a "sub_state_run", which gives +We want to add the ability for Salt states to have a "sub_state_run", which gives these external state engines the ability to report the status of each of the "sub states" they ran independently, rather than reporting on the status of the entire group of sub states. The result is that state output will be very clear and verbose, users will be able to see -exactly which idem state failed in their idem SLS or exactly which configurations passed +exactly which idem state failed in their idem SLS or exactly which cookbooks passed or failed in Chef, and exactly which ansible plays were successful when statefully executing a playbook. @@ -35,8 +35,8 @@ executing a playbook. Definitions ----------- -- External state engine: A salt module that has the ability to execute a group of configurations (I.E Ansible, Chef, Idem) -- Sub state run: A new key in the return of salt states that allows definitions of external state engine run details +- External state engine: A Salt module that has the ability to execute a group of configurations (I.E Ansible, Chef, Idem) +- Sub state run: A new key in the return of Salt states that allows definitions of external state engine run details How it is used @@ -44,11 +44,11 @@ How it is used Some states can return multiple state runs from an external engine. State modules that extend tools like Ansible, Chef, and Idem can run multiple external "states" and then return their results individually in the "sub_state_run" portion of their return -as long as their individual state runs are formatted like salt states with low and high data. +as long as their individual state runs are formatted like Salt states with low and high data. For example, the idem state module can execute multiple idem states via it's runtime and report the status of all those runs by attaching them to "sub_state_run" in it's state return. -These sub_state_runs will be formatted and printed alongside other salt states. +These sub_state_runs will be formatted and printed alongside other Salt states. Example ------- @@ -136,18 +136,18 @@ def test_sub_state_output_check_changes_is_dict(self): ## Alternatives [alternatives]: #alternatives -The impact of NOT doing this is that when calling Idem SLS, Ansible playbooks, and Chef configurations, +The impact of NOT doing this is that when calling Idem SLS, Ansible playbooks, and Chef cookbooks, The output of state runs will not be verbose and it will not be clear what individual changes -were made after executing a playbook/sls/configuration. +were made after executing a playbook/sls/cookbook. ## Unresolved questions [unresolved]: #unresolved-questions None, the proof of concept is functional, we just to double check with everyone before -making a change to something as critical to salt as the state engine. +making a change to something as critical to Salt as the state engine. # Drawbacks [drawbacks]: #drawbacks -Doing this modifies the state return code, a critical part of salt. +Doing this modifies the state return code, a critical part of Salt. If it is done poorly then it could cause unintended grief down the road. From e4dd22118b2c00f752c6313af450bddee1579426 Mon Sep 17 00:00:00 2001 From: Tyler Johnson Date: Wed, 12 Aug 2020 17:05:16 -0600 Subject: [PATCH 4/4] updating some wording --- 0024-sub-state-returns.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/0024-sub-state-returns.md b/0024-sub-state-returns.md index 332da38..adc7713 100644 --- a/0024-sub-state-returns.md +++ b/0024-sub-state-returns.md @@ -16,18 +16,18 @@ They will be parsed and printed alongside all the Salt state returns. [motivation]: #motivation Salt has the ability to run Ansible playbooks, Idem SLSs, and Chef cookbooks. -These "external state engines" run multiple states in their own way and Salt currently -only reports whether or not external state runs as a whole were run successfully. +These "external state engines" run multiple "states" in their own way and Salt currently +only reports whether or not external state runs as a whole were successful and doesn't report individual changes. I.E Return status of True if an entire Ansible playbook was executed successfully in an ansiblegate state -We want to add the ability for Salt states to have a "sub_state_run", which gives +We want to add the ability for Salt state returns to have a "sub_state_run" key, which gives these external state engines the ability to report the status of each of the "sub states" they ran independently, rather than reporting on the status of the entire group of sub states. -The result is that state output will be very clear and verbose, users will be able to see -exactly which idem state failed in their idem SLS or exactly which cookbooks passed -or failed in Chef, and exactly which ansible plays were successful when statefully -executing a playbook. +The result is that state output will be very clear and verbose. +Users will be able to see exactly which `idem state` passed/failed in an `idem SLS`, +exactly which recipes passed/failed in a Chef cookbook, +and exactly which ansible plays passed/failed in a playbook # Design