From 92759c7dafa82aa7f6c036e79b60c498698f20a4 Mon Sep 17 00:00:00 2001 From: Noah Gildersleeve Date: Thu, 15 Aug 2024 14:31:06 -0700 Subject: [PATCH] added in tests for bad checksums on ubuntu images, and added in some pydoc docs in the images and volumes sections added in opensuse checksum and added associated config data fixed pep8 errors made some requested changes from PR review PR review changes removed old status check fixed an improper assertion, added a default for image_checksum, and added a setter for opensuse_checksum changed assertion message to be more clear changed checksum default type removed default checksums from default fixed some pep8 errors fixed some errors and made some adjustments based on PR review comments --- apiclient/harvester_api/managers/images.py | 8 +- apiclient/harvester_api/managers/images.pyi | 2 + config.yml | 8 +- harvester_e2e_tests/conftest.py | 12 + harvester_e2e_tests/fixtures/api_client.py | 12 + harvester_e2e_tests/fixtures/images.py | 9 +- .../integrations/test_1_images.py | 8 +- .../integrations/test_1_volumes.py | 284 +++++++++--------- .../integrations/test_3_vm_functions.py | 2 +- 9 files changed, 192 insertions(+), 153 deletions(-) diff --git a/apiclient/harvester_api/managers/images.py b/apiclient/harvester_api/managers/images.py index 8ab3bc37e..fe28737be 100644 --- a/apiclient/harvester_api/managers/images.py +++ b/apiclient/harvester_api/managers/images.py @@ -11,7 +11,8 @@ class ImageManager(BaseManager): DOWNLOAD_fmt = "v1/harvester/harvesterhci.io.virtualmachineimages/{ns}/{uid}/download" _KIND = "VirtualMachineImage" - def create_data(self, name, url, desc, stype, namespace, display_name=None, storageclass=None): + def create_data(self, name, url, desc, stype, namespace, image_checksum=None, + display_name=None, storageclass=None): data = { "apiVersion": "{API_VERSION}", "kind": self._KIND, @@ -26,6 +27,7 @@ def create_data(self, name, url, desc, stype, namespace, display_name=None, stor "spec": { "displayName": display_name or name, "sourceType": stype, + "checksum": image_checksum, "url": url } } @@ -38,10 +40,10 @@ def create(self, name, namespace=DEFAULT_NAMESPACE, **kwargs): return self._create(self.PATH_fmt.format(uid=name, ns=namespace), **kwargs) def create_by_url( - self, name, url, namespace=DEFAULT_NAMESPACE, + self, name, url, imageChecksum, namespace=DEFAULT_NAMESPACE, description="", display_name=None, storageclass=None ): - data = self.create_data(name, url, description, "download", namespace, + data = self.create_data(name, url, description, "download", namespace, imageChecksum, display_name, storageclass) return self.create("", namespace, json=data) diff --git a/apiclient/harvester_api/managers/images.pyi b/apiclient/harvester_api/managers/images.pyi index b628a9411..ff4ad4b24 100644 --- a/apiclient/harvester_api/managers/images.pyi +++ b/apiclient/harvester_api/managers/images.pyi @@ -18,6 +18,7 @@ class ImageManager(BaseManager): desc: str, stype: str, namespace: str, + imageChecksum=str, display_name: str = ... ) -> dict: """ @@ -43,6 +44,7 @@ class ImageManager(BaseManager): self, name: str, url: str, + imageChecksum: str, namespace: str = ..., description: str = ..., display_name: str = ... diff --git a/config.yml b/config.yml index cf51d8d87..f446adfb4 100644 --- a/config.yml +++ b/config.yml @@ -1,5 +1,5 @@ # Harvester Cluster -endpoint: 'https://localhost:30443' +endpoint: 'https://localhost' username: 'admin' password: 'password1234' # Be used to access Harvester node, fill in one of following is enough. @@ -23,7 +23,13 @@ sleep-timeout: 3 node-scripts-location: 'scripts/vagrant' opensuse-image-url: https://download.opensuse.org/repositories/Cloud:/Images:/Leap_15.5/images/openSUSE-Leap-15.5.x86_64-NoCloud.qcow2 +# sha512sum for opensuse image-url +opensuse-checksum: '' + ubuntu-image-url: https://cloud-images.ubuntu.com/releases/releases/22.04/release/ubuntu-22.04-server-cloudimg-amd64.img +# sha512sum for ubuntu image-url +ubuntu-checksum: '' + # URL to download all images image-cache-url: '' diff --git a/harvester_e2e_tests/conftest.py b/harvester_e2e_tests/conftest.py index 76dba4bef..7da280584 100644 --- a/harvester_e2e_tests/conftest.py +++ b/harvester_e2e_tests/conftest.py @@ -274,6 +274,18 @@ def pytest_addoption(parser): default=config_data.get('terraform-provider-rancher'), help=('Version of Terraform Rancher Provider') ) + parser.addoption( + '--ubuntu-checksum', + action='store', + default=config_data.get('ubuntu-checksum'), + help=('Checksum for ubuntu_image') + ) + parser.addoption( + '--opensuse-checksum', + action='store', + default=config_data.get('opensuse-checksum'), + help=('Checksum for opensuse_image') + ) def pytest_configure(config): diff --git a/harvester_e2e_tests/fixtures/api_client.py b/harvester_e2e_tests/fixtures/api_client.py index 9437e227e..51d51a769 100644 --- a/harvester_e2e_tests/fixtures/api_client.py +++ b/harvester_e2e_tests/fixtures/api_client.py @@ -70,6 +70,18 @@ def rancher_wait_timeout(request): return request.config.getoption("--rancher-cluster-wait-timeout", 1800) +@pytest.fixture(scope="session") +def ubuntu_checksum(request): + """Returns Ubuntu checksum from config""" + return request.config.getoption("--ubuntu-checksum") + + +@pytest.fixture(scope="session") +def opensuse_checksum(request): + """Returns openSUSE checksum from config""" + return request.config.getoption("--opensuse-checksum") + + @pytest.fixture(scope="session") def host_state(request): class HostState: diff --git a/harvester_e2e_tests/fixtures/images.py b/harvester_e2e_tests/fixtures/images.py index 642e9df1e..9826b81df 100644 --- a/harvester_e2e_tests/fixtures/images.py +++ b/harvester_e2e_tests/fixtures/images.py @@ -14,6 +14,7 @@ @pytest.fixture(scope="session") def image_opensuse(request, api_client): image_server = request.config.getoption("--image-cache-url") + image_checksum = request.config.getoption("--opensuse-checksum", default=None) url = urlparse( request.config.getoption("--opensuse-image-url") ) @@ -22,12 +23,13 @@ def image_opensuse(request, api_client): *_, image_name = url.path.rsplit("/", 1) url = urlparse(urljoin(f"{image_server}/", image_name)) - return ImageInfo(url, name="opensuse", ssh_user="opensuse") + return ImageInfo(url, image_checksum, name="opensuse", ssh_user="opensuse") @pytest.fixture(scope="session") def image_ubuntu(request): image_server = request.config.getoption("--image-cache-url") + image_checksum = request.config.getoption("--ubuntu-checksum", default=None) url = urlparse( request.config.getoption("--ubuntu-image-url") or DEFAULT_UBUNTU_IMAGE_URL ) @@ -36,7 +38,7 @@ def image_ubuntu(request): *_, image_name = url.path.rsplit("/", 1) url = urlparse(urljoin(f"{image_server}/", image_name)) - return ImageInfo(url, name="ubuntu", ssh_user="ubuntu") + return ImageInfo(url, image_checksum, name="ubuntu", ssh_user="ubuntu") @pytest.fixture(scope="session") @@ -49,13 +51,14 @@ def image_k3s(request): class ImageInfo: - def __init__(self, url_result, name="", ssh_user=None): + def __init__(self, url_result, image_checksum=None, name="", ssh_user=None): self.url_result = url_result if name: self.name = name else: self.name = self.url.rsplit("/", 1)[-1] self.ssh_user = ssh_user + self.image_checksum = image_checksum def __repr__(self): return f"{__class__.__name__}({self.url_result})" diff --git a/harvester_e2e_tests/integrations/test_1_images.py b/harvester_e2e_tests/integrations/test_1_images.py index 3ce8524d2..f7f126f27 100644 --- a/harvester_e2e_tests/integrations/test_1_images.py +++ b/harvester_e2e_tests/integrations/test_1_images.py @@ -37,8 +37,8 @@ def fake_invalid_image_file(): yield Path(f.name) -def create_image_url(api_client, name, image_url, wait_timeout): - code, data = api_client.images.create_by_url(name, image_url) +def create_image_url(api_client, name, image_url, image_checksum, wait_timeout): + code, data = api_client.images.create_by_url(name, image_url, image_checksum) assert 201 == code, (code, data) image_spec = data.get("spec") @@ -112,6 +112,7 @@ def get_image(api_client, image_name): @pytest.fixture(scope="class") def cluster_network(api_client, vlan_nic): + # We should change this at some point. It fails if the total cnet name is over 12 chars cnet = f"cnet-{vlan_nic}" code, data = api_client.clusternetworks.get(cnet) if code != 200: @@ -266,7 +267,8 @@ def test_create_image_url(self, image_info, unique_name, api_client, wait_timeou """ image_name = f"{image_info.name}-{unique_name}" image_url = image_info.url - create_image_url(api_client, image_name, image_url, wait_timeout) + create_image_url(api_client, image_name, image_url, + image_info.image_checksum, wait_timeout) @pytest.mark.skip_version_if("> v1.2.0", "<= v1.4.0", reason="Issue#4293 fix after `v1.4.0`") @pytest.mark.p0 diff --git a/harvester_e2e_tests/integrations/test_1_volumes.py b/harvester_e2e_tests/integrations/test_1_volumes.py index d83c06679..d25998438 100644 --- a/harvester_e2e_tests/integrations/test_1_volumes.py +++ b/harvester_e2e_tests/integrations/test_1_volumes.py @@ -1,8 +1,6 @@ -from time import sleep -from datetime import datetime, timedelta - import yaml import pytest +from hashlib import sha512 pytest_plugins = [ 'harvester_e2e_tests.fixtures.api_client', @@ -12,9 +10,20 @@ @pytest.fixture(scope="module") def ubuntu_image(api_client, unique_name, image_ubuntu, polling_for): + """ + Generates a Ubuntu image + + 1. Creates an image name based on unique_name + 2. Create the image based on URL + 3. Response for creation should be 201 + 4. Loop while waiting for image to be created + 5. Yield the image with the namespace and name + 6. Delete the image + 7. The response for getting the image name should be 404 after deletion + """ image_name = f"img-{unique_name}" - - code, data = api_client.images.create_by_url(image_name, image_ubuntu.url) + code, data = api_client.images.create_by_url(image_name, image_ubuntu.url, + image_ubuntu.image_checksum) assert 201 == code, f"Fail to create image\n{code}, {data}" code, data = polling_for("image do created", lambda c, d: c == 200 and d.get('status', {}).get('progress') == 100, @@ -33,6 +42,40 @@ def ubuntu_image(api_client, unique_name, image_ubuntu, polling_for): api_client.images.get, image_name) +@pytest.fixture(scope="module") +def ubuntu_image_bad_checksum(api_client, unique_name, image_ubuntu, polling_for): + """ + Generates a Ubuntu image with a bad sha512 checksum + + 1. Creates an image name based on unique_name + 2. Create the image based on URL with a bad statically assigned checksum + 3. Response for creation should be 201 + 4. Loop while waiting for image to be created + 5. Yield the image with the namespace and name + 6. Delete the image + 7. The response for getting the image name should be 404 after deletion + """ + + image_name = f"img-{unique_name + '-badchecksum'}" + # Random fake checksum to use in test + fake_checksum = sha512(b'not_a_valid_checksum').hexdigest() + code, data = api_client.images.create_by_url(image_name, image_ubuntu.url, fake_checksum) + assert 201 == code, f"Fail to create image\n{code}, {data}" + code, data = polling_for("image do created", + lambda c, d: c == 200 and d.get('status', {}).get('progress') == 100, + api_client.images.get, image_name) + namespace = data['metadata']['namespace'] + name = data['metadata']['name'] + yield dict(ssh_user=image_ubuntu.ssh_user, id=f"{namespace}/{name}", display_name=image_name) + code, data = api_client.images.get(image_name) + if 200 == code: + code, data = api_client.images.delete(image_name) + assert 200 == code, f"Fail to cleanup image\n{code}, {data}" + polling_for("image do deleted", + lambda c, d: 404 == c, + api_client.images.get, image_name) + + @pytest.fixture(scope="class") def ubuntu_vm(api_client, unique_name, ubuntu_image, polling_for): vm_name = f"vm-{unique_name}" @@ -72,152 +115,109 @@ def ubuntu_vm(api_client, unique_name, ubuntu_image, polling_for): @pytest.mark.p0 @pytest.mark.volumes -@pytest.mark.parametrize("source_type", ["New", "VM-Image"]) @pytest.mark.parametrize("create_as", ["json", "yaml"]) -class TestVolume: - fixtures, volumes = dict(), dict() - - @pytest.mark.dependency() - def test_create_volume( - self, api_client, unique_name, ubuntu_image, create_as, source_type, polling_for - ): - image_id, storage_cls = None, None - unique_name = f"{create_as}-{source_type.lower()}-{unique_name}" - self.volumes[f"{create_as}-{source_type}"] = unique_name - - if source_type == "VM-Image": - image_id, storage_cls = ubuntu_image['id'], f"longhorn-{ubuntu_image['display_name']}" - - spec = api_client.volumes.Spec("10Gi", storage_cls) - if create_as == 'yaml': - kws = dict(headers={'Content-Type': 'application/yaml'}, json=None, - data=yaml.dump(spec.to_dict(unique_name, 'default', image_id=image_id))) - else: - kws = dict() - code, data = api_client.volumes.create(unique_name, spec, image_id=image_id, **kws) - assert 201 == code, (code, unique_name, data, image_id) - - polling_for("volume do created", - lambda code, data: 200 == code and data['status']['phase'] == "Bound", - api_client.volumes.get, unique_name) - - code, data = api_client.volumes.get(unique_name) - mdata, annotations = data['metadata'], data['metadata']['annotations'] - assert 200 == code, (code, data) - assert unique_name == mdata['name'], (code, data) - # status - assert not mdata['state']['error'], (code, data) - assert not mdata['state']['transitioning'], (code, data) - assert data['status']['phase'] == "Bound", (code, data) - # source - if source_type == "VM-Image": - assert image_id == annotations['harvesterhci.io/imageId'], (code, data) - else: - assert not annotations.get('harvesterhci.io/imageId'), (code, data) - # attachment - assert not annotations.get("harvesterhci.io/owned-by"), (code, data) - - @pytest.mark.dependency(depends=["TestVolume::test_create_volume"], param=True) - def test_clone_volume(self, api_client, wait_timeout, create_as, source_type): - self.fixtures.update(api_client=api_client, wait_timeout=wait_timeout) - unique_name = self.volumes[f"{create_as}-{source_type}"] - code, data = api_client.volumes.get(unique_name) - assert 200 == code, (code, data) +@pytest.mark.parametrize("source_type", ["New", "VM Image"]) +def test_create_volume(api_client, unique_name, ubuntu_image, create_as, source_type, polling_for): + """ + 1. Create a volume from image + 2. Create should respond with 201 + 3. Wait for volume to create + 4. Failures should be at 0 + 5. Get volume metadata + 6. Volume should not be in error or transitioning state + 7. ImageId should match what was used in create + 8. Delete volume + 9. Delete volume should reply 404 after delete + Ref. + """ + image_id, storage_cls = None, None + if source_type == "VM Image": + image_id, storage_cls = ubuntu_image['id'], f"longhorn-{ubuntu_image['display_name']}" - cloned_name = f"cloned-{unique_name}" - code, data = api_client.volumes.clone(unique_name, cloned_name) - assert 204 == code, (code, data) - - endtime = datetime.now() + timedelta(seconds=wait_timeout) - while endtime > datetime.now(): - code, data = api_client.volumes.get(cloned_name) - if "Bound" == data['status']['phase']: - break - sleep(5) - else: - raise AssertionError( - "Volume not changed to phase: _Bound_ with {wait_timeout} timed out\n" - f"Got error: {code}, {data}" - ) - - self.volumes[cloned_name] = cloned_name - - @classmethod - def teardown_class(cls): - api_client, wait_timeout = cls.fixtures['api_client'], cls.fixtures['wait_timeout'] - - vol_names = cls.volumes.values() - for name in vol_names: - api_client.volumes.delete(name) - - endtime = datetime.now() + timedelta(seconds=wait_timeout) - while endtime > datetime.now(): - code, data = api_client.volumes.get() - volumes = [v['metadata']['name'] for v in data['data']] - if all(v not in volumes for v in vol_names): - break - sleep(3) - else: - raise AssertionError( - "Volumes not deleted correctly\n" - f"existing: {volumes}\n" - f"created: {vol_names}" - ) + spec = api_client.volumes.Spec("10Gi", storage_cls) + if create_as == 'yaml': + kws = dict(headers={'Content-Type': 'application/yaml'}, json=None, + data=yaml.dump(spec.to_dict(unique_name, 'default', image_id=image_id))) + else: + kws = dict() + code, data = api_client.volumes.create(unique_name, spec, image_id=image_id, **kws) + assert 201 == code, (code, unique_name, data, image_id) + + polling_for("volume do created", + lambda code, data: 200 == code and data['status']['phase'] == "Bound", + api_client.volumes.get, unique_name) + code2, data2 = api_client.images.get(ubuntu_image['display_name']) + # This grabs the failed count for the image + failed: int = data2['status']['failed'] + # This makes sure that the failures are 0 + assert failed <= 3, 'Image failed more than 3 times' + + code, data = api_client.volumes.get(unique_name) + mdata, annotations = data['metadata'], data['metadata']['annotations'] + assert 200 == code, (code, data) + assert unique_name == mdata['name'], (code, data) + # status + assert not mdata['state']['error'], (code, data) + assert not mdata['state']['transitioning'], (code, data) + assert data['status']['phase'] == "Bound", (code, data) + # source + if source_type == "VM Image": + assert image_id == annotations['harvesterhci.io/imageId'], (code, data) + else: + assert not annotations.get('harvesterhci.io/imageId'), (code, data) + # teardown + polling_for("volume do deleted", lambda code, _: 404 == code, + api_client.volumes.delete, unique_name) -@pytest.mark.p0 -@pytest.mark.negative +@pytest.mark.p1 @pytest.mark.volumes -def test_volume_export(api_client, wait_timeout, unique_name, ubuntu_image): - ''' ref: https://github.com/harvester/tests/issues/1057 - - 1. Create image - 2. Create volume from the image - 3. export the volume to new image - 4. delete the new image - ''' - image_id, storage_cls = ubuntu_image['id'], f"longhorn-{ubuntu_image['display_name']}" - spec = api_client.volumes.Spec("10Gi", storage_cls) +@pytest.mark.negative +@pytest.mark.parametrize("create_as", ["json", "yaml"]) +@pytest.mark.parametrize("source_type", ["New", "VM Image"]) +def test_create_volume_bad_checksum(api_client, unique_name, ubuntu_image_bad_checksum, + create_as, source_type, polling_for): + """ + 1. Create a volume from image with a bad checksum + 2. Create should respond with 201 + 3. Wait for volume to create + 4. Wait for 4 failures in the volume fail status + 5. Failures should be set at 4 + 6. Delete volume + 7. Delete volume should reply 404 after delete + Ref. https://github.com/harvester/tests/issues/1121 + """ + image_id, storage_cls = None, None + if source_type == "VM Image": + image_id, storage_cls = ubuntu_image_bad_checksum['id'], \ + f"longhorn-{ubuntu_image_bad_checksum['display_name']}" - # Create Volume from image and wait it bounded - code, data = api_client.volumes.create(unique_name, spec, image_id=image_id) - assert 201 == code, (code, data) - endtime = datetime.now() + timedelta(seconds=wait_timeout) - while endtime > datetime.now(): - code, data = api_client.volumes.get(unique_name) - if "Bound" == data['status']['phase']: - break - sleep(5) + spec = api_client.volumes.Spec("10Gi", storage_cls) + if create_as == 'yaml': + kws = dict(headers={'Content-Type': 'application/yaml'}, json=None, + data=yaml.dump(spec.to_dict(unique_name, 'default', image_id=image_id))) else: - raise AssertionError( - "Volume not changed to phase: _Bound_ with {wait_timeout} timed out\n" - f"Got error: {code}, {data}" - ) - - # Export volume to new image - code, data = api_client.volumes.export(unique_name, unique_name, "harvester-longhorn") - assert 204 == code, (code, data) - # check the new image is available and creating - code, data = api_client.images.get() - assert 200 == code, (code, data) - new_img = next(d for d in data['items'] if unique_name == d['spec']['displayName']) - assert new_img, (code, data['items']) - assert 100 > new_img['status'].get('progress', 0), (code, new_img) - # Delete the source volume - code, data = api_client.volumes.delete(unique_name) - assert 422 == code, (code, data) + kws = dict() + code, data = api_client.volumes.create(unique_name, spec, image_id=image_id, **kws) + assert 201 == code, (code, unique_name, data, image_id) + + polling_for("volume do created", + lambda code, data: 200 == code and data['status']['phase'] == "Bound", + api_client.volumes.get, unique_name) + code2, data2 = api_client.images.get(ubuntu_image_bad_checksum['display_name']) + polling_for("failed to process sync file", + lambda code2, data2: 200 == code2 and data2['status']['failed'] == 4, + api_client.images.get, ubuntu_image_bad_checksum['display_name']) + + # This grabs the failed count for the image + code2, data2 = api_client.images.get(ubuntu_image_bad_checksum['display_name']) + failed: int = data2['status']['failed'] + # This makes sure that the tests fails with bad checksum + assert failed == 4, 'Image download correctly failed more than 3 times with bad checksum' # teardown - fns = [(api_client.volumes, unique_name), (api_client.images, new_img['metadata']['name'])] - endtime = datetime.now() + timedelta(seconds=wait_timeout) - while endtime > datetime.now(): - fn, name = fns[-1] - code, data = fn.delete(name) - if 404 == code: - fns.pop() - if not fns: - break - sleep(3) + polling_for("volume do deleted", lambda code, _: 404 == code, + api_client.volumes.delete, unique_name) @pytest.mark.p0 diff --git a/harvester_e2e_tests/integrations/test_3_vm_functions.py b/harvester_e2e_tests/integrations/test_3_vm_functions.py index 841f131c9..0d3657f81 100644 --- a/harvester_e2e_tests/integrations/test_3_vm_functions.py +++ b/harvester_e2e_tests/integrations/test_3_vm_functions.py @@ -1698,7 +1698,7 @@ def test_update_vm_machine_type( else: raise AssertionError( f"Failed to create VM({cpu} core, {mem} RAM) with errors:\n" - f"Phase: {data.get('status', {}).get('phase','')}\t" + f"Phase: {data.get('status', {}).get('phase', '')}\t" f"Status: {data.get('status', {})}\n" f"API Status({code}): {data}" )