From 14fe796885aad31ffaa4be5b8c5a1792155d8d26 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Mon, 9 Nov 2020 21:32:09 +0100 Subject: [PATCH] Add missing role to GKE nodepool service account, refactor test runners and parallelize tests (#164) * add missing role to GKE nodepool service account * refactor plan test runners * remove spurious print statements from test * use concurrency via locking the fixture dir * add filelock to test requirements * fix pytest arg in cloud build * and yet another dep and args fix * fix e2e runner, use correct runner in env e2e test * revert parallel test changes, split modules and environments triggers * I should stop experimenting in PRs --- ...yaml => cloudbuild.test.environments.yaml} | 10 --- .ci/cloudbuild.test.examples.yaml | 3 + .ci/cloudbuild.test.modules.yaml | 46 +++++++++++ .gitignore | 1 + networking/shared-vpc-gke/main.tf | 9 ++- tests/conftest.py | 81 ++++++++++--------- tests/examples/test_plan.py | 6 +- tests/foundations/environments/test_plan.py | 14 ++-- .../hub_and_spoke_peering/test_plan.py | 2 - tests/networking/shared_vpc_gke/test_plan.py | 2 +- 10 files changed, 111 insertions(+), 63 deletions(-) rename .ci/{cloudbuild.test.yaml => cloudbuild.test.environments.yaml} (83%) create mode 100644 .ci/cloudbuild.test.modules.yaml diff --git a/.ci/cloudbuild.test.yaml b/.ci/cloudbuild.test.environments.yaml similarity index 83% rename from .ci/cloudbuild.test.yaml rename to .ci/cloudbuild.test.environments.yaml index d1945de5..69bd7c9b 100644 --- a/.ci/cloudbuild.test.yaml +++ b/.ci/cloudbuild.test.environments.yaml @@ -25,16 +25,6 @@ steps: rm terraform_${_TERRAFORM_VERSION}_linux_amd64.zip && chmod 755 /builder/home/.local/bin/terraform && mkdir -p /workspace/.terraform.d/plugin-cache - # TODO(ludoo): add a step that detects change files and sets tests to run - - name: python:3-alpine - id: test-modules - entrypoint: pytest - args: - - -vv - - tests/modules - env: - - PATH=/usr/local/bin:/usr/bin:/bin:/builder/home/.local/bin - - TF_CLI_CONFIG_FILE=/workspace/.ci/.terraformrc - name: python:3-alpine id: test-environments entrypoint: pytest diff --git a/.ci/cloudbuild.test.examples.yaml b/.ci/cloudbuild.test.examples.yaml index 981bf28c..27851e04 100644 --- a/.ci/cloudbuild.test.examples.yaml +++ b/.ci/cloudbuild.test.examples.yaml @@ -36,6 +36,9 @@ steps: - PATH=/usr/local/bin:/usr/bin:/bin:/builder/home/.local/bin - TF_CLI_CONFIG_FILE=/workspace/.ci/.terraformrc +options: + machineType: "N1_HIGHCPU_8" + substitutions: _TERRAFORM_VERSION: 0.13.3 diff --git a/.ci/cloudbuild.test.modules.yaml b/.ci/cloudbuild.test.modules.yaml new file mode 100644 index 00000000..c6c0bb07 --- /dev/null +++ b/.ci/cloudbuild.test.modules.yaml @@ -0,0 +1,46 @@ +# Copyright 2020 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +steps: + - name: python:3-alpine + id: prepare + entrypoint: sh + args: + - -c + - | + python -m pip install --user --no-warn-script-location -r tests/requirements.txt && + wget https://releases.hashicorp.com/terraform/${_TERRAFORM_VERSION}/terraform_${_TERRAFORM_VERSION}_linux_amd64.zip && + unzip terraform_${_TERRAFORM_VERSION}_linux_amd64.zip -d /builder/home/.local/bin && + rm terraform_${_TERRAFORM_VERSION}_linux_amd64.zip && + chmod 755 /builder/home/.local/bin/terraform && + mkdir -p /workspace/.terraform.d/plugin-cache + - name: python:3-alpine + id: test-modules + entrypoint: pytest + args: + - -vv + - tests/modules + env: + - PATH=/usr/local/bin:/usr/bin:/bin:/builder/home/.local/bin + - TF_CLI_CONFIG_FILE=/workspace/.ci/.terraformrc + +options: + machineType: "N1_HIGHCPU_8" + +substitutions: + _TERRAFORM_VERSION: 0.13.3 + +tags: + - "ci" + - "test" diff --git a/.gitignore b/.gitignore index 8875b023..bba894be 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ !tests/**/terraform.tfvars **/__pycache__ **/.pytest_cache +**/.test.lock .idea .vscode backend.tf diff --git a/networking/shared-vpc-gke/main.tf b/networking/shared-vpc-gke/main.tf index bf38397b..c954bd20 100644 --- a/networking/shared-vpc-gke/main.tf +++ b/networking/shared-vpc-gke/main.tf @@ -51,10 +51,13 @@ module "project-svc-gce" { attach = true host_project = module.project-host.project_id } + # https://cloud.google.com/stackdriver/docs/solutions/gke/troubleshooting#write_permissions iam = { - "roles/logging.logWriter" = [module.vm-bastion.service_account_iam_email], - "roles/monitoring.metricWriter" = [module.vm-bastion.service_account_iam_email], - "roles/owner" = var.owners_gce, + "roles/owner" = var.owners_gke + "roles/container.developer" = [module.vm-bastion.service_account_iam_email], + "roles/logging.logWriter" = [module.cluster-1-nodepool-1.service_account_iam_email], + "roles/monitoring.metricWriter" = [module.cluster-1-nodepool-1.service_account_iam_email], + "roles/stackdriver.resourceMetadata.writer" = [module.cluster-1-nodepool-1.service_account_iam_email] } } diff --git a/tests/conftest.py b/tests/conftest.py index 2f34dee0..7429269e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -23,12 +23,44 @@ BASEDIR = os.path.dirname(os.path.dirname(__file__)) @pytest.fixture(scope='session') -def e2e_plan_runner(plan_runner): +def _plan_runner(): + "Returns a function to run Terraform plan on a fixture." + + def run_plan(fixture_path, targets=None, **tf_vars): + "Runs Terraform plan and returns parsed output." + tf = tftest.TerraformTest(fixture_path, BASEDIR, + os.environ.get('TERRAFORM', 'terraform')) + tf.setup() + return tf.plan(output=True, tf_vars=tf_vars, targets=targets) + + return run_plan + + +@pytest.fixture(scope='session') +def plan_runner(_plan_runner): + "Returns a function to run Terraform plan on a module fixture." + + def run_plan(fixture_path, targets=None, **tf_vars): + "Runs Terraform plan and returns plan and module resources." + plan = _plan_runner(fixture_path, targets=targets, **tf_vars) + # skip the fixture + root_module = plan.root_module['child_modules'][0] + return plan, root_module['resources'] + + return run_plan + + +@pytest.fixture(scope='session') +def e2e_plan_runner(_plan_runner): "Returns a function to run Terraform plan on an end-to-end fixture." - def run_plan(fixture_path, targets=None): + def run_plan(fixture_path, targets=None, **tf_vars): "Runs Terraform plan on an end-to-end module using defaults, returns data." - _, modules = plan_runner(fixture_path, is_module=False, targets=targets) + plan = _plan_runner(fixture_path, targets=targets, **tf_vars) + # skip the fixture + root_module = plan.root_module['child_modules'][0] + modules = dict((mod['address'], mod['resources']) + for mod in root_module['child_modules']) resources = [r for m in modules.values() for r in m] return modules, resources @@ -36,41 +68,16 @@ def e2e_plan_runner(plan_runner): @pytest.fixture(scope='session') -def plan_runner(): - "Returns a function to run Terraform plan on a fixture." +def example_plan_runner(_plan_runner): + "Returns a function to run Terraform plan on documentation examples." - def run_plan(fixture_path, is_module=True, targets=None, **tf_vars): - "Runs Terraform plan and returns parsed output" - tf = tftest.TerraformTest(fixture_path, BASEDIR, - os.environ.get('TERRAFORM', 'terraform')) - tf.setup() - plan = tf.plan(output=True, tf_vars=tf_vars, targets=targets) - root_module = plan.planned_values['root_module']['child_modules'][0] - if is_module: - return (plan, root_module['resources']) - modules = dict((mod['address'], mod['resources']) - for mod in root_module['child_modules']) - return plan, modules - - return run_plan - - -@pytest.fixture(scope='session') -def example_plan_runner(): - "Returns a function to run Terraform plan on an example." - - def run_plan(fixture_path, is_module=True, targets=None, **tf_vars): - "Runs Terraform plan and returns parsed output" - tf = tftest.TerraformTest(fixture_path, BASEDIR, - os.environ.get('TERRAFORM', 'terraform')) - tf.setup() - plan = tf.plan(output=True, tf_vars=tf_vars, targets=targets) - modules = plan.modules - resources = [] - for name, module in modules.items(): - for _, resource in module.resources.items(): - resources.append(resource) - return plan, modules, resources + def run_plan(fixture_path): + "Runs Terraform plan and returns count of modules and resources." + plan = _plan_runner(fixture_path) + # the fixture is the example we are testing + return ( + len(plan.modules), + sum(len(m.resources) for m in plan.modules.values())) return run_plan diff --git a/tests/examples/test_plan.py b/tests/examples/test_plan.py index 3c0b55ef..2abaa11c 100644 --- a/tests/examples/test_plan.py +++ b/tests/examples/test_plan.py @@ -33,6 +33,6 @@ def test_example(example_plan_runner, tmp_path, example): expected_modules = int(match.group(1)) if match is not None else 1 expected_resources = int(match.group(2)) if match is not None else 1 - plan, modules, resources = example_plan_runner(str(tmp_path)) - assert expected_modules == len(modules) - assert expected_resources == len(resources) + num_modules, num_resources = example_plan_runner(str(tmp_path)) + assert expected_modules == num_modules + assert expected_resources == num_resources diff --git a/tests/foundations/environments/test_plan.py b/tests/foundations/environments/test_plan.py index a2c7c6a3..ef3f7d79 100644 --- a/tests/foundations/environments/test_plan.py +++ b/tests/foundations/environments/test_plan.py @@ -20,9 +20,9 @@ import pytest FIXTURES_DIR = os.path.join(os.path.dirname(__file__), 'fixture') -def test_folder_roles(plan_runner): +def test_folder_roles(e2e_plan_runner): "Test folder roles." - _, modules = plan_runner(FIXTURES_DIR, is_module=False) + modules, _ = e2e_plan_runner(FIXTURES_DIR) for env in ['test', 'prod']: resources = modules[f'module.test.module.environment-folders["{env}"]'] folders = [r for r in resources if r['type'] == 'google_folder'] @@ -35,13 +35,13 @@ def test_folder_roles(plan_runner): assert len(bindings) == 5 -def test_org_roles(plan_runner): +def test_org_roles(e2e_plan_runner): "Test folder roles." - vars = { + tf_vars = { 'organization_id': 'organizations/123', 'iam_xpn_config': '{grant = true, target_org = true}' } - _, modules = plan_runner(FIXTURES_DIR, is_module=False, **vars) + modules, _ = e2e_plan_runner(FIXTURES_DIR, **tf_vars) for env in ['test', 'prod']: resources = modules[f'module.test.module.environment-folders["{env}"]'] folder_bindings = [r['index'] @@ -53,6 +53,6 @@ def test_org_roles(plan_runner): if r['type'] == 'google_organization_iam_member'] assert len(org_bindings) == 2 assert {b['values']['role'] for b in org_bindings} == { - 'roles/resourcemanager.organizationViewer', - 'roles/compute.xpnAdmin' + 'roles/resourcemanager.organizationViewer', + 'roles/compute.xpnAdmin' } diff --git a/tests/networking/hub_and_spoke_peering/test_plan.py b/tests/networking/hub_and_spoke_peering/test_plan.py index fad58fb1..1ce41422 100644 --- a/tests/networking/hub_and_spoke_peering/test_plan.py +++ b/tests/networking/hub_and_spoke_peering/test_plan.py @@ -23,7 +23,5 @@ FIXTURES_DIR = os.path.join(os.path.dirname(__file__), 'fixture') def test_resources(e2e_plan_runner): "Test that plan works and the numbers of resources is as expected." modules, resources = e2e_plan_runner(FIXTURES_DIR) - import pprint - pprint.pprint(resources) assert len(modules) == 18 assert len(resources) == 53 diff --git a/tests/networking/shared_vpc_gke/test_plan.py b/tests/networking/shared_vpc_gke/test_plan.py index 7f0bf42f..abf7e9b4 100644 --- a/tests/networking/shared_vpc_gke/test_plan.py +++ b/tests/networking/shared_vpc_gke/test_plan.py @@ -24,4 +24,4 @@ def test_resources(e2e_plan_runner): "Test that plan works and the numbers of resources is as expected." modules, resources = e2e_plan_runner(FIXTURES_DIR) assert len(modules) == 10 - assert len(resources) == 43 + assert len(resources) == 45