From e20d416bd85623bef1dc58b6eeb11bc6bbe3d3d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20R=C3=BCegg?= Date: Tue, 12 May 2020 10:14:10 +0200 Subject: [PATCH 1/5] Make cloud region an optional fact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Write tests to ensure behaviour of target rendering. Signed-off-by: Simon Rüegg --- commodore/cluster.py | 40 ++++++++++++++++--------------- tests/test_target.py | 56 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 19 deletions(-) create mode 100644 tests/test_target.py diff --git a/commodore/cluster.py b/commodore/cluster.py index 3256dd57..be07092d 100644 --- a/commodore/cluster.py +++ b/commodore/cluster.py @@ -22,36 +22,36 @@ def fetch_cluster(cfg, clusterid): def reconstruct_api_response(target_yml): target_data = yaml_load(target_yml)['parameters'] return { - "id": target_data['cluster']['name'], - "facts": { - "cloud": target_data['cloud']['region'], - "distribution": target_data['cluster']['dist'], - "region": target_data['cloud']['region'], + 'id': target_data['cluster']['name'], + 'facts': { + 'cloud': target_data['cloud']['region'], + 'distribution': target_data['cluster']['dist'], + 'region': target_data['cloud']['region'], }, - "gitRepo": { - "url": target_data['cluster']['catalog_url'], + 'gitRepo': { + 'url': target_data['cluster']['catalog_url'], }, - "tenant": target_data['customer']['name'], + 'tenant': target_data['customer']['name'], } def _full_target(cluster, components, catalog): - cloud_provider = cluster['facts']['cloud'] - cloud_region = cluster['facts']['region'] - cluster_distro = cluster['facts']['distribution'] + cluster_facts = cluster['facts'] + for required_fact in ['distribution', 'cloud']: + if required_fact not in cluster_facts: + raise click.ClickException(f"Required fact '{required_fact}' not set") + + cluster_distro = cluster_facts['distribution'] + cloud_provider = cluster_facts['cloud'] cluster_id = cluster['id'] customer = cluster['tenant'] component_defaults = [f"defaults.{cn}" for cn in components if (P('inventory/classes/defaults') / f"{cn}.yml").is_file()] global_defaults = ['global.common', f"global.{cluster_distro}", f"global.{cloud_provider}"] - if not cluster_distro: - raise click.ClickException("Required fact 'distribution' not set") - if not cloud_provider: - raise click.ClickException("Required fact 'cloud' not set") - if cloud_region: - global_defaults.append(f"global.{cloud_provider}.{cloud_region}") + if 'region' in cluster_facts: + global_defaults.append(f"global.{cloud_provider}.{cluster_facts['region']}") global_defaults.append(f"{customer}.{cluster_id}") - return { + target = { 'classes': component_defaults + global_defaults, 'parameters': { 'target_name': 'cluster', @@ -62,13 +62,15 @@ def _full_target(cluster, components, catalog): }, 'cloud': { 'provider': f"{cloud_provider}", - 'region': f"{cloud_region}", }, 'customer': { 'name': f"{customer}" }, } } + if 'region' in cluster_facts: + target['parameters']['cloud']['region'] = cluster_facts['region'] + return target def update_target(cfg, cluster): diff --git a/tests/test_target.py b/tests/test_target.py new file mode 100644 index 00000000..a76be200 --- /dev/null +++ b/tests/test_target.py @@ -0,0 +1,56 @@ +""" +Unit-tests for target generation +""" + +import click +import pytest +import commodore.cluster as cluster + + +cluster_obj = { + 'id': 'mycluster', + 'tenant': 'mytenant', + 'facts': { + 'distribution': 'rancher', + 'cloud': 'cloudscale', + } +} +components = ['test-component'] +catalog = 'ssh://git@git.example.com/cluster-catalogs/mycluster' + + +def test_render_target(): + target = cluster._full_target(cluster_obj, components, catalog) + facts = cluster_obj['facts'] + assert target != "" + all_classes = ([f"defaults.{cn}" for cn in components] + + ['global.common', + f"global.{facts['distribution']}", + f"global.{facts['cloud']}", + f"{cluster_obj['tenant']}.{cluster_obj['id']}", + ]) + assert len(target['classes']) == len( + all_classes), "rendered target includes different amount of classes" + # Test order of included classes + for i in range(len(all_classes)): + assert target['classes'][i] == all_classes[i] + assert target['parameters']['target_name'] == 'cluster' + + +def test_optional_facts(): + cluster_obj['facts']['region'] = 'rma1' + target = cluster._full_target(cluster_obj, components, catalog) + facts = cluster_obj['facts'] + assert f"global.{facts['cloud']}.{facts['region']}" in target['classes'] + + +def test_missing_facts(): + cl = { + 'id': 'mycluster', + 'tenant': 'mytenant', + 'facts': { + 'distribution': 'rancher', + } + } + with pytest.raises(click.ClickException): + cluster._full_target(cl, components, catalog) From f2baf738ab3039f0fb9888681064b16076dfedc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20R=C3=BCegg?= Date: Tue, 12 May 2020 10:15:40 +0200 Subject: [PATCH 2/5] Run tox in Pipenv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Docker setup is very slow since it copies around a lot of files. Signed-off-by: Simon Rüegg --- Pipfile | 2 +- tox.mk | 23 +++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/Pipfile b/Pipfile index 5a15b315..b2d1485c 100644 --- a/Pipfile +++ b/Pipfile @@ -24,5 +24,5 @@ build_kapitan_helm_binding = "./tools/build_kapitan_helm_binding.sh" autopep = "autopep8 --in-place --aggressive --recursive --verbose ./" local_reveal = "./tools/reveal.sh" compile = "kapitan compile -J . dependencies/ --refs-path ./catalog/refs" -test = "docker run -it --rm -v $PWD:/src:ro -v $PWD/.dtox:/app/.tox docker.io/painless/tox" +test = "tox" update_requirements = "tox -e requirements" diff --git a/tox.mk b/tox.mk index e08fc564..bb2218d0 100644 --- a/tox.mk +++ b/tox.mk @@ -1,29 +1,32 @@ -.PHONY: lint_flake8 lint_pylint lint_safety lint_bandit lint_readme +.PHONY: tox lint_flake8 lint_pylint lint_safety lint_bandit lint_readme -TOX_COMMAND = docker run --rm -v $(PWD):/src:ro -v $(PWD)/.tox:/app/.tox docker.io/painless/tox +TOX_COMMAND = pipenv run tox + +tox: + $(TOX_COMMAND) lint_flake8: - $(TOX_COMMAND) tox -e flake8 + $(TOX_COMMAND) -e flake8 lint_pylint: - $(TOX_COMMAND) tox -e pylint + $(TOX_COMMAND) -e pylint lint_safety: - $(TOX_COMMAND) tox -e safety + $(TOX_COMMAND) -e safety lint_bandit: - $(TOX_COMMAND) tox -e bandit + $(TOX_COMMAND) -e bandit lint_readme: - $(TOX_COMMAND) tox -e readme + $(TOX_COMMAND) -e readme .PHONY: test_py36 test_py37 test_py38 test_py36: - $(TOX_COMMAND) tox -e py36 + $(TOX_COMMAND) -e py36 test_py37: - $(TOX_COMMAND) tox -e py37 + $(TOX_COMMAND) -e py37 test_py38: - $(TOX_COMMAND) tox -e py38 + $(TOX_COMMAND) -e py38 From cc6b6c0870fc381a46eb3bf621fbdf24d339b56e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20R=C3=BCegg?= Date: Tue, 12 May 2020 10:45:52 +0200 Subject: [PATCH 3/5] Use GitHub actions matrix build --- .github/workflows/test.yml | 77 ++++++++++++-------------------------- 1 file changed, 23 insertions(+), 54 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 77d16f07..6213d90b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,66 +5,35 @@ on: - master jobs: - lint_flake8: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 + tests: + runs-on: ubuntu-latest + strategy: + matrix: + command: + - lint_flake8 + - lint_pylint + - lint_safety + - lint_bandit + - lint_readme + - test_py36 + - test_py37 + - test_py38 + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-python@v2 + - name: Install Pipenv + run: | + pip install pipenv tox - name: Flake8 - run: make lint_flake8 - lint_pylint: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 - - name: Pylint - run: make lint_pylint - lint_safety: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 - - name: Safety - run: make lint_safety - lint_bandit: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 - - name: Bandit - run: make lint_bandit - lint_readme: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 - - name: Readme - run: make lint_readme - test_py36: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 - - name: Run tests on Python 3.6 - run: make test_py36 - test_py37: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 - - name: Run tests on Python 3.7 - run: make test_py37 - test_py38: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 + run: make ${{ matrix.command }} + - name: Run tests on Python 3.8 run: make test_py38 build: needs: - - lint_flake8 - - lint_pylint - - lint_safety - - lint_bandit - - lint_readme - - test_py36 - - test_py37 - - test_py38 + - tests runs-on: ubuntu-latest steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - name: Build image run: make docker From 7e39960de383889182a52801396c8b3805bf8755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20R=C3=BCegg?= Date: Tue, 12 May 2020 11:46:28 +0200 Subject: [PATCH 4/5] Fix local mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To also make the region optional. Signed-off-by: Simon Rüegg --- commodore/cluster.py | 8 ++++-- tests/test_target.py | 66 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/commodore/cluster.py b/commodore/cluster.py index be07092d..5c572456 100644 --- a/commodore/cluster.py +++ b/commodore/cluster.py @@ -21,18 +21,20 @@ def fetch_cluster(cfg, clusterid): def reconstruct_api_response(target_yml): target_data = yaml_load(target_yml)['parameters'] - return { + api_resp = { 'id': target_data['cluster']['name'], 'facts': { - 'cloud': target_data['cloud']['region'], + 'cloud': target_data['cloud']['provider'], 'distribution': target_data['cluster']['dist'], - 'region': target_data['cloud']['region'], }, 'gitRepo': { 'url': target_data['cluster']['catalog_url'], }, 'tenant': target_data['customer']['name'], } + if 'region' in target_data['cloud']: + api_resp['facts']['region'] = target_data['cloud']['region'] + return api_resp def _full_target(cluster, components, catalog): diff --git a/tests/test_target.py b/tests/test_target.py index a76be200..736cc556 100644 --- a/tests/test_target.py +++ b/tests/test_target.py @@ -54,3 +54,69 @@ def test_missing_facts(): } with pytest.raises(click.ClickException): cluster._full_target(cl, components, catalog) + + +def test_reconstruct_api_response(tmp_path): + targetyml = tmp_path / 'cluster.yml' + with open(targetyml, 'w') as file: + file.write('''classes: +- defaults.argocd +- global.common +- global.k3d +- global.localdev +- t-delicate-pine-3938.c-twilight-water-9032 +parameters: + cloud: + provider: localdev + region: north + cluster: + catalog_url: ssh://git@git.vshn.net/syn-dev/cluster-catalogs/srueg-k3d-int.git + dist: k3d + name: c-twilight-water-9032 + customer: + name: t-delicate-pine-3938 + target_name: cluster ''') + + api_response = cluster.reconstruct_api_response(targetyml) + assert api_response['id'] == 'c-twilight-water-9032' + assert api_response['tenant'] == 't-delicate-pine-3938' + assert api_response['facts']['distribution'] == 'k3d' + assert api_response['facts']['region'] == 'north' + + +def test_reconstruct_api_response_no_region(tmp_path): + targetyml = tmp_path / 'cluster.yml' + with open(targetyml, 'w') as file: + file.write('''classes: +parameters: + cloud: + provider: localdev + cluster: + catalog_url: ssh://git@git.vshn.net/syn-dev/cluster-catalogs/srueg-k3d-int.git + dist: k3d + name: c-twilight-water-9032 + customer: + name: t-delicate-pine-3938 + target_name: cluster ''') + + api_response = cluster.reconstruct_api_response(targetyml) + assert 'region' not in api_response['facts'] + + +def test_reconstruct_api_response_missing_fact(tmp_path): + targetyml = tmp_path / 'cluster.yml' + with open(targetyml, 'w') as file: + file.write('''classes: +parameters: + cloud: + region: north + cluster: + catalog_url: ssh://git@git.vshn.net/syn-dev/cluster-catalogs/srueg-k3d-int.git + dist: k3d + name: c-twilight-water-9032 + customer: + name: t-delicate-pine-3938 + target_name: cluster ''') + + with pytest.raises(KeyError): + cluster.reconstruct_api_response(targetyml) From edb978d13ab2557ae7ab242fb19308c1e3158841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20R=C3=BCegg?= Date: Tue, 12 May 2020 11:50:08 +0200 Subject: [PATCH 5/5] Rename variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To make Flake8 happy Signed-off-by: Simon Rüegg --- commodore/git.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/commodore/git.py b/commodore/git.py index 7c8c5ee6..9d39da8d 100644 --- a/commodore/git.py +++ b/commodore/git.py @@ -161,10 +161,10 @@ def stage_all(repo): # and a_blob as after. before = c.b_blob.data_stream.read().decode('utf-8').split('\n') after = c.a_blob.data_stream.read().decode('utf-8').split('\n') - u = difflib.unified_diff(before, after, lineterm='', - fromfile=c.b_path, tofile=c.a_path) - u = [_colorize_diff(l) for l in u] - difftext.append('\n'.join(u).strip()) + diff = difflib.unified_diff(before, after, lineterm='', + fromfile=c.b_path, tofile=c.a_path) + diff = [_colorize_diff(line) for line in diff] + difftext.append('\n'.join(diff).strip()) return '\n'.join(difftext), changed