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.

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

Update keeper module #33

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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: 0 additions & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
[metadata]
name = spamostack
version = 1.0.0
summary = Tool for creating requests to OpenStack with fake drivers
description-file = README.rst
license = Apache-2.0
Expand Down
21 changes: 21 additions & 0 deletions spamostack/keeper.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import logging
import random
import traceback
import uuid

from spam_factory import SpamFactory

Expand Down Expand Up @@ -119,6 +120,10 @@ def get(self, client_name, resource_name, param=None, func=None,
else:
probe = getattr(el, param)(*args, **kwargs)

if (resource_name == "flavors"
Copy link
Member

Choose a reason for hiding this comment

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

I think it's not quite right place to it, that changes affects all the get operations for flavors.
As I remember default flavors have specific names, so what if make a change in main.py on 82 line just for getting that bunch of flavors? That way only default flavors could appear in cache and not the user created flavors if they got here. For getting them you can use even the same admin_keeper.get() operation with filter function for a specific line of flavor names) And after that you can look at how cirros avoid to being deleted by the name in spam_factory.py on 454 line, and do the same :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@FromZeus Hi, thank for review! I think that hardcoding a bunch of names is a wrong way :)
But, also i think that I can move validate_uuid method to spam_factory module, and iterate over list of flavors - and if id of flavor is uuid - we can remove this flavor. This allow us doesn't to break our cloud, and doesn't disrupt logic of keeper.get() method
Reason, why I using uuid validation - because standard flavors creates with custom id (not uuid) and our flavors will be created with auto-generated uuid

Copy link
Member

Choose a reason for hiding this comment

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

Hey :) Hm... What if somebody will create a custom flavor by hands, so that flavor will have uuid too, then while validating of uuid that flavor will be deleted. Is this possible situation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Possibly yes :)
But if we hardcode some parameters, this will be wrong too (from my POV).
As possible solution, we can save current state to db, and duplicate this info for more safety. Or, we can change mechanism with assigning custom id, like spamo-{iteration_number}, and keep resource, if id doesn't start with spamo

Copy link
Member

Choose a reason for hiding this comment

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

Sure, if you could just pass an id which will contain spamo- as prefix and then just check it in spam_factory.py delete method of flavor that would be a better solution :)

Copy link

Choose a reason for hiding this comment

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

I'm agree with the use of the defined prefix for all resources to be able to remove them whenever we want.

Copy link
Member

Choose a reason for hiding this comment

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

All the resources have already a distinguish as a placement to the cache thus they could be cleaned up with spamostack --clean all for example. But flavors are an exception in sense the default flavors placed to cache for initial usage. So there is no need to use that prefix for all the resources or is there?

and not self.validate_uuid4(probe)):
continue

if func(probe):
result.append(el)
except Exception as exc:
Expand Down Expand Up @@ -156,6 +161,22 @@ def get(self, client_name, resource_name, param=None, func=None,

return result

@staticmethod
def validate_uuid4(uuid_string):
"""This method allow us validate uuid4 for keeping all standard flavors

@param uuid_string: UUID, with (or without) slashes
@type uuid_string: `str`, or `object` with uuid representation
"""
uuid_string = uuid_string.replace("-", "")

try:
val = uuid.UUID(uuid_string, version=4)
except ValueError:
return False

return val.hex == uuid_string

def clean(self, component_names):
"""Delete all the resources for specific component

Expand Down
10 changes: 0 additions & 10 deletions spamostack/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,6 @@
import logging


class SpamFileHandler(logging.FileHandler):

def __init__(self):
logging.FileHandler.__init__(self, "RM.log")
fmt = '%(asctime)s %(filename)s %(levelname)s: %(message)s'
fmt_date = '%Y-%m-%dT%T%Z'
formatter = logging.Formatter(fmt, fmt_date)
self.setFormatter(formatter)


class SpamStreamHandler(logging.StreamHandler):

def __init__(self):
Expand Down