Initial mistral removal from st2 #5011

amanda11 · 2020-08-04T15:31:00Z

Remove mistral and postgres from st2ci workflows as part of the Mistral deprecation

Addresses st2from #4762

Resolves #5012

amanda11 · 2020-08-04T15:31:42Z

Initial mistral removal, to see what happens on the CI runs. Further testing to be done...

amanda11 · 2020-08-04T17:37:10Z

Not ready for review, missed a whole bunch of changes!

…as only if ALL tasks passed

arm4b · 2020-08-10T14:15:42Z

FYI CircleCI build is green now as we merged related st2-packages change: StackStorm/st2-packages#656

amanda11 · 2020-08-10T14:16:32Z

@armab Thanks for merge, the CircleCI tests are now passing.

I still need to work out why I've broken python3 - I would have expected that I'd break python2.7 unit-tests not python3. But I'm investigating this...

amanda11

I've added a review on my own code - with queries to be answered/points to validate.

amanda11 · 2020-08-10T14:42:19Z

st2actions/st2actions/container/base.py

@@ -249,7 +249,7 @@ def _clean_up_auth_token(self, runner, status):
        it doesn't throw.
        """
        # Deletion of the runner generated auth token is delayed until the token expires.
-        # Async actions such as Mistral workflows uses the auth token to launch other
+        # Async actions use the auth token to launch other


What do we want to do about this statement for auth taken, the example comment is about Mistral. Is this relevant now mistral is removed?

Keep this as is because the concept of async actions is generic although Mistral is the only runner that requires this at the moment.

amanda11 · 2020-08-10T14:44:07Z

st2api/st2api/controllers/exp/validation.py

@@ -1,54 +0,0 @@
-# Copyright 2019 Extreme Networks, Inc.
-#


This package now only has an init.py - do we want to leave the st2api/controllers/exp package?

amanda11 · 2020-08-10T14:46:14Z

st2client/tests/unit/test_execution_tail_command.py

-}
-
-MOCK_LIVEACTION_4_CHILD_1_RUNNING = {
-    'id': 'idmistralchild1',


Do we need these tests rewritten for Orquesta?

That would be nice if you can take a look and give it a try. The tail output for orquesta workflow is different though. Thanks!

That would be nice if you can take a look and give it a try. The tail output for orquesta workflow is different though. Thanks!

Taking a look at the output in the scheduler log then it looks like the children of an orquesta workflow have the same fields like task_name but in an orquesta context. There may be different other parameters, but out of the ones that are mocked in these replies they seem to be the same.
I also took a look at a tail of an orquesta log and the output looks the same.

If I'm missing other changes to make to these mock objects please let me know.

amanda11 · 2020-08-10T14:53:11Z

st2common/tests/fixtures/requirements-used-for-tests.txt

@@ -9,7 +9,6 @@ flex==6.14.0
 # commments....
 git+https://github.com/Kami/logshipper.git@stackstorm_patched#egg=logshipper
 git+https://github.com/StackStorm/orquesta.git@224c1a589a6007eb0598a62ee99d674e7836d369#egg=orquesta
-git+https://github.com/StackStorm/python-mistralclient.git#egg=python-mistralclient


For some reason had to manually alter this requirements file even says say don't edit by hand - couldn't find any Makefile or anything that updated it. Is comment at top of file correct?

amanda11 · 2020-08-10T14:55:04Z

st2common/tests/unit/test_runners_base.py

@@ -35,23 +35,11 @@ def test_get_runner_failure_not_found(self):
        self.assertRaisesRegexp(ActionRunnerCreateError, expected_msg,
                                get_runner, 'invalid-name-not-found')

-    def test_get_query_module_success(self):


Only runner that seemed to support query and callback was mistral - so felt had to delete the UT. But I've left in the code about queries in main ST2.

Yes, Mistral is the only one async runner that uses the query/callback. Can we set up a mock async runner in the test setup and then use that here so we preserve the functions and tests for any async runner in the future?

@m4dcoder Is there a way to get it to load mock runners. At moment it looks like the unit-test will only find the real modules, not any in fixtures. I had a play with the fixturesloader, but it doesn't seem to load them at the right level - just in the db.
So the stevedore doesn't load them.

I could MagicMock at the stevedore level, but I wasn't sure if we wanted to test with that in.

Any thoughts? (For now I've put back the tests but commented out)

amanda11 · 2020-08-10T14:57:09Z

st2tests/integration/orquesta/test_wiring_rerun.py

@@ -95,3 +95,53 @@ def test_rerun_task_of_workflow_already_succeeded(self):
        self.assertNotEqual(ex.id, orig_st2_ex_id)
        ex = self._wait_for_state(ex, action_constants.LIVEACTION_STATUS_SUCCEEDED)
        self.assertEqual(ex.context['workflow_execution'], orig_wf_ex_id)
+
+    def test_rerun_and_reset_with_items_task(self):


There were tests for re-run with and without reset in mistral, but couldn't find equivalent in orquesta - so wrote new tests.

amanda11 · 2020-08-10T14:58:26Z

st2tests/st2tests/fixtures/generic/workflows/wb_invalid_syntax.yaml

@@ -1,72 +0,0 @@
-version: "2.0"
-name: "examples.mistral-complex"


These set of workflows were mistral, have not gone through all of these to see if we need to re-write to create orquesta equivalent.

I don't think you need to worry about ensuring there is an Orquesta equivalent as part of this change. It would be good to have those, but there is a decent set of Orquesta examples, so don't worry too much about it right now.

amanda11 · 2020-08-10T15:02:35Z

tox.ini

-       -e{toxinidir}/st2client
-       -e{toxinidir}/st2common
-commands =
-    nosetests --rednose --immediate -sv contrib/runners/mistral_v2/tests/unit/


As the unit tests nightly only seemed to do mistral - I removed them.

amanda11 · 2020-08-10T15:07:47Z

st2client/st2client/commands/action.py

@@ -1269,7 +1266,7 @@ def __init__(self, resource, *args, **kwargs):
                                 help='Name of the workflow tasks to re-run.')
        self.parser.add_argument('--no-reset', dest='no_reset', nargs='*',
                                 help='Name of the with-items tasks to not reset. This only '
-                                      'applies to Mistral workflows. By default, all iterations '
+                                      'applies to Orquesta workflows. By default, all iterations '


Option looks to have always been supported for Orquesta - it is in the documents and we had a unit-test, but the parser just referred to Mistral - so think it should have previously said both - so adjusted to Orquesta.

amanda11 · 2020-08-10T15:10:34Z

st2common/bin/st2-track-result

@@ -52,10 +52,6 @@ def add_result_tracker(exec_id):
    # Check runner type.


If results tracker was only supported for mistral, then should this whole file go? Or should we leave it in with the runner_type != mistral-v2 so the logic is there if need to add for new runners?
What about the actual results tracker itself?

m4dcoder

Great job! This takes a lot of time and effort. This looks good overall. I just have a few minor requests on preserving tests for query/callback for async runner.

m4dcoder · 2020-08-10T17:50:47Z

st2client/tests/unit/test_execution_tail_command.py

-}
-
-MOCK_LIVEACTION_4_CHILD_1_RUNNING = {
-    'id': 'idmistralchild1',


That would be nice if you can take a look and give it a try. The tail output for orquesta workflow is different though. Thanks!

m4dcoder · 2020-08-10T18:12:00Z

st2common/bin/st2-track-result

@@ -44,73 +44,5 @@ def setup():
    script_setup.setup(config, register_mq_exchanges=False)


-def add_result_tracker(exec_id):


Can we keep the st2 result tracker and minimize changes to it? st2 result tracker is a generic solution to pull results from a remote runner instead of having the runner push result into st2. It is not specific to Mistral. Although Mistral is the only one that uses it.

Can we keep the st2 result tracker and minimize changes to it? st2 result tracker is a generic solution to pull results from a remote runner instead of having the runner push result into st2. It is not specific to Mistral. Although Mistral is the only one that uses it.

Hi @m4dcoder I've put back the st2-track-result back to where it was, but with an additional comment to explain why it's there, and to update when other runners support the result tracker.

m4dcoder · 2020-08-10T18:12:19Z

st2common/bin/st2-track-result

-            add_result_tracker(child_exec_id)
-
-
-def del_result_tracker(exec_id):


See comment above.

m4dcoder · 2020-08-10T18:23:05Z

st2actions/st2actions/container/base.py

@@ -249,7 +249,7 @@ def _clean_up_auth_token(self, runner, status):
        it doesn't throw.
        """
        # Deletion of the runner generated auth token is delayed until the token expires.
-        # Async actions such as Mistral workflows uses the auth token to launch other
+        # Async actions use the auth token to launch other


Keep this as is because the concept of async actions is generic although Mistral is the only runner that requires this at the moment.

m4dcoder · 2020-08-10T18:33:09Z

st2common/tests/unit/test_query_base.py

@@ -40,9 +40,9 @@ def test_fire_queries_doesnt_loop(self):
        mock_query_state_1 = QueryContext(
            uuid.uuid4().hex,
            uuid.uuid4().hex,
-            'mistral_v2',
+            'orquesta_runner',


The orquesta runner doesn't use the Querier. Mistral is the only one that uses the querier. I want to preserve the generic solution here. Can we add a mock async runner in the test setup and use that here instead?

I'll look into getting that mock runner in this week, and update tests to use the mock runner.

m4dcoder · 2020-08-10T18:34:23Z

st2common/tests/unit/test_runners_base.py

@@ -35,23 +35,11 @@ def test_get_runner_failure_not_found(self):
        self.assertRaisesRegexp(ActionRunnerCreateError, expected_msg,
                                get_runner, 'invalid-name-not-found')

-    def test_get_query_module_success(self):


Yes, Mistral is the only one async runner that uses the query/callback. Can we set up a mock async runner in the test setup and then use that here so we preserve the functions and tests for any async runner in the future?

…tests

CHANGELOG.rst

Co-authored-by: Eugen C. <[email protected]>

arm4b · 2020-08-24T11:51:30Z

@m4dcoder looks like this mistral removal PR needs a second review round?

m4dcoder

LGTM. Once this is merged and new 3.3dev packages are built, please do a test run on the packages end to end to make sure st2 isn't generally broken.

Initial mistral removal from st2

a3c009f

pull-request-size bot added the size/XXL PR that changes 1000+ lines. You should absolutely split your PR into several. label Aug 4, 2020

amanda11 marked this pull request as draft August 4, 2020 15:32

amanda11 added 9 commits August 4, 2020 19:50

Add some more changes

ebd4e65

Fix some unit-tests

7503797

Amend name in test action

085fc7a

Update requirements-used-for-tests

c688c71

Add integration orquesta re-run test, as only had mistral before

3e53e6b

Fix lint and amend new test yaml

0762975

Amend new orquesta test yaml file

5473808

Removing re-run orquesta with items due to #5016

d443efa

Create orquesta with-items re-run test as validated that issue seen w…

eab2c08

…as only if ALL tasks passed

arm4b mentioned this pull request Aug 10, 2020

Mistral Deprecation Game Plan #4762

Closed

25 tasks

arm4b added the feature label Aug 10, 2020

Merge branch 'master' into remove_mistral

6650451

arm4b added the breaking change label Aug 10, 2020

Minor fixes

c2d38a4

amanda11 commented Aug 10, 2020

View reviewed changes

amanda11 added 3 commits August 10, 2020 15:24

tools/launchdev.sh

71ad684

Add simple JSON

a20f9dc

Add requirements after rebuilt

446642d

m4dcoder requested changes Aug 10, 2020

View reviewed changes

amanda11 added 2 commits August 10, 2020 19:51

Put back result tracker

ff67a99

Update py3 expected text

f11876f

amanda11 marked this pull request as ready for review August 10, 2020 22:23

amanda11 changed the title ~~Initial mistral removal from st2 [WIP]~~ Initial mistral removal from st2 Aug 10, 2020

amanda11 added 4 commits August 11, 2020 16:53

Added get_query_module and get_callback_modules but as commented out …

11aec44

…tests

Put in orquesta execution tail

c7a35c3

Fix flake8 error

a70d5c9

Merge master

bd85071

amanda11 requested a review from m4dcoder August 13, 2020 16:10

Minor update to UT

5524784

winem mentioned this pull request Aug 13, 2020

Deprecate mistral - remove mistral from the HA config and st2.*.conf samples #5026

Closed

amanda11 added 5 commits August 14, 2020 17:18

Add mock query callback runner

55b3ebc

Amend so python 3 pick up mock runner and fix flake8 and UT error

87b3c35

Add echo of pythonpath for investigation

707d495

Add mock runner to python3.6

8c3db45

Updated travis install requirements

d37418e

arm4b reviewed Aug 17, 2020

View reviewed changes

CHANGELOG.rst Outdated Show resolved Hide resolved

arm4b added this to the 3.3.0 milestone Aug 17, 2020

amanda11 and others added 2 commits August 17, 2020 21:07

Update CHANGELOG.rst

cb501c6

Co-authored-by: Eugen C. <[email protected]>

Merge branch 'master' into remove_mistral

a88e797

m4dcoder approved these changes Aug 24, 2020

View reviewed changes

Merge branch 'master' into remove_mistral

049a559

amanda11 merged commit 0e83fc5 into StackStorm:master Aug 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial mistral removal from st2 #5011

Initial mistral removal from st2 #5011

amanda11 commented Aug 4, 2020 •

edited

Loading

amanda11 commented Aug 4, 2020 •

edited

Loading

amanda11 commented Aug 4, 2020

arm4b commented Aug 10, 2020 •

edited

Loading

amanda11 commented Aug 10, 2020

amanda11 left a comment

amanda11 Aug 10, 2020

m4dcoder Aug 10, 2020

amanda11 Aug 10, 2020

amanda11 Aug 10, 2020

m4dcoder Aug 10, 2020

amanda11 Aug 11, 2020

amanda11 Aug 10, 2020

amanda11 Aug 10, 2020

m4dcoder Aug 10, 2020

amanda11 Aug 11, 2020

amanda11 Aug 10, 2020

amanda11 Aug 10, 2020

LindsayHill Aug 18, 2020

amanda11 Aug 10, 2020

amanda11 Aug 10, 2020

amanda11 Aug 10, 2020

m4dcoder left a comment

m4dcoder Aug 10, 2020

m4dcoder Aug 10, 2020

amanda11 Aug 10, 2020

m4dcoder Aug 10, 2020

m4dcoder Aug 10, 2020

m4dcoder Aug 10, 2020

amanda11 Aug 10, 2020

m4dcoder Aug 10, 2020

arm4b commented Aug 24, 2020

m4dcoder left a comment

		@@ -1,72 +0,0 @@
		version: "2.0"
		name: "examples.mistral-complex"

		@@ -52,10 +52,6 @@ def add_result_tracker(exec_id):
		# Check runner type.

		@@ -44,73 +44,5 @@ def setup():
		script_setup.setup(config, register_mq_exchanges=False)


		def add_result_tracker(exec_id):

		add_result_tracker(child_exec_id)


		def del_result_tracker(exec_id):

Initial mistral removal from st2 #5011

Initial mistral removal from st2 #5011

Conversation

amanda11 commented Aug 4, 2020 • edited Loading

amanda11 commented Aug 4, 2020 • edited Loading

amanda11 commented Aug 4, 2020

arm4b commented Aug 10, 2020 • edited Loading

amanda11 commented Aug 10, 2020

amanda11 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m4dcoder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b commented Aug 24, 2020

m4dcoder left a comment

Choose a reason for hiding this comment

amanda11 commented Aug 4, 2020 •

edited

Loading

amanda11 commented Aug 4, 2020 •

edited

Loading

arm4b commented Aug 10, 2020 •

edited

Loading