Fix migration scripts execution sequence

The following changes have been introduced as a fix for this issue:

1. Changed the sorting on the migration script file names to be based
on the first number on the file name.
2. Added file name format validation: "nnn-*.*", where "nnn" string
shall contain only digits.
3. Fixed the name of two migration scripts that were not following the
correct format (not using "-" separator).
4. Added set of unit tests to test and validate the execution of
migration scripts code.

Manual upgrade testing to STX 5.0 has been executed.

Closes-Bug: 1887985
Signed-off-by: Adriano Oliveira <adriano.oliveira@windriver.com>
Change-Id: I04fdb8a3b3e177c609c4037825810a531954d99c

.zuul.yaml

@@ -12,6 +12,8 @@
         - sysinv-tox-flake8
         - sysinv-tox-pylint
         - sysinv-tox-bandit
+        - controllerconfig-tox-py27
+        - controllerconfig-tox-py36
         - controllerconfig-tox-flake8
         - controllerconfig-tox-pylint
         - cgtsclient-tox-py27
@@ -26,6 +28,8 @@
         - sysinv-tox-flake8
         - sysinv-tox-pylint
         - sysinv-tox-bandit
+        - controllerconfig-tox-py27
+        - controllerconfig-tox-py36
         - controllerconfig-tox-flake8
         - controllerconfig-tox-pylint
         - cgtsclient-tox-py27
@@ -105,6 +109,30 @@
       tox_envlist: bandit
       tox_extra_args: -c sysinv/sysinv/sysinv/tox.ini
 
+- job:
+    name: controllerconfig-tox-py27
+    parent: tox
+    description: Run py27 tests for controllerconfig
+    required-projects:
+      - starlingx/fault
+    files:
+      - controllerconfig/*
+    vars:
+      tox_envlist: py27
+      tox_extra_args: -c controllerconfig/controllerconfig/tox.ini
+
+- job:
+    name: controllerconfig-tox-py36
+    parent: tox
+    description: Run py36 tests for controllerconfig
+    required-projects:
+      - starlingx/fault
+    files:
+      - controllerconfig/*
+    vars:
+      tox_envlist: py36
+      tox_extra_args: -c controllerconfig/controllerconfig/tox.ini
+
 - job:
     name: controllerconfig-tox-flake8
     parent: tox

controllerconfig/controllerconfig/.stestr.conf

@@ -0,0 +1,3 @@
+[DEFAULT]
+test_path=./controllerconfig/tests
+top_dir=./controllerconfig

controllerconfig/controllerconfig/controllerconfig/tests/__init__.py

@@ -0,0 +1,5 @@
+#
+# Copyright (c) 2021 Wind River Systems, Inc.
+#
+# SPDX-License-Identifier: Apache-2.0
+#

controllerconfig/controllerconfig/controllerconfig/tests/upgrades/__init__.py

@@ -0,0 +1,5 @@
+#
+# Copyright (c) 2021 Wind River Systems, Inc.
+#
+# SPDX-License-Identifier: Apache-2.0
+#

controllerconfig/controllerconfig/controllerconfig/tests/upgrades/test_migration_scripts.py

@@ -0,0 +1,144 @@
+# Copyright (c) 2021 Wind River Systems, Inc.
+#
+# SPDX-License-Identifier: Apache-2.0
+#
+
+"""Base test code to test migration scripts
+First, focus on the migration script name validation
+Second, the validation script sequence call
+"""
+
+from mockproc import mockprocess
+from os import listdir
+from os.path import isfile
+from os.path import join
+from tempfile import mkdtemp
+
+import os
+import unittest
+
+from controllerconfig.upgrades import utils
+
+
+# The way to assert is to pass a script execution that writes the script file
+# name into a file
+# The content of the file will contain the sequence of the called scripts
+script_body = '''#! /usr/bin/env python
+with open('%s', 'a+') as f:
+    f.write("%s")
+'''
+
+from_release = "20.06"
+to_release = "20.12"
+action = "migrate"
+
+# Lists to add scripts to be called, use a ":" separator for
+# parsing/asserting
+validScripts1 = ["71-bla1-bla2-bla3.sh", "8-bla1-bla2-bla3.py:",
+                 "21-bla1-bla2-bla3.sh:"]
+
+validScripts2 = ["75-deployment-ns-upgrade.py:", "65-k8s-app-upgrade.sh:",
+                 "10-sysinv-adjust-partitions.py:",
+                 "60-helm-releases-data-migration.py:",
+                 "55-armada-helm-upgrade.py:",
+                 "95-apply-mandatory-psp-policies.py:",
+                 "10-sysinv-adjust-partitions.py:",
+                 "85-update-sc-admin-endpoint-cert.py:",
+                 "70-active-secured-etcd-after-upgrade.sh:",
+                 "50-dcmanager-subcloud-status-migration.py:",
+                 "45-sysinv-remove-identity-shared-service.py:",
+                 "25-coredns-configmap.sh:",
+                 "20-exempt-admin-from-lockout.sh:",
+                 "115-foo-bar-test-ok.sh:", "299-foo-bar-test-ok.sh:",
+                 "2123-foo-bar-test-ok.sh"]
+
+invalidScripts1 = ["70-bla1-bla2-bla3.sh", "7-bla1-bla2-bla3.py:",
+                   "20-bla1-bla2-bla3.sh:", "-20-bla1-bla2-bla3.sh"]
+
+invalidScripts2 = ["95-apply-mandatory-psp-policies.py",
+                   "10-sysinv-adjust-partitions.py:",
+                   "85-update-sc-admin-endpoint-cert.py:",
+                   "70_active-secured-etcd-after-upgrade.sh:"]
+
+
+# Append scripts to be executed according to the passed list
+def addScripts(self, scripts, output_filename):
+    for script in scripts:
+        self.scripts.append(script, returncode=0, script=script_body %
+                            (output_filename, script))
+
+
+# Test with the files under "controllerconfig/upgrade-scripts"
+def addRealMigrationScripts(self, output_filename):
+    path = os.getcwd() + "/upgrade-scripts"
+    for f in listdir(path):
+        if isfile(join(path, f)):
+            self.scripts.append(f, returncode=0, script=script_body %
+                                (output_filename, f))
+
+
+def assertProperSorted(scripts):
+    output = False
+    sequence = []
+    for script in scripts:
+        sequence.append(int(script.split("-")[0]))
+    if sorted(sequence) == sequence:
+        output = True
+    return output
+
+
+class TestMigrationScripts(unittest.TestCase):
+
+    def setUp(self):
+        self.scripts_dir = mkdtemp()
+        self.output_filename = mkdtemp() + "/output.txt"
+        # Re-create the file for each run
+        open(self.output_filename, 'w+').close()
+        self.scripts = mockprocess.MockProc(self.scripts_dir)
+
+    def test_migration_scripts_success_1(self):
+        addScripts(self, validScripts1, self.output_filename)
+        with self.scripts:
+            utils.execute_migration_scripts(from_release, to_release, action,
+                                            self.scripts_dir)
+        with open(self.output_filename, 'r') as f:
+            output = str(f.read())
+        if(assertProperSorted(output.split(':'))):
+            pass
+
+    def test_migration_scripts_success_2(self):
+        addScripts(self, validScripts2, self.output_filename)
+        with self.scripts:
+            utils.execute_migration_scripts(from_release, to_release, action,
+                                            self.scripts_dir)
+        with open(self.output_filename, 'r') as f:
+            output = str(f.read())
+        if(assertProperSorted(output.split(':'))):
+            pass
+
+    def test_real_migration_scripts(self):
+        addRealMigrationScripts(self, self.output_filename)
+        with self.scripts:
+            utils.execute_migration_scripts(from_release, to_release, action,
+                                            self.scripts_dir)
+        with open(self.output_filename, 'r') as f:
+            output = str(f.read())
+        if(assertProperSorted(output.split(':'))):
+            pass
+
+    def test_migration_scripts_validation_fail_1(self):
+        addScripts(self, invalidScripts1, self.output_filename)
+        with self.assertRaises(ValueError):
+            with self.scripts:
+                utils.execute_migration_scripts(from_release, to_release,
+                                                action, self.scripts_dir)
+
+    def test_migration_scripts_validation_fail_2(self):
+        addScripts(self, invalidScripts2, self.output_filename)
+        with self.assertRaises(ValueError):
+            with self.scripts:
+                utils.execute_migration_scripts(from_release, to_release,
+                                                action, self.scripts_dir)
+
+    def tearDown(self):
+        os.remove(self.output_filename)

controllerconfig/controllerconfig/controllerconfig/upgrades/utils.py

@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016-2020 Wind River Systems, Inc.
+# Copyright (c) 2016-2021 Wind River Systems, Inc.
 #
 # SPDX-License-Identifier: Apache-2.0
 #
@@ -50,7 +50,8 @@ ACTION_MIGRATE = "migrate"
 ACTION_ACTIVATE = "activate"
 
 
-def execute_migration_scripts(from_release, to_release, action):
+def execute_migration_scripts(from_release, to_release, action,
+                              migration_script_dir="/etc/upgrade.d"):
     """ Execute migration scripts with an action:
           start: Prepare for upgrade on release N side. Called during
                  "system upgrade-start".
@@ -60,8 +61,6 @@ def execute_migration_scripts(from_release, to_release, action):
 
     devnull = open(os.devnull, 'w')
 
-    migration_script_dir = "/etc/upgrade.d"
-
     LOG.info("Executing migration scripts with from_release: %s, "
              "to_release: %s, action: %s" % (from_release, to_release, action))
 
@@ -70,7 +69,16 @@ def execute_migration_scripts(from_release, to_release, action):
     files = [f for f in os.listdir(migration_script_dir)
              if os.path.isfile(os.path.join(migration_script_dir, f)) and
              os.access(os.path.join(migration_script_dir, f), os.X_OK)]
-    files.sort()
+    # From file name, get the number to sort the calling sequence,
+    # abort when the file name format does not follow the pattern
+    # "nnn-*.*", where "nnn" string shall contain only digits, corresponding
+    # to a valid unsigned integer (first sequence of characters before "-")
+    try:
+        files.sort(key=lambda x: int(x.split("-")[0]))
+    except Exception:
+        LOG.exception("Migration script sequence validation failed, invalid "
+                      "file name format")
+        raise
 
     # Execute each migration script
     for f in files:

controllerconfig/controllerconfig/test-requirements.txt

@@ -1,9 +1,10 @@
 hacking!=0.13.0,<0.14,>=0.12.0 # Apache-2.0
 pylint <=1.9.3;python_version<'3.0'
 pytest
-mock
+mockproc>= 0.3.1 # BSD
 coverage>=3.6
 PyYAML>=3.10.0 # MIT
 os-testr>=0.8.0 # Apache-2.0
+stestr>=1.0.0 # Apache-2.0
 testresources>=0.2.4 # Apache-2.0/BSD
 testrepository>=0.0.18 # Apache-2.0/BSD

controllerconfig/controllerconfig/tox.ini

@@ -4,7 +4,7 @@
 # and then run "tox" from this directory.
 
 [tox]
-envlist = flake8, pylint
+envlist = flake8, pylint, py27, py36
 # Tox does not work if the path to the workdir is too long, so move it to /tmp
 toxworkdir = /tmp/{env:USER}_cctox
 stxdir = {toxinidir}/../../..
@@ -21,7 +21,9 @@ deps = -r{toxinidir}/requirements.txt
        -e{[tox]stxdir}/fault/fm-api
        -e{[tox]stxdir}/config/tsconfig/tsconfig
        -e{[tox]stxdir}/config/sysinv/sysinv/sysinv
-       -e{[tox]stxdir}/config/sysinv/cgts-client/cgts-client
+
+commands =
+  find . -type f -name "*.pyc" -delete
 
 [testenv:venv]
 commands = {posargs}
@@ -36,6 +38,22 @@ basepython = python2.7
 deps = -r{toxinidir}/test-requirements.txt
 commands = flake8 {posargs}
 
+[testenv:py27]
+basepython = python2.7
+deps = {[testenv]deps}
+commands =
+  {[testenv]commands}
+  stestr run {posargs}
+  stestr slowest
+
+[testenv:py36]
+basepython = python3.6
+deps = {[testenv]deps}
+commands =
+  {[testenv]commands}
+  stestr run {posargs}
+  stestr slowest
+
 [flake8]
 # H series are hacking
 # H101: Use TODO(NAME)