Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue/662 default storage class #745

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Issue/662 default storage class #745

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6cb6530
compiled new requirements.txt
Aug 29, 2024
16dcee6
added makdirs check in helper.py
Aug 29, 2024
e8ea514
deleted gen_sonsobuoy
Sep 2, 2024
22061f9
re-compiled requirements
Sep 2, 2024
10bf140
test successful
Sep 3, 2024
f021a04
re-added gen_sonobuoy_result_file
Sep 4, 2024
9cc32ce
re-added gen_sonobuoy_result_file
Sep 4, 2024
9952be3
added cleanup() as subfunction to check_default_persistentvolumeclaim…
Sep 9, 2024
7e9061a
added cleanup() as subfunction to check_default_persistentvolumeclaim…
Sep 9, 2024
92093b0
rebased
Sep 9, 2024
91678b1
resolved sonobuoy filename issue
Oct 2, 2024
8901137
Default Retries adjustable on module level
Oct 2, 2024
3afa301
cleanup on module level and cleanup on ressource conflict
Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
**/__pycache__/
.venv/
.idea
.sandbox
.DS_Store
node_modules
Tests/kaas/results/
Expand Down
20 changes: 9 additions & 11 deletions Tests/kaas/k8s-default-storage-class/helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import sys
import logging
from kubernetes import client, config
import os

manual_result_file_template = {
"name": None,
Expand Down Expand Up @@ -38,7 +39,6 @@ def __init__(self, *args, return_code: int):


def setup_k8s_client(kubeconfigfile=None):

if kubeconfigfile:
logger.debug(f"using kubeconfig file '{kubeconfigfile}'")
config.load_kube_config(kubeconfigfile)
Expand All @@ -56,19 +56,17 @@ def setup_k8s_client(kubeconfigfile=None):


def gen_sonobuoy_result_file(error_n: int, error_msg: str, test_file_name: str):

test_name = test_file_name.replace(".py", "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test_name = test_file_name.replace(".py", "")
test_name = test_file_name.removesuffix(".py")


test_status = "passed"

if error_n != 0:
test_status = test_name + "_" + str(error_n)
result_file = manual_result_file_template
result_file["name"] = test_name
result_file["status"] = test_status
result_file["details"]["messages"] = error_msg

result_file = manual_result_file_template

result_file["name"] = test_name
result_file["status"] = test_status
result_file["details"]["messages"] = error_msg
directory_path = os.path.dirname(f"./{test_name}.result.yaml")
os.makedirs(directory_path, exist_ok=True)

with open(f"./{test_name}.result.yaml", "w") as file:
yaml.dump(result_file, file)
with open(f"./{test_name}.result.yaml", "w") as file:
yaml.dump(result_file, file)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still creates a file hierarchy replicating the absolute path to the script file, e.g. for me it creates:

/home/martinmorgenstern/workspace/scs/standards/Tests/home/martinmorgenstern/workspace/scs/standards/Tests/kaas/k8s-default-storage-class

The reason is that __file__ (which is the absolute path to the script) is used as the test_file_name argument passed to gen_sonobuoy_result_file and used without further processing.

In main(), you should use os.path.basename(__file__) to get only the script filename without the directories.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes you're right. ithought i already changed that

28 changes: 16 additions & 12 deletions Tests/kaas/k8s-default-storage-class/k8s-default-storage-class-check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@


def check_default_storageclass(k8s_client_storage):

api_response = k8s_client_storage.list_storage_class(_preload_content=False)
storageclasses = api_response.read().decode("utf-8")
storageclasses_dict = json.loads(storageclasses)
Expand Down Expand Up @@ -81,6 +80,15 @@ def check_default_persistentvolumeclaim_readwriteonce(k8s_api_instance, storage_
4. Delete resources used for testing
"""

# 0. Define CleanUp
def cleanup():
logger.debug(f"delete pod:{pod_name}")
api_response = k8s_api_instance.delete_namespaced_pod(pod_name, namespace)
logger.debug(f"delete pvc:{pvc_name}")
api_response = k8s_api_instance.delete_namespaced_persistent_volume_claim(
pvc_name, namespace
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please move the cleanup() function to the module level and use function arguments
  • The api_response is not checked. If that's intended, leave it out. But it might be better to check it and report to the user.
  • Also note that, during cleanup, exceptions could occur, which are currently not handled.


namespace = "default"
pvc_name = "test-pvc"
pv_name = "test-pv"
Expand Down Expand Up @@ -135,10 +143,9 @@ def check_default_persistentvolumeclaim_readwriteonce(k8s_api_instance, storage_
pod_info = json.loads(api_response.read().decode("utf-8"))
pod_status = pod_info["status"]["phase"]

# Check if pod is up and running:
# Check if pod is up and running: Default Retries 30
fraugabel marked this conversation as resolved.
Show resolved Hide resolved
retries = 0
while pod_status != "Running" and retries <= 30:

api_response = k8s_api_instance.read_namespaced_pod(
pod_name, namespace, _preload_content=False
)
Expand All @@ -152,6 +159,7 @@ def check_default_persistentvolumeclaim_readwriteonce(k8s_api_instance, storage_
if pod_status != "Running":
raise SCSTestException(
"pod is not Running not able to setup test Enviornment",
cleanup(),
fraugabel marked this conversation as resolved.
Show resolved Hide resolved
return_code=13,
)

Expand All @@ -172,28 +180,24 @@ def check_default_persistentvolumeclaim_readwriteonce(k8s_api_instance, storage_
if pv["status"]["phase"] != "Bound":
raise SCSTestException(
"Not able to bind pv to pvc",
cleanup(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

return_code=41,
)

if "ReadWriteOnce" not in pv["spec"]["accessModes"]:
raise SCSTestException(
"access mode 'ReadWriteOnce' is not supported",
cleanup(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

return_code=42,
)

# 4. Delete resources used for testing
logger.debug(f"delete pod:{pod_name}")
api_response = k8s_api_instance.delete_namespaced_pod(pod_name, namespace)
logger.debug(f"delete pvc:{pvc_name}")
api_response = k8s_api_instance.delete_namespaced_persistent_volume_claim(
pvc_name, namespace
)
cleanup()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are four places were a cleanup() call is inserted, but there should be one central place.

My suggestion would be to make use of the try ... finally construct, doing the cleanup in the finally block.

Or you could build a context manager that supports the with statement. An example of this can be found with the TestEnvironment class in the IaaS entropy check (usage).

Copy link
Contributor Author

@fraugabel fraugabel Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, i'll checkout the context manager, that seems to be pretty neat. i also intend to make a precheck, whether the environment is cleaned up or whether, there are left overs


return 0


def main(argv):

initialize_logging()
return_code = 0
return_message = "return_message: FAILED"
Expand Down Expand Up @@ -226,7 +230,7 @@ def main(argv):
return_message = f"{exception_message}"
return_code = 1

# Check if default storage class is defined (MENTETORY)
# Check if default storage class is defined (MANDATORY)
try:
logger.info("check_default_storageclass()")
default_class_name = check_default_storageclass(k8s_storage_api)
Expand All @@ -239,7 +243,7 @@ def main(argv):
return_message = f"{exception_message}"
return_code = 1

# Check if default_persistent volume has ReadWriteOnce defined (MENTETORY)
# Check if default_persistent volume has ReadWriteOnce defined (MANDATORY)
try:
logger.info("check_default_persistentvolume_readwriteonce()")
return_code = check_default_persistentvolumeclaim_readwriteonce(
Expand Down
3 changes: 3 additions & 0 deletions Tests/requirements.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
aiohttp
click
fabric
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fabric

This seems like a leftover from the rebase. The fabric dependency has been removed in main via 18a3dae.

Please regenerate the requirements.txt again afterwards.

kubernetes
kubernetes_asyncio
python-dateutil
PyYAML
openstacksdk
requests
tomli

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

33 changes: 33 additions & 0 deletions Tests/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,13 @@ async-timeout==4.0.3
# via aiohttp
attrs==24.2.0
# via aiohttp
bcrypt==4.2.0
# via paramiko
cachetools==5.5.0
# via google-auth
certifi==2024.7.4
# via
# kubernetes
# kubernetes-asyncio
# requests
cffi==1.17.0
Expand All @@ -38,6 +43,8 @@ frozenlist==1.4.1
# via
# aiohttp
# aiosignal
google-auth==2.34.0
# via kubernetes
idna==3.7
# via
# requests
Expand All @@ -54,6 +61,8 @@ jsonpointer==3.0.0
# via jsonpatch
keystoneauth1==5.7.0
# via openstacksdk
kubernetes==30.1.0
# via -r requirements.in
kubernetes-asyncio==30.3.1
# via -r requirements.in
multidict==6.0.5
Expand All @@ -62,6 +71,10 @@ multidict==6.0.5
# yarl
netifaces==0.11.0
# via openstacksdk
oauthlib==3.2.2
# via
# kubernetes
# requests-oauthlib
openstacksdk==3.3.0
# via -r requirements.in
os-service-types==1.7.0
Expand All @@ -76,25 +89,40 @@ pbr==6.0.0
# stevedore
platformdirs==4.2.2
# via openstacksdk
pyasn1==0.6.0
# via
# pyasn1-modules
# rsa
pyasn1-modules==0.4.0
# via google-auth
pycparser==2.22
# via cffi
python-dateutil==2.9.0.post0
# via
# -r requirements.in
# kubernetes
# kubernetes-asyncio
pyyaml==6.0.2
# via
# -r requirements.in
# kubernetes
# kubernetes-asyncio
# openstacksdk
requests==2.32.3
# via
# -r requirements.in
# keystoneauth1
# kubernetes
# requests-oauthlib
requests-oauthlib==2.0.0
# via kubernetes
requestsexceptions==1.4.0
# via openstacksdk
rsa==4.9
# via google-auth
six==1.16.0
# via
# kubernetes
# kubernetes-asyncio
# python-dateutil
stevedore==5.2.0
Expand All @@ -107,7 +135,12 @@ typing-extensions==4.12.2
# via dogpile-cache
urllib3==2.2.2
# via
# kubernetes
# kubernetes-asyncio
# requests
websocket-client==1.8.0
# via kubernetes
wrapt==1.16.0
# via deprecated
yarl==1.9.4
# via aiohttp