Skip to content

Commit

Permalink
Enhance property filter parsing with validation and tests
Browse files Browse the repository at this point in the history
- Add property filter parsing and conversion functions in 'utils.py'.
- Update 'ListNode' to support property filtering with improved logging.
- Add unit tests for parsing, validation, and node filtering.
  • Loading branch information
Surbhi Kanthed committed Jun 20, 2024
1 parent 86ab4eb commit d00fabc
Show file tree
Hide file tree
Showing 9 changed files with 367 additions and 5 deletions.
74 changes: 74 additions & 0 deletions esileapclient/common/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import re
import operator
import logging

# Configure the logger
LOG = logging.getLogger(__name__)
logging.basicConfig(level=logging.INFO)

# Define constants for operator pattern and filter pattern
OPS = {
'>=': operator.ge,
'<=': operator.le,
'>': operator.gt,
'<': operator.lt,
'=': operator.eq,
}

OPERATOR_PATTERN = '|'.join(re.escape(op) for op in OPS.keys())
FILTER_PATTERN = re.compile(rf'([^><=]+)({OPERATOR_PATTERN})(.+)')


def convert_value(value_str):
"""Convert a value string to an appropriate type for comparison."""
try:
return int(value_str)
except ValueError:
try:
return float(value_str)
except ValueError:
return value_str


def parse_property_filter(filter_str):
"""Parse a property filter string into a key, operator, and value."""
match = FILTER_PATTERN.match(filter_str)
if not match:
raise ValueError(f"Invalid property filter format: {filter_str}")
key, op_str, value_str = match.groups()
if op_str not in OPS:
raise ValueError(f"Invalid operator in property filter: {op_str}")
value = convert_value(value_str)
return key.strip(), OPS[op_str], value


def node_matches_property_filters(node, property_filters):
"""Check if a node matches all property filters."""
properties = node.get('resource_properties', node.get('properties', {}))
for key, op, value in property_filters:
if key not in properties:
return False
node_value = convert_value(properties.get(key, ''))
if not op(node_value, value):
return False
return True


def filter_nodes_by_properties(nodes, properties):
"""Filter a list of nodes based on property filters."""
if not properties:
return nodes
property_filters = []
for prop in properties:
try:
property_filters.append(parse_property_filter(prop))
except ValueError as e:
LOG.error(f"Error parsing property filter '{prop}': {e}")
raise

filtered_nodes = [
node for node in nodes
if node_matches_property_filters(node, property_filters)
]

return filtered_nodes
15 changes: 14 additions & 1 deletion esileapclient/osc/v1/lease.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from osc_lib import utils as oscutils

from esileapclient.v1.lease import Lease as LEASE_RESOURCE
from esileapclient.common import utils

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -189,6 +190,15 @@ def get_parser(self, prog_name):
dest='purpose',
required=False,
help="Show the purpose of leasing the node")
parser.add_argument(
'--property',
dest='properties',
required=False,
action='append',
help="Filter offers by properties. Format: 'key>=value'. "
"Can be specified multiple times. "
f"Supported operators are: {', '.join(utils.OPS.keys())}",
metavar='"key>=value"')
return parser

def take_action(self, parsed_args):
Expand All @@ -213,6 +223,9 @@ def take_action(self, parsed_args):

data = list(client.leases(**filters))

filtered_leases = utils.filter_nodes_by_properties(
data, parsed_args.properties)

if parsed_args.long:
columns = LEASE_RESOURCE.long_fields.keys()
labels = LEASE_RESOURCE.long_fields.values()
Expand All @@ -222,7 +235,7 @@ def take_action(self, parsed_args):

return (labels,
(oscutils.get_item_properties(s, columns)
for s in data))
for s in filtered_leases))


class ShowLease(command.ShowOne):
Expand Down
21 changes: 19 additions & 2 deletions esileapclient/osc/v1/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from osc_lib import utils as oscutils

from esileapclient.v1.node import Node as NODE_RESOURCE
from esileapclient.common import utils

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -48,20 +49,36 @@ def get_parser(self, prog_name):
dest='lessee',
required=False,
help="Filter nodes by lessee.")
parser.add_argument(
'--property',
dest='properties',
required=False,
action='append',
help="Filter offers by properties. Format: 'key>=value'. "
"Can be specified multiple times. "
f"Supported operators are: {', '.join(utils.OPS.keys())}",
metavar='"key>=value"')

return parser

def take_action(self, parsed_args):

client = self.app.client_manager.lease

# Initial filters dictionary
filters = {
'resource_class': parsed_args.resource_class,
'owner': parsed_args.owner,
'lessee': parsed_args.lessee
}

data = list(client.nodes(**filters))
# Retrieve all nodes with initial filters
all_nodes = list(client.nodes(**filters))

# Apply filtering based on properties
filtered_nodes = utils.filter_nodes_by_properties(
all_nodes, parsed_args.properties
)

if parsed_args.long:
columns = NODE_RESOURCE.detailed_fields.keys()
Expand All @@ -72,4 +89,4 @@ def take_action(self, parsed_args):

return (labels,
(oscutils.get_item_properties(s, columns)
for s in data))
for s in filtered_nodes))
15 changes: 14 additions & 1 deletion esileapclient/osc/v1/offer.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from esileapclient.v1.lease import Lease as LEASE_RESOURCE
from esileapclient.v1.offer import Offer as OFFER_RESOURCE
from esileapclient.common import utils

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -148,6 +149,15 @@ def get_parser(self, prog_name):
dest='resource_class',
required=False,
help="Show all leases with given resource-class.")
parser.add_argument(
'--property',
dest='properties',
required=False,
action='append',
help="Filter offers by properties. Format: 'key>=value'. "
"Can be specified multiple times. "
f"Supported operators are: {', '.join(utils.OPS.keys())}",
metavar='"key>=value"')

return parser

Expand Down Expand Up @@ -176,6 +186,9 @@ def take_action(self, parsed_args):

data = list(client.offers(**filters))

filtered_leases = utils.filter_nodes_by_properties(
data, parsed_args.properties)

if parsed_args.long:
columns = OFFER_RESOURCE.long_fields.keys()
labels = OFFER_RESOURCE.long_fields.values()
Expand All @@ -185,7 +198,7 @@ def take_action(self, parsed_args):

return (labels,
(oscutils.get_item_properties(s, columns)
for s in data))
for s in filtered_leases))


class ShowOffer(command.ShowOne):
Expand Down
91 changes: 91 additions & 0 deletions esileapclient/tests/unit/common/test_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import unittest
from esileapclient.common import utils


class TestUtils(unittest.TestCase):

def setUp(self):
self.nodes = [
{'properties': {'cpus': '40', 'memory_mb': '131072'}},
{'properties': {'cpus': '80', 'memory_mb': '262144'}},
{'properties': {'cpus': '20', 'memory_mb': '65536'}},
]

def test_convert_value(self):
self.assertEqual(utils.convert_value('10'), 10)
self.assertEqual(utils.convert_value('10.5'), 10.5)
self.assertEqual(utils.convert_value('text'), 'text')

def test_parse_property_filter(self):
key, op, value = utils.parse_property_filter('cpus>=40')
self.assertEqual(key, 'cpus')
self.assertEqual(op, utils.OPS['>='])
self.assertEqual(value, 40)

key, op, value = utils.parse_property_filter('memory_mb<=131072')
self.assertEqual(key, 'memory_mb')
self.assertEqual(op, utils.OPS['<='])
self.assertEqual(value, 131072)

# Test for invalid filter format
self.assertRaisesRegex(
ValueError,
'Invalid property filter format: invalid_filter',
utils.parse_property_filter, 'invalid_filter'
)

# Test for invalid operator
self.assertRaisesRegex(
ValueError,
'Invalid property filter format: cpus!40',
utils.parse_property_filter, 'cpus!40'
)

def test_node_matches_property_filters(self):
filters = [
utils.parse_property_filter('cpus>=40'),
utils.parse_property_filter('memory_mb>=131072')
]
self.assertTrue(utils.node_matches_property_filters(
self.nodes[1], filters))
self.assertFalse(utils.node_matches_property_filters(
self.nodes[2], filters))

# Test for non-existent property
filters = [utils.parse_property_filter('non_existent_property>=100')]
self.assertFalse(utils.node_matches_property_filters(
self.nodes[0], filters))

def test_filter_nodes_by_properties(self):
properties = ['cpus>=40']
filtered_nodes = utils.filter_nodes_by_properties(
self.nodes, properties)
self.assertEqual(len(filtered_nodes), 2)

properties = ['memory_mb<=131072']
filtered_nodes = utils.filter_nodes_by_properties(
self.nodes, properties)
self.assertEqual(len(filtered_nodes), 2)

properties = ['cpus>100']
filtered_nodes = utils.filter_nodes_by_properties(
self.nodes, properties)
self.assertEqual(len(filtered_nodes), 0)

properties = ['cpus<40']
filtered_nodes = utils.filter_nodes_by_properties(
self.nodes, properties)
self.assertEqual(len(filtered_nodes), 1)
self.assertEqual(filtered_nodes[0]['properties']['cpus'], '20')

# Test for error parsing property filter
properties = ['invalid_filter']
with self.assertLogs('esileapclient.common.utils', level='ERROR') as c:
self.assertRaisesRegex(
ValueError,
"Invalid property filter format: invalid_filter",
utils.filter_nodes_by_properties, self.nodes, properties
)
self.assertTrue(any(
"Invalid property filter format: invalid_filter" in message
for message in c.output))
11 changes: 10 additions & 1 deletion esileapclient/tests/unit/osc/v1/fakes.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,16 @@
event_type = 'fake.event'
event_time = "3000-07-01T12"
object_type = 'lease'
node_properties = {'cpu': '40', 'traits': ['trait1', 'trait2']}
node_properties = {
'cpus': '40',
'memory_mb': '131072',
'local_gb': '1200',
'cpu_arch': 'x86_64',
'vendor': 'fake-vendor',
'cpu_model_name': 'fake-model',
'cpu_frequency': '2000.0',
'traits': ['trait1', 'trait2']
}

OFFER = {
'availabilities': json.loads(lease_availabilities),
Expand Down
55 changes: 55 additions & 0 deletions esileapclient/tests/unit/osc/v1/test_lease.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import copy
import json
from osc_lib.tests import utils as osctestutils
from unittest import mock

from esileapclient.osc.v1 import lease
from esileapclient.tests.unit.osc.v1 import base
Expand Down Expand Up @@ -181,6 +182,33 @@ def test_lease_list(self):
),)
self.assertEqual(datalist, tuple(data))

@mock.patch('esileapclient.common.utils.filter_nodes_by_properties')
def test_lease_list_with_property_filter(self, mock_filter_nodes):
arglist = ['--property', 'cpus>=40']
verifylist = [('properties', ['cpus>=40'])]

parsed_args = self.check_parser(self.cmd, arglist, verifylist)
columns, data = self.cmd.take_action(parsed_args)

filters = {
'status': parsed_args.status,
'offer_uuid': parsed_args.offer_uuid,
'start_time': str(parsed_args.time_range[0]) if
parsed_args.time_range else None,
'end_time': str(parsed_args.time_range[1]) if
parsed_args.time_range else None,
'project_id': parsed_args.project_id,
'owner_id': parsed_args.owner_id,
'view': 'all' if parsed_args.all else None,
'resource_type': parsed_args.resource_type,
'resource_uuid': parsed_args.resource_uuid,
'resource_class': parsed_args.resource_class,
'purpose': parsed_args.purpose
}

self.client_mock.leases.assert_called_with(**filters)
mock_filter_nodes.assert_called_with(mock.ANY, parsed_args.properties)

def test_lease_list_long(self):
arglist = ['--long']
verifylist = [('long', True)]
Expand Down Expand Up @@ -242,6 +270,33 @@ def test_lease_list_long(self):
),)
self.assertEqual(datalist, tuple(data))

@mock.patch('esileapclient.common.utils.filter_nodes_by_properties')
def test_lease_list_long_with_property_filter(self, mock_filter_nodes):
arglist = ['--long', '--property', 'memory_mb<=262144']
verifylist = [('long', True), ('properties', ['memory_mb<=262144'])]

parsed_args = self.check_parser(self.cmd, arglist, verifylist)
columns, data = self.cmd.take_action(parsed_args)

filters = {
'status': parsed_args.status,
'offer_uuid': parsed_args.offer_uuid,
'start_time': str(parsed_args.time_range[0]) if
parsed_args.time_range else None,
'end_time': str(parsed_args.time_range[1]) if
parsed_args.time_range else None,
'project_id': parsed_args.project_id,
'owner_id': parsed_args.owner_id,
'view': 'all' if parsed_args.all else None,
'resource_type': parsed_args.resource_type,
'resource_uuid': parsed_args.resource_uuid,
'resource_class': parsed_args.resource_class,
'purpose': parsed_args.purpose
}

self.client_mock.leases.assert_called_with(**filters)
mock_filter_nodes.assert_called_with(mock.ANY, parsed_args.properties)


class TestLeaseShow(TestLease):
def setUp(self):
Expand Down
Loading

0 comments on commit d00fabc

Please sign in to comment.