From 4f3c8efd04387e8c8687ad422182411306ff693e Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 10 Apr 2024 09:55:40 -0400 Subject: [PATCH 01/13] Drop Python 3.9 from CI --- .github/workflows/tests.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6293f99..020557f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,7 +10,9 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ['3.8', '3.9', '3.10'] + python-version: + - '3.8' + - '3.10' steps: - name: Check out code uses: actions/checkout@v2 From acbddf25b78170fda381eefc7829c3c68b2de9be Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 10 Apr 2024 09:56:36 -0400 Subject: [PATCH 02/13] Add Python 3.12 to the testing matrix Ubuntu 24.04 carries python 3.12, hence the importance to gate on it. --- .github/workflows/tests.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 020557f..a02cbcc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,8 +11,9 @@ jobs: strategy: matrix: python-version: - - '3.8' - - '3.10' + - '3.8' # focal + - '3.10' # jammy + - '3.12' # noble steps: - name: Check out code uses: actions/checkout@v2 From c6e03f2a7ef6475bc9aa97b9b5f857adb2b3d111 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 10 Apr 2024 10:07:42 -0400 Subject: [PATCH 03/13] Replace inspect.getargspec() with inspect.getfullargspec() inspect.getargspec() is deprecated and finally removed in Python 3.11[0], the replacement is to use inspect.getfullargspec(), see [1] [0] https://github.com/python/cpython/commit/d89fb9a5a610a257014d112bdceef73d7df14082 [1] https://docs.python.org/3.10/library/inspect.html#inspect.getargspec --- charmtools/build/tactics.py | 4 ++-- tests/test_build.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/charmtools/build/tactics.py b/charmtools/build/tactics.py index 853d2f3..b983d1c 100644 --- a/charmtools/build/tactics.py +++ b/charmtools/build/tactics.py @@ -1,4 +1,4 @@ -from inspect import getargspec +from inspect import getfullargspec import errno import json import logging @@ -43,7 +43,7 @@ def get(cls, entity, target, layer, next_config, current_config, given entity. """ for candidate in current_config.tactics + DEFAULT_TACTICS: - argspec = getargspec(candidate.trigger) + argspec = getfullargspec(candidate.trigger) if len(argspec.args) == 2: # old calling convention name = candidate.__name__ diff --git a/tests/test_build.py b/tests/test_build.py index 481f6be..5333a73 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -588,9 +588,9 @@ def test_layer_options(self, log): }, }) - @mock.patch('charmtools.build.tactics.getargspec') + @mock.patch('charmtools.build.tactics.getfullargspec') @mock.patch('charmtools.utils.walk') - def test_custom_tactics(self, mwalk, mgetargspec): + def test_custom_tactics(self, mwalk, mgetfullargspec): def _layer(tactics): return mock.Mock(config=build.builder.BuildConfig({'tactics': tactics}), @@ -617,7 +617,7 @@ def _layer(tactics): ['third', 'second', 'first'], ]) - mgetargspec.return_value = mock.Mock(args=[1, 2, 3, 4]) + mgetfullargspec.return_value = mock.Mock(args=[1, 2, 3, 4]) current_config = mock.Mock(tactics=[ mock.Mock(name='1', **{'trigger.return_value': False}), mock.Mock(name='2', **{'trigger.return_value': False}), From 2fbbe1d73e5e27a4af9b241f222c6ad37050a064 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 10 Apr 2024 10:09:53 -0400 Subject: [PATCH 04/13] Replace assertEquals() with assertEqual() The former has been removed in newer versions of python. --- tests/test_build.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_build.py b/tests/test_build.py index 5333a73..052e4a3 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -179,10 +179,10 @@ def test_tester_layer(self, pv): cyaml = base / "layer.yaml" self.assertTrue(cyaml.exists()) cyaml_data = yaml.safe_load(cyaml.open()) - self.assertEquals(cyaml_data['includes'], ['layers/test-base', + self.assertEqual(cyaml_data['includes'], ['layers/test-base', 'layers/mysql']) - self.assertEquals(cyaml_data['is'], 'foo') - self.assertEquals(cyaml_data['options']['mysql']['qux'], 'one') + self.assertEqual(cyaml_data['is'], 'foo') + self.assertEqual(cyaml_data['options']['mysql']['qux'], 'one') self.assertTrue((base / "hooks/config-changed").exists()) @@ -205,13 +205,13 @@ def test_tester_layer(self, pv): sigs = base / ".build.manifest" self.assertTrue(sigs.exists()) data = json.load(sigs.open()) - self.assertEquals(data['signatures']["README.md"], [ + self.assertEqual(data['signatures']["README.md"], [ u'foo', "static", u'cfac20374288c097975e9f25a0d7c81783acdbc81' '24302ff4a731a4aea10de99']) - self.assertEquals(data["signatures"]['metadata.yaml'], [ + self.assertEqual(data["signatures"]['metadata.yaml'], [ u'foo', "dynamic", u'12c1f6fc865da0660f6dc044cca03b0244e883d9a99fdbdfab6ef6fc2fed63b7' @@ -611,7 +611,7 @@ def _layer(tactics): builder.plan_layers(layers, {}) calls = [call[1]['current_config'].tactics for call in mwalk.call_args_list] - self.assertEquals(calls, [ + self.assertEqual(calls, [ ['first'], ['second', 'first'], ['third', 'second', 'first'], @@ -629,9 +629,9 @@ def _layer(tactics): mock.Mock(), current_config, mock.Mock()) - self.assertEquals([t.trigger.called for t in current_config.tactics], + self.assertEqual([t.trigger.called for t in current_config.tactics], [True, True, True]) - self.assertEquals([t.called for t in current_config.tactics], + self.assertEqual([t.called for t in current_config.tactics], [False, False, True]) From 48100c9799629a68c80f81afcd684b961dad15e4 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 10 Apr 2024 10:33:19 -0400 Subject: [PATCH 05/13] Skip pinning of deps when running python >= 3.12 --- charmtools/build/tactics.py | 3 +++ charmtools/utils.py | 26 ++++++++++++++++++++++++-- tests/test_utils.py | 9 +++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/charmtools/build/tactics.py b/charmtools/build/tactics.py index b983d1c..a93f3ea 100644 --- a/charmtools/build/tactics.py +++ b/charmtools/build/tactics.py @@ -1253,6 +1253,9 @@ def __call__(self): ).exit_on_error()() if self.upgrade_deps: utils.upgrade_venv_core_packages(self._venv, env=self._get_env()) + elif utils.get_python_version(self._venv, + env=self._get_env()) >= utils.PY312: + log.debug('Skip pinning of setuptools, because Python>=3.12') else: utils.pin_setuptools_for_pep440(self._venv, env=self._get_env()) log.debug( diff --git a/charmtools/utils.py b/charmtools/utils.py index 9e9e10a..6ef42a2 100644 --- a/charmtools/utils.py +++ b/charmtools/utils.py @@ -20,6 +20,7 @@ from path import Path as path log = logging.getLogger('utils') +PY312 = (3, 12, 0) @contextmanager @@ -681,8 +682,8 @@ def pin_setuptools_for_pep440(venv_dir, env=None): :type env: Optional[Dict[str,str]] :returns: This function is called for its side effect """ - log.debug('Pinning setuptools < 67 for pep440 non compliant packages "{}"' - .format(venv_dir)) + log.info('Pinning setuptools < 67 for pep440 non compliant packages "{}"' + .format(venv_dir)) Process((os.path.join(venv_dir, 'bin/pip'), 'install', '-U', 'pip<23.1', 'setuptools<67'), env=env).exit_on_error()() @@ -702,3 +703,24 @@ def get_venv_package_list(venv_dir, env=None): if result: return result.output result.exit_on_error() + + +def get_python_version(venv_dir, env=None): + """Get the Python interpreter version in the virtualenv. + + :param venv_dir: Full path to virtualenv in which packages will be listed + :type venv_dir: str + :param env: Environment to use when executing command + :type env: Optional[Dict[str,str]] + :returns: Tuple with major, minor and microversion + :rtype: Tuple[str] + """ + result = Process((os.path.join(venv_dir, 'bin/python3'), '--version'), + env=env)() + m = re.match(r'^Python[ ]+(\d+)\.(\d+)\.(\d+).*', result.output) + if not m: + raise ValueError('Cannot identify the python version: %s' % result) + + return (int(m.group(1)), + int(m.group(2)), + int(m.group(3))) diff --git a/tests/test_utils.py b/tests/test_utils.py index 4339cf9..7fb29a3 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -88,3 +88,12 @@ def test_get_venv_package_list(self, mock_Process, mock_sys_exit): mock_Process().return_value = utils.ProcessResult('fakecmd', 1, '', '') utils.get_venv_package_list('/some/dir', env={'some': 'envvar'}) mock_sys_exit.assert_called_once_with(1) + + @unittest.mock.patch.object(utils, "Process") + def test_get_oython_version(self, process_klass): + process_klass().return_value = utils.ProcessResult( + ['python3', '--version'], 0, b'Python 3.12.4', b'') + self.assertEqual( + utils.get_python_version('/some/dir', env={'some': 'envvar'}), + (3, 12, 4) + ) From ca076383b31a941d0ce9670ee529c29c06bd5e1d Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Mon, 15 Apr 2024 10:53:06 -0400 Subject: [PATCH 06/13] Install package as editable Replace '-e {toxinidir}' with 'package = editable', this allows adding the source code early in the process and avoid packages listed in the deps section to pull in charm-tools from pypi. --- tox.ini | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index 42e2dd4..01d6c82 100644 --- a/tox.ini +++ b/tox.ini @@ -5,7 +5,7 @@ [tox] envlist = py3 -skipsdist = true +package = editable [testenv] install_command = pip install {opts} {packages} @@ -18,7 +18,6 @@ deps = coverage mock responses - -e {toxinidir} https://github.com/openstack-charmers/charm-templates-openstack/archive/master.zip#egg=charm_templates_openstack ipdb From 7c8208890416805371f556e54fb6745a1f3814f8 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Tue, 16 Apr 2024 17:16:16 -0400 Subject: [PATCH 07/13] Drop charm-templates-openstack from deps The package charm-templates-openstack depends on charm-tools, including it in the list of `deps` in tox.ini creates a circular dependency, since the `deps` list is processed before installing the local package. --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index 01d6c82..07ae3e7 100644 --- a/tox.ini +++ b/tox.ini @@ -18,7 +18,6 @@ deps = coverage mock responses - https://github.com/openstack-charmers/charm-templates-openstack/archive/master.zip#egg=charm_templates_openstack ipdb [testenv:lint] From 57b4a3ed1c06370900fc6c8d6b940c1fca82ca0b Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Tue, 16 Apr 2024 17:31:09 -0400 Subject: [PATCH 08/13] setup.py: Drop upper limit in pinning of pip --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index a3d0176..7ba1de1 100755 --- a/setup.py +++ b/setup.py @@ -52,7 +52,7 @@ 'pathspec<0.11;python_version >= "3.7"', 'otherstuf<=1.1.0', 'path.py>=10.5,<13', - 'pip>=1.5.4,<23', + 'pip>=1.5.4', 'jujubundlelib<0.6', 'virtualenv>=1.11.4,<21', 'colander<1.9', From bf5b9dbed8b5fd60f1d9c96647f4b1b542bf36e1 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Tue, 16 Apr 2024 17:46:46 -0400 Subject: [PATCH 09/13] Test with charmcraft 2.x and 3.x --- .github/workflows/tests.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a02cbcc..45ab5ba 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -42,6 +42,11 @@ jobs: path: ${{ steps.snap-build.outputs.snap }} integration: + strategy: + matrix: + charmcraft_channel: + - "2.x/stable" + - "3.x/beta" name: Integration test needs: build runs-on: ubuntu-latest @@ -70,7 +75,7 @@ jobs: - name: Build reactive charm with charmcraft run: | set -euxo pipefail - sudo snap install --classic --channel latest/edge charmcraft + sudo snap install --classic --channel ${{ matrix.charmcraft_channel }} charmcraft cat << EOF | tee tests/charm-minimal/charmcraft.yaml type: charm parts: From 1e76f4c2b1df7f27dbef32284430dddb420081e8 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 17 Apr 2024 10:54:39 -0400 Subject: [PATCH 10/13] Testing --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 45ab5ba..502bd42 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -82,6 +82,7 @@ jobs: charm-tools: plugin: nil override-build: | + ls -lR \$CRAFT_PROJECT_DIR/ snap install --dangerous --classic \$CRAFT_PROJECT_DIR/parts/charm/src/charm-snap/*.snap rm -rf \$CRAFT_PROJECT_DIR/parts/charm/src/charm-snap charm: From 7ecdffc33dd8233c4f42a69b765caa420a56d944 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 17 Apr 2024 12:51:12 -0400 Subject: [PATCH 11/13] Fix path to the charm snap --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 502bd42..81d83eb 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -83,7 +83,7 @@ jobs: plugin: nil override-build: | ls -lR \$CRAFT_PROJECT_DIR/ - snap install --dangerous --classic \$CRAFT_PROJECT_DIR/parts/charm/src/charm-snap/*.snap + snap install --dangerous --classic /root/project/charm-snap/charm_0.0.0_amd64.snap rm -rf \$CRAFT_PROJECT_DIR/parts/charm/src/charm-snap charm: after: [charm-tools] From 240c4bc59b25861f5558c3a07e1a2c0497523488 Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 17 Apr 2024 12:51:26 -0400 Subject: [PATCH 12/13] Add upterm action to interactively debug failures in the CI --- .github/workflows/tests.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 81d83eb..54139b2 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -108,6 +108,10 @@ jobs: architectures: [amd64] EOF charmcraft pack -p tests/charm-minimal -v + ## action to interactively debug CI failures. + # - name: Setup upterm session + # if: failure() + # uses: lhotari/action-upterm@v1 - name: Upload charmcraft execution logs if: always() uses: actions/upload-artifact@v3 From 498852768ed25a269d79ca87ce7663d51567097a Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 17 Apr 2024 13:18:25 -0400 Subject: [PATCH 13/13] Add charmcraft-3.x specific step --- .github/workflows/tests.yml | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 54139b2..11074de 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -72,7 +72,8 @@ jobs: name: charm-snap path: tests/charm-minimal/charm-snap - - name: Build reactive charm with charmcraft + - name: Build reactive charm with charmcraft-2.x + if: ${{ matrix.charmcraft_channel == '2.x/stable' }} run: | set -euxo pipefail sudo snap install --classic --channel ${{ matrix.charmcraft_channel }} charmcraft @@ -108,6 +109,37 @@ jobs: architectures: [amd64] EOF charmcraft pack -p tests/charm-minimal -v + - name: Build reactive charm with charmcraft-3.x + if: ${{ matrix.charmcraft_channel == '3.x/beta' }} + run: | + set -euxo pipefail + sudo snap install --classic --channel ${{ matrix.charmcraft_channel }} charmcraft + cat << EOF | tee tests/charm-minimal/charmcraft.yaml + type: charm + parts: + charm-tools: + plugin: nil + override-build: | + ls -lR \$CRAFT_PROJECT_DIR/ + snap install --dangerous --classic /root/project/charm-snap/charm_0.0.0_amd64.snap + rm -rf \$CRAFT_PROJECT_DIR/parts/charm/src/charm-snap + charm: + after: [charm-tools] + source: . + plugin: reactive + reactive-charm-build-arguments: + - -v + - --binary-wheels-from-source + - --upgrade-buildvenv-core-deps + build-packages: + - python3-dev + - libpq-dev + base: ubuntu@24.04 + platforms: + amd64: + EOF + charmcraft pack -p tests/charm-minimal -v + mv minimal_amd64.charm minimal_ubuntu-24.04-amd64.charm ## action to interactively debug CI failures. # - name: Setup upterm session # if: failure() @@ -126,3 +158,4 @@ jobs: minimal_ubuntu-18.04-amd64.charm minimal_ubuntu-20.04-amd64.charm minimal_ubuntu-22.04-amd64.charm + minimal_ubuntu-24.04-amd64.charm