From f1126dc58a33fa62518966e54c7dc636ad6eaf92 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 20 Mar 2021 14:59:26 -0500 Subject: [PATCH 01/12] improve CI output - remove git clone progress reporting (reduce CI noise) - report current user --- tools/launchdev.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/launchdev.sh b/tools/launchdev.sh index 9e6fc9bc46..3d4a97b26e 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -55,6 +55,7 @@ function init(){ CURRENT_DIR=`pwd` CURRENT_USER=`whoami` CURRENT_USER_GROUP=`id -gn` + echo "Current user:group = ${CURRENT_USER}:${CURRENT_USER_GROUP}" if [[ (${COMMAND_PATH} == /*) ]] ; then @@ -153,7 +154,9 @@ function st2start(){ cp -Rp ./contrib/examples $PACKS_BASE_DIR # Clone st2tests in /tmp directory. pushd /tmp - git clone https://github.com/StackStorm/st2tests.git + echo Cloning https://github.com/StackStorm/st2tests.git + # -q = no progress reporting (better for CI). Errors will still print. + git clone -q https://github.com/StackStorm/st2tests.git ret=$? if [ ${ret} -eq 0 ]; then cp -Rp ./st2tests/packs/fixtures $PACKS_BASE_DIR From 3046d0524e104de90b99030e2fbf8c823513f598 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 20 Mar 2021 22:18:39 -0500 Subject: [PATCH 02/12] improve GHA folding --- scripts/travis/prepare-integration.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/travis/prepare-integration.sh b/scripts/travis/prepare-integration.sh index dc6efa83a4..c46c71f813 100755 --- a/scripts/travis/prepare-integration.sh +++ b/scripts/travis/prepare-integration.sh @@ -16,6 +16,9 @@ st2 --version # Clean up old screen log files rm -f logs/screen-*.log +# ::group::/::endgroup:: is helpful github actions syntax to fold this section. +echo ::group::launchdev.sh start -x + # start dev environment in screens ./tools/launchdev.sh start -x @@ -28,6 +31,9 @@ echo " === START: Catting screen process log files. ===" cat logs/screen-*.log echo " === END: Catting screen process log files. ===" +# github actions: fold for launchdev.sh start -x +echo ::endgroup:: + # Setup the virtualenv for the examples pack which is required for orquesta integration tests. st2 run packs.setup_virtualenv packs=examples From fbd09dd15d5b117d398541ac4f6e9b2174661a1d Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 20 Mar 2021 23:18:25 -0500 Subject: [PATCH 03/12] fix virtualenv lock files issue again --- scripts/travis/prepare-integration.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/travis/prepare-integration.sh b/scripts/travis/prepare-integration.sh index c46c71f813..ba61659190 100755 --- a/scripts/travis/prepare-integration.sh +++ b/scripts/travis/prepare-integration.sh @@ -45,3 +45,6 @@ chmod 777 logs/* # root needs to access write some lock files when creating virtualenvs # o=other; X=only set execute bit if user execute bit is set (eg on dirs) chmod -R o+rwX ./virtualenv/ +# newer virtualenv versions are putting lock files under ~/.local +# as this script runs with sudo, HOME is actually the CI user's home +chmod -R o+rwX ${HOME}/.local/share/virtualenv From 204723c28003e0720bc8ba7a80e449d3f7bfc819 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 20 Mar 2021 19:16:14 -0500 Subject: [PATCH 04/12] Switch tests to check w/ CA that is present in GHA --- st2common/tests/integration/test_rabbitmq_ssl_listener.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/st2common/tests/integration/test_rabbitmq_ssl_listener.py b/st2common/tests/integration/test_rabbitmq_ssl_listener.py index e64a22995d..224a50d4a4 100644 --- a/st2common/tests/integration/test_rabbitmq_ssl_listener.py +++ b/st2common/tests/integration/test_rabbitmq_ssl_listener.py @@ -130,7 +130,7 @@ def test_ssl_connection_ca_certs_provided(self): # 2. Validate server cert against other CA bundle (failure) # CA bundle which was not used to sign the server cert - ca_cert_path = os.path.join("/etc/ssl/certs/thawte_Primary_Root_CA.pem") + ca_cert_path = os.path.join("/etc/ssl/certs/SecureTrust_CA.pem") cfg.CONF.set_override( name="ssl_cert_reqs", override="required", group="messaging" @@ -147,7 +147,7 @@ def test_ssl_connection_ca_certs_provided(self): self.assertRaisesRegexp(ssl.SSLError, expected_msg, connection.connect) # 3. Validate server cert against other CA bundle (failure) - ca_cert_path = os.path.join("/etc/ssl/certs/thawte_Primary_Root_CA.pem") + ca_cert_path = os.path.join("/etc/ssl/certs/SecureTrust_CA.pem") cfg.CONF.set_override( name="ssl_cert_reqs", override="optional", group="messaging" @@ -165,7 +165,7 @@ def test_ssl_connection_ca_certs_provided(self): # 4. Validate server cert against other CA bundle (failure) # We use invalid bundle but cert_reqs is none - ca_cert_path = os.path.join("/etc/ssl/certs/thawte_Primary_Root_CA.pem") + ca_cert_path = os.path.join("/etc/ssl/certs/SecureTrust_CA.pem") cfg.CONF.set_override(name="ssl_cert_reqs", override="none", group="messaging") cfg.CONF.set_override( From 349be7ae80837ce703e02fc00579c15265e9e08b Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 20 Mar 2021 22:52:42 -0500 Subject: [PATCH 05/12] apt download cache --- .github/workflows/ci.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 22e9c215eb..4d638cf231 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -114,6 +114,12 @@ jobs: uses: actions/setup-python@v2 with: python-version: '${{ matrix.python-version }}' + - name: Get date components for use in cache-keys + id: date + run: | + echo "::set-output name=year::$(/bin/date -u "+%Y")" + echo "::set-output name=month::$(/bin/date -u "+%m")" + echo "::set-output name=week::$(/bin/date -u "+%U")" - uses: actions/cache@v2 with: path: | @@ -122,6 +128,16 @@ jobs: key: ${{ runner.os }}-${{ matrix.python-version }}-${{ hashFiles('requirements.txt', 'test-requirements.txt') }} restore-keys: | ${{ runner.os }}-${{ matrix.python }}- + - uses: actions/cache@v2 + with: + path: | + /var/cache/apt/archives/*.deb + /var/cache/apt/archives/partial/*.deb + /var/cache/apt/*.bin + key: ${{ runner.os }}-apt-${{ steps.date.outputs.year }}-${{ steps.date.outputs.week }} + restore-keys: | + ${{ runner.os }}-apt-${{ steps.date.outputs.year }}- + ${{ runner.os }}-apt- - name: Install apt depedencies run: | # install dev dependencies for Python YAML and LDAP packages From e290349191f97530491d6582b60f34763b36fc0e Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 19 Mar 2021 23:54:34 -0500 Subject: [PATCH 06/12] use bitnami/rabbitmq service container in GHA - no random ports - use separate rabbitmq.conf file on github - ci: temporarily disable all but ci-integration - ci: cp ssl_certs into rabbitmq container - reenable management plugin - manual custom.conf - add python-version for ci-integration tests - add default rabbitmq user/pass - do not hardcode RMQ user/pass in tests - override more rabbitmq test urls - fix integration test conf files on the fly - go back to guest:guest with rabbitmq - finish reverting rmq guest:guest stuff - Reconfigure RabbitMQ for ci-unit tests as well --- .github/workflows/ci.yaml | 91 +++++++++++++++++++++++------------- scripts/github/rabbitmq.conf | 11 +++++ 2 files changed, 69 insertions(+), 33 deletions(-) create mode 100644 scripts/github/rabbitmq.conf diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 4d638cf231..7ab51894b3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -44,28 +44,45 @@ jobs: # - name: 'Micro Benchmarks' # task: 'micro-benchmarks' # python-version: '3.6' - # Integration tests are not working yet, still done in Travis - # - name: 'Integration Tests' - # task: 'ci-integration' + - name: 'Integration Tests' + task: 'ci-integration' + python-version: '3.6' services: mongo: image: mongo:4.0 ports: - 27017:27017 - # Can't use RabbitMQ here for Integrations because we rely on custom config - # and SSL certs that are in the repo. In GHA, these services are started first - # before the code is checked out, so this is a non-starter, we need to do it - # manually below (TODO) + + # In GHA, these services are started first before the code is checked out. + # We use bitnami images to facilitate reconfiguring RabbitMQ during ci-integration tests. + # We rely on custom config and SSL certs that are in the repo. + # Many images require config in env vars (which we can't change during the test job) + # or they require config in entrypoint args (which we can't override for GHA services) + # bitnami builds ways to get config files from mounted volumes. rabbitmq: - # use the -management version so it has the management tools installed - image: rabbitmq:3.8-management + image: bitnami/rabbitmq:3.8 + volumes: + - /home/runner/rabbitmq_conf:/bitnami/conf # RABBITMQ_MOUNTED_CONF_DIR + env: + # tell bitnami/rabbitmq to enable this by default + RABBITMQ_PLUGINS: rabbitmq_management + RABBITMQ_USERNAME: guest + RABBITMQ_PASSWORD: guest + + # These are strictly docker options, not entrypoint args (GHA restriction) + options: >- + --name rabbitmq + ports: - # SSL port - - 5671:5671 - # standard port - - 5672:5672 - # management port - - 15672:15672 + # These 6 ports are exposed by bitnami/rabbitmq (see https://www.rabbitmq.com/networking.html#ports) + # host_port:container_port/protocol + - 5671:5671/tcp # AMQP SSL port + - 5672:5672/tcp # AMQP standard port + - 15672:15672/tcp # Management: HTTP, CLI + #- 15671:15671/tcp # Management: SSL port + #- 25672:25672/tcp # inter-node or CLI + #- 4369:4369/tcp # epmd + env: TASK: '${{ matrix.task }}' @@ -165,28 +182,36 @@ jobs: run: | echo "$ST2_CI_REPO_PATH" sudo ST2_CI_REPO_PATH="${ST2_CI_REPO_PATH}" scripts/travis/permissions-workaround.sh - - name: Setup RabbitMQ (NOT WORKING YET) - if: "${{ env.TASK == 'ci-integration' }}" + - name: Reconfigure RabbitMQ + if: "${{ env.TASK == 'ci-unit' || env.TASK == 'ci-integration' }}" + # bitnami image allows (see bitnami/rabbitmq readme): + # Here we're copying a rabbitmq.config file which won't do anything. + # We need to switch to custom.conf or advanced.config. + timeout-minutes: 2 # may die if rabbitmq fails to start run: | + set -x # Use custom RabbitMQ config which enables SSL / TLS listener on port 5671 with test certs - # Travis runs as the 'travis' user, GitHub actions run as the 'runner' user, - # And the cert filepaths are slightly different between the two. - # Example: - # Travis-CI: /home/travis/build/StackStorm/st2/st2tests/st2tests/fixtures/ssl_certs/ca/ca_certificate_bundle.pem - # GitHub Actions: /home/runner/work/st2/st2/st2tests/st2tests/fixtures/ssl_certs/ca/ca_certificate_bundle.pem - sed -i 's|/home/travis/build/StackStorm|/home/runner/work/st2|g' scripts/travis/rabbitmq.config - # Now that we've manged the config file, install it - sudo cp scripts/travis/rabbitmq.config /etc/rabbitmq/rabbitmq.config - # Install rabbitmq_management RabbitMQ plugin - sudo service rabbitmq-server restart - sleep 5 - sudo rabbitmq-plugins enable rabbitmq_management + sudo cp scripts/github/rabbitmq.conf /home/runner/rabbitmq_conf/custom.conf + # The code is checked out after the container is already up, so we don't mount them. + # We copy those certs into the dir that is mounted to /bitnami/conf + sudo cp -r st2tests/st2tests/fixtures/ssl_certs /home/runner/rabbitmq_conf/ + # refresh rabbitmq config - based on ENTRYPOINT logic + docker exec rabbitmq bash -c 'cat /bitnami/conf/custom.conf >> /opt/bitnami/rabbitmq/etc/rabbitmq/rabbitmq.conf' + # sleep to prevent interleaved output in GHA logs + docker exec rabbitmq cat /opt/bitnami/rabbitmq/etc/rabbitmq/rabbitmq.conf && sleep 0.1 + echo + echo restarting rabbitmq container + docker restart rabbitmq + # wait for rabbitmq container to restart + until [ "$(docker inspect -f {{.State.Running}} rabbitmq)" == "true" ]; do sleep 0.1; done + echo enabled RabbitMQ plugins: + # print plugins list to: (1) ease debugging, (2) pause till rabbitmq is really running + docker exec rabbitmq rabbitmq-plugins list -e + echo sudo wget http://guest:guest@localhost:15672/cli/rabbitmqadmin -O /usr/local/bin/rabbitmqadmin sudo chmod +x /usr/local/bin/rabbitmqadmin - sudo service rabbitmq-server restart - # chmod to make glob work (*.log to avoid log dir) - sudo chmod a+rx /var/log/rabbitmq - sudo tail -n 30 /var/log/rabbitmq/*.log + # print logs from stdout (RABBITMQ_LOGS=-) + docker logs --tail=20 rabbitmq - name: Print versions run: | # Print various binary versions diff --git a/scripts/github/rabbitmq.conf b/scripts/github/rabbitmq.conf new file mode 100644 index 0000000000..1c8032292b --- /dev/null +++ b/scripts/github/rabbitmq.conf @@ -0,0 +1,11 @@ +# bitnami/rabbitmq configuration file (gets merged with rabbitmq.conf) +listeners.ssl.default = 5671 +# /bitnami/conf is a directory mounted into the bitnami/rabbitmq container +ssl_options.cacertfile = /bitnami/conf/ssl_certs/ca/ca_certificate_bundle.pem +ssl_options.certfile = /bitnami/conf/ssl_certs/server/server_certificate.pem +ssl_options.keyfile = /bitnami/conf/ssl_certs/server/private_key.pem +ssl_options.verify = verify_peer +ssl_options.fail_if_no_peer_cert = false + +# this is "insecure" but it doesn't matter for CI, and it simplifies integration test machinery +loopback_users = none From 783335e57971a63fee594130236948e7bb9b08ce Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 20 Mar 2021 23:31:28 -0500 Subject: [PATCH 07/12] bust python cache --- .github/workflows/ci.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7ab51894b3..cfa42e3bf6 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -142,9 +142,12 @@ jobs: path: | ~/.cache/pip virtualenv - key: ${{ runner.os }}-${{ matrix.python-version }}-${{ hashFiles('requirements.txt', 'test-requirements.txt') }} + # TODO: maybe make the virtualenv a partial cache to exclude st2*? + # !virtualenv/lib/python*/site-packages/st2* + # !virtualenv/bin/st2* + key: ${{ runner.os }}-python-${{ matrix.python-version }}-${{ hashFiles('requirements.txt', 'test-requirements.txt') }} restore-keys: | - ${{ runner.os }}-${{ matrix.python }}- + ${{ runner.os }}-python-${{ matrix.python }}- - uses: actions/cache@v2 with: path: | From 312efb4e0ffa65ab65fec16cae9f23bccad479cd Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sun, 21 Mar 2021 13:16:29 -0500 Subject: [PATCH 08/12] Use virtualenv from st2 virtualenv by default If a virtualenv was built with symlinks instead of copies of the python binary, as happens by default on GHA integration tests, then we end up using the system virtualenv binary instead of what we installed. Currently GHA has the same version of virtualenv, but we build virtualenvs with --system-site-packages, and we need it to use the st2 virtualenv as the "system" dir instead of the GHA provided one. So, don't get the realpath of the python binary. Also, fall back to system virtualenv bin for backwards compat. --- st2common/st2common/config.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index e7b30a9a7c..d97cf08546 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -393,8 +393,13 @@ def register_opts(ignore_errors=False): # Runner options default_python_bin_path = sys.executable - base_dir = os.path.dirname(os.path.realpath(default_python_bin_path)) + # If the virtualenv uses a symlinked python, then try using virtualenv from that venv + # first before looking for virtualenv installed in python's system-site-packages. + base_dir = os.path.dirname(default_python_bin_path) default_virtualenv_bin_path = os.path.join(base_dir, "virtualenv") + if not os.path.exists(default_virtualenv_bin_path): + base_dir = os.path.dirname(os.path.realpath(default_python_bin_path)) + default_virtualenv_bin_path = os.path.join(base_dir, "virtualenv") action_runner_opts = [ # Common runner options From f87e999c13157f61a8bcd15c5e16d9d007072c6c Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sun, 21 Mar 2021 09:19:11 -0500 Subject: [PATCH 09/12] Installing virtualenv in --user on github - install virtualenv in --user - pip uninstall --user - Try to recreate failure on Travis - leave TODO/FIXME around broken test --- .github/workflows/ci.yaml | 16 +++++++++++++++- .travis.yml | 6 ++++++ .../integration/test_pythonrunner_behavior.py | 15 +++++++++++++++ st2common/st2common/util/virtualenvs.py | 8 ++++++++ 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index cfa42e3bf6..5d3d1e3c66 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -142,6 +142,7 @@ jobs: path: | ~/.cache/pip virtualenv + ~/virtualenv # TODO: maybe make the virtualenv a partial cache to exclude st2*? # !virtualenv/lib/python*/site-packages/st2* # !virtualenv/bin/st2* @@ -168,7 +169,19 @@ jobs: run: | # Note: Use the verison of virtualenv pinned in fixed-requirements.txt so we # only have to update it one place when we change the version - pip install --upgrade --force-reinstall $(grep "^virtualenv" fixed-requirements.txt) + # Note: Use --user to avoid polluting system site-packages (which breaks one of our tests) + # TODO: simplify this once fixed in contrib/runners/python_runner/tests/integration/test_pythonrunner_behavior.py + if [[ ! -f ~/virtualenv/bin/virtualenv ]]; then # use the cached version whenever possible + pip install --user --upgrade --force-reinstall $(grep "^virtualenv" fixed-requirements.txt) + virtualenv --no-download ~/virtualenv + ~/virtualenv/bin/pip install --upgrade --force-reinstall $(grep "^virtualenv" fixed-requirements.txt) + # drop the --user install virtualenv to prevent polluting tests + pip freeze --user | xargs pip uninstall -y + mkdir -p ~/.local/bin + ln -s ~/virtualenv/bin/virtualenv ~/.local/bin/virtualenv + fi + which virtualenv + virtualenv --version - name: Install requirements run: | ./scripts/travis/install-requirements.sh @@ -221,6 +234,7 @@ jobs: git --version pip --version pip list + virtualenv --version # Print out various environment variables info make play - name: make diff --git a/.travis.yml b/.travis.yml index ebb2e9bc94..63c0b7d991 100644 --- a/.travis.yml +++ b/.travis.yml @@ -85,6 +85,12 @@ cache: #- .tox/ install: + # This triggers the same behavior on travis as we found on github because it installs six in the system-site-packages + #- /opt/python/3.6.7/bin/pip install --upgrade --force-reinstall $(grep "^virtualenv" fixed-requirements.txt) + #- mkdir -p ~/.local/bin + #- ln -s /opt/python/3.6.7/bin/virtualenv ~/.local/bin/virtualenv + - which virtualenv + - virtualenv --version - ./scripts/travis/install-requirements.sh # prep a travis-specific dev conf file that uses travis instead of stanley - cp conf/st2.dev.conf "${ST2_CONF}" ; sed -i -e "s/stanley/${ST2_CI_USER}/" "${ST2_CONF}" diff --git a/contrib/runners/python_runner/tests/integration/test_pythonrunner_behavior.py b/contrib/runners/python_runner/tests/integration/test_pythonrunner_behavior.py index a6d300be23..a694ffceac 100644 --- a/contrib/runners/python_runner/tests/integration/test_pythonrunner_behavior.py +++ b/contrib/runners/python_runner/tests/integration/test_pythonrunner_behavior.py @@ -40,6 +40,12 @@ class PythonRunnerBehaviorTestCase(CleanFilesTestCase, CleanDbTestCase): + + # If you need these logs, then you probably also want to uncomment + # extra debug log messages in st2common/st2common/util/virtualenvs.py + # and pass --logging-level=DEBUG to nosetests + # DISPLAY_LOG_MESSAGES = True + def setUp(self): super(PythonRunnerBehaviorTestCase, self).setUp() config.parse_args() @@ -74,6 +80,15 @@ def test_priority_of_loading_library_after_setup_pack_virtualenv(self): (_, output, _) = self._run_action( pack_name, "get_library_path.py", {"module": "six"} ) + # FIXME: This test fails if system site-packages has six because + # it won't get installed in the virtualenv (w/ --system-site-packages) + # system site-packages is never from a virtualenv. + # Travis has python installed in /opt/python/3.6.7 + # with a no-system-site-packages virtualenv at /home/travis/virtualenv/python3.6.7 + # GitHub Actions python is in /opt/hostedtoolcache/Python/3.6.13/x64/ + # But ther isn't a virtualenv, so when we pip installed `virtualenv`, + # (which depends on, and therefore installs `six`) + # we installed it in system-site-packages not an intermediate virtualenv self.assertEqual(output["result"].find(self.virtualenvs_path), 0) # Conversely, this expects that 'mock' module file-path is not under sandbox library, diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index 62cfc99b52..ad64f1e3af 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -284,6 +284,10 @@ def install_requirements( ) exit_code, stdout, stderr = run_command(cmd=cmd, env=env) + # Normally we don't want this, even in debug logs. But it is useful to + # investigate pip behavior changes & broken virtualenv integration tests. + # logger.debug(f"\npip stdout=\n{stdout}") + if exit_code != 0: stdout = to_ascii(stdout) stderr = to_ascii(stderr) @@ -330,6 +334,10 @@ def install_requirement(virtualenv_path, requirement, proxy_config=None, logger= ) exit_code, stdout, stderr = run_command(cmd=cmd, env=env) + # Normally we don't want this, even in debug logs. But it is useful to + # investigate pip behavior changes & broken virtualenv integration tests. + # logger.debug(f"\npip stdout=\n{stdout}") + if exit_code != 0: raise Exception( 'Failed to install requirement "%s": %s' % (requirement, stdout) From ac6fb7957ebc06452a7ca39d754a49a340ef48e0 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 22 Mar 2021 23:11:53 +0100 Subject: [PATCH 10/12] Add workaround for failing stdin test - Use close_fds=True. - add workaround for failing test --- .../python_runner/python_action_wrapper.py | 6 ++++++ .../test_python_action_process_wrapper.py | 13 ++++++++++--- st2common/st2common/util/shell.py | 9 +++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/contrib/runners/python_runner/python_runner/python_action_wrapper.py b/contrib/runners/python_runner/python_runner/python_action_wrapper.py index 06158496be..795b0114a8 100644 --- a/contrib/runners/python_runner/python_runner/python_action_wrapper.py +++ b/contrib/runners/python_runner/python_runner/python_action_wrapper.py @@ -359,6 +359,12 @@ def _get_action_instance(self): stdin_data = sys.stdin.readline().strip() + if not stdin_data: + # This could indicate that parent process (e.g. process which runs the tests has + # incorrectly opened the stdin and that one is then inherited by the process which is + # spawning it which will cause issues) + raise ValueError("Received no valid parameters data from sys.stdin") + try: stdin_parameters = orjson.loads(stdin_data) stdin_parameters = stdin_parameters.get("parameters", {}) diff --git a/contrib/runners/python_runner/tests/integration/test_python_action_process_wrapper.py b/contrib/runners/python_runner/tests/integration/test_python_action_process_wrapper.py index e1d39361a2..c97a380fb2 100644 --- a/contrib/runners/python_runner/tests/integration/test_python_action_process_wrapper.py +++ b/contrib/runners/python_runner/tests/integration/test_python_action_process_wrapper.py @@ -142,14 +142,21 @@ def test_stdin_params_timeout_no_stdin_data_provided(self): "python %s --pack=dummy --file-path=%s --config='%s' " "--stdin-parameters" % (WRAPPER_SCRIPT_PATH, file_path, config) ) - exit_code, stdout, stderr = run_command(command_string, shell=True) + exit_code, stdout, stderr = run_command( + command_string, shell=True, close_fds=True + ) - expected_msg = ( + # Depending on how tests are spawned, sys.stdin may be opened and this will cause issues + # with this tests so we simply check for two different errors which are considered + # acceptable. + expected_msg_1 = ( "ValueError: No input received and timed out while waiting for parameters " "from stdin" ) + expected_msg_2 = "ValueError: Received no valid parameters data from sys.stdin" + self.assertEqual(exit_code, 1) - self.assertIn(expected_msg, stderr) + self.assertTrue(expected_msg_1 in stderr or expected_msg_2 in stderr) def test_stdin_params_invalid_format_friendly_error(self): config = {} diff --git a/st2common/st2common/util/shell.py b/st2common/st2common/util/shell.py index 113f7ae162..171085be74 100644 --- a/st2common/st2common/util/shell.py +++ b/st2common/st2common/util/shell.py @@ -47,6 +47,7 @@ def run_command( shell=False, cwd=None, env=None, + close_fds=None, ): """ Run the provided command in a subprocess and wait until it completes. @@ -73,6 +74,9 @@ def run_command( environment from the current process is inherited. :type env: ``dict`` + :param close_fds: True to close all the fds. By default when None is provided we rely on + default upstream behavior which may be Python version specific. + :rtype: ``tuple`` (exit_code, stdout, stderr) """ if not isinstance(cmd, (list, tuple) + six.string_types): @@ -83,6 +87,10 @@ def run_command( if not env: env = os.environ.copy() + kwargs = {} + if close_fds is not None: + kwargs["close_fds"] = close_fds + process = concurrency.subprocess_popen( args=cmd, stdin=stdin, @@ -91,6 +99,7 @@ def run_command( env=env, cwd=cwd, shell=shell, + **kwargs, ) stdout, stderr = process.communicate() exit_code = process.returncode From b2b7917eb1e06645ca97a244a73aaed29b118eb5 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 22 Mar 2021 22:34:49 -0500 Subject: [PATCH 11/12] fix check-dependency-conflicts by updating pip-tools When we updated pip, we missed updating pip-tools to support the newer pip version, so there were errors (that didn't fail the build, oops) like these (fixed in pip-tools 5.4, pip-tools-PR-1216: virtualenv/bin/pip-compile req.txt Traceback (most recent call last): File "virtualenv/bin/pip-compile", line 8, in sys.exit(cli()) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/click/core.py", line 829, in __call__ return self.main(*args, **kwargs) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/click/core.py", line 782, in main rv = self.invoke(ctx) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/click/core.py", line 1066, in invoke return ctx.invoke(self.callback, **ctx.params) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/click/core.py", line 610, in invoke return callback(*args, **kwargs) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/click/decorators.py", line 21, in new_func return f(get_current_context(), *args, **kwargs) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/piptools/scripts/compile.py", line 458, in cli results = resolver.resolve(max_rounds=max_rounds) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/piptools/resolver.py", line 173, in resolve has_changed, best_matches = self._resolve_one_round() File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/piptools/resolver.py", line 278, in _resolve_one_round their_constraints.extend(self._iter_dependencies(best_match)) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/piptools/resolver.py", line 388, in _iter_dependencies dependencies = self.repository.get_dependencies(ireq) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/piptools/repositories/local.py", line 75, in get_dependencies return self.repository.get_dependencies(ireq) File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/piptools/repositories/pypi.py", line 232, in get_dependencies download_dir, ireq, wheel_cache File "/home/runner/work/st2/st2/virtualenv/lib/python3.6/site-packages/piptools/repositories/pypi.py", line 163, in resolve_reqs wheel_download_dir=self._wheel_download_dir, TypeError: make_requirement_preparer() got an unexpected keyword argument 'wheel_download_dir' --- Makefile | 2 +- test-requirements.txt | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 35d88c479e..14a13ea566 100644 --- a/Makefile +++ b/Makefile @@ -674,7 +674,7 @@ check-dependency-conflicts: @echo # Verify there are no conflicting dependencies cat st2*/requirements.txt contrib/runners/*/requirements.txt | sort -u > req.txt && \ - $(VIRTUALENV_DIR)/bin/pip-compile req.txt; \ + $(VIRTUALENV_DIR)/bin/pip-compile req.txt || exit 1; \ if [[ -e req.txt ]]; then rm req.txt; fi .PHONY: virtualenv diff --git a/test-requirements.txt b/test-requirements.txt index 23738101bc..1c45f904b0 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -31,8 +31,10 @@ rstcheck>=3.3.1,<3.4 tox==3.14.1 pyrabbit # pip-tools provides pip-compile: to check for version conflicts -# pip-tools 5.4 needs pip>=20.1, but we use 20.0.2 -pip-tools<5.4 +# pip-tools 5.3 needs pip<20.3 +# pip-tools 5.4 needs pip>=20.1 +# pip-tools 6.0 needs pip>=20.3 +pip-tools>=5.4,<6.1 pytest==5.4.3 pytest-benchmark==3.2.3 # zstandard is used for micro benchmarks From 3193858465e87f8cb835b9b9e16e01f26e2d7b9d Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Tue, 23 Mar 2021 01:56:42 -0500 Subject: [PATCH 12/12] Handle restored ~/virtualenv cache correctly We still need the symlink in ~/.local/bin after restoring the cached ~/virtualenv directory. Also, be more verbose by printing the commands. --- .github/workflows/ci.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5d3d1e3c66..585e6b51d2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -167,6 +167,7 @@ jobs: sudo apt-get -f -y install libldap2-dev libsasl2-dev libssl-dev libyaml-dev ldap-utils - name: Install virtualenv run: | + set -x # Note: Use the verison of virtualenv pinned in fixed-requirements.txt so we # only have to update it one place when we change the version # Note: Use --user to avoid polluting system site-packages (which breaks one of our tests) @@ -177,9 +178,9 @@ jobs: ~/virtualenv/bin/pip install --upgrade --force-reinstall $(grep "^virtualenv" fixed-requirements.txt) # drop the --user install virtualenv to prevent polluting tests pip freeze --user | xargs pip uninstall -y - mkdir -p ~/.local/bin - ln -s ~/virtualenv/bin/virtualenv ~/.local/bin/virtualenv fi + mkdir -p ~/.local/bin + ln -s ~/virtualenv/bin/virtualenv ~/.local/bin/virtualenv which virtualenv virtualenv --version - name: Install requirements