How to make sense of complex nested loops in python aka as "am I doing this right?"

user_17864222 0
Viewed 46

My program is designed to get a bunch of different objects from an API and manipulate them, then via loops (or some other method) and verify that some expected conditions exist and report when they do not.

More specifically, I'm checking that for each container in a docker swarm, the ports are open as per the service definition, on each worker on which the container runs.

I won't supply the whole program, but here are examples of the objects (cut down in size for posterity). Also be aware I've anonomised the data so there won't be matches where there ordinarily would be:

nodes_simple: (A list of docker worker nodes)

[
    {
        "NodeID": "aaaaaaaaaaaaaaaaa_worker3",
        "Hostname": "worker03"
    },
    {
        "NodeID": "bbbbbbbbbbbbbbbbb_worker2",
        "Hostname": "worker02"
    },
]

services_simple: (A list of Docker swarm services)

[
    {
        "ServiceID": "asdfadsfadsfsdafadssadf",
        "Name": "service1",
        "Ports": [
            {
                "Port": 31900,
                "Protocol": "tcp"
            },
            {
                "Port": 31901,
                "Protocol": "tcp"
            }
        ]
    },
    {
        "ServiceID": "dfadsasffafgergerg",
        "Name": "service2",
        "Ports": [
            {
                "Port": 30961,
                "Protocol": "tcp"
            },
            {
                "Port": 30965,
                "Protocol": "tcp"
            }
        ]
    }
]

tasks_simple: (A list of Docker swarm "tasks" - running containers, basically)

[
    {
        "ID": "afsdfsdfadsfasdfdsfdsa",
        "ServiceID": "sdafasdfdsfsadfasdfa",
        "NodeID": "asdfadsfasdfadsfadsfasdf",
        "ContainerID": "gfsgadfgadffgdfgkdfhgikldjfijdajgiodaerjopijgjerjgidfjijgfioajdfg"
    },
    {
        "ID": "afgadsfrgdfgadfgafdgadf",
        "ServiceID": "asdggdfdgfdgfdgagdfgaga",
        "NodeID": "ghjhdgyjhyjtyjtyjdtyj",
        "ContainerID": "rgaegergaer876g8a7r6g897er6er6g76ar7e6ga786gre78a6rg786aeergaegre"
    },
    {
        "ID": "agrggregrarggegergegr",
        "ServiceID": "agdragregergagergergger",
        "NodeID": "agregergrenrthjkfkhukujhku",
        "ContainerID": "fd6fds7a6g7ds78fgvfdf7a6g6df87g678afd6g7f6adf876g7dfsa6786g87a6g7a"
    }
]

worker_iptables: (A list of dicts representing open ports on each Docker swarm node - taken from iptables)

[{'host': 'worker01',
  'ports': [{'Port': '32555', 'Protocol': 'tcp'},
            {'Port': '32505', 'Protocol': 'tcp'},
            {'Port': '32555', 'Protocol': 'tcp'},
            {'Port': '32505', 'Protocol': 'tcp'},
            {'Port': '32235', 'Protocol': 'tcp'},
            {'Port': '32235', 'Protocol': 'tcp'},
            {'Port': '32655', 'Protocol': 'tcp'},
            {'Port': '32655', 'Protocol': 'tcp'}]},
 {'host': 'worker02',
  'ports': [{'Port': '34553', 'Protocol': 'tcp'},
            {'Port': '34503', 'Protocol': 'tcp'},
            {'Port': '34553', 'Protocol': 'tcp'},
            {'Port': '34503', 'Protocol': 'tcp'},
            {'Port': '34233', 'Protocol': 'tcp'},
            {'Port': '34233', 'Protocol': 'tcp'},
            {'Port': '34653', 'Protocol': 'tcp'},
            {'Port': '34653', 'Protocol': 'tcp'}]}]

I first manipulate the tasks_simple object and run a print line to help me make sense of it:

    # Loop through each task.
    # Replace NodeID with Node Name
    # Replace ServiceID with Service Name
    # Add in ports as per service to each task
    for task in tasks_simple:
        for node in nodes_simple:
            if task['NodeID'] == node['NodeID']:
                task['NodeID'] = node['Hostname']
        for service in services_simple:
            if task['ServiceID'] == service['ServiceID']:
                task['ServiceID'] = service['Name']
                task['Ports'] = service['Ports']
        # If the task has ports
        if 'Ports' in task:
            print(f"Task {task['TaskID']} of Service {task['ServiceID']} should have Ports {task['Ports']} open on Worker {task['NodeID']}")

This manipulates the tasks_simple to look like:

[{'ContainerID': 'ew4f68we46we46f54we654f65we4f65we45ergsergdrgrdzgzdrgdgzdrgdrgdrg',
  'NodeID': 'worker02',
  'Ports': [{'Port': 30230, 'Protocol': 'tcp'},
            {'Port': 30231, 'Protocol': 'tcp'}],
  'ServiceID': 'service1',
  'TaskID': 'sdfgdfgsdfgdsfgdsghgsfghfg'},
 {'ContainerID': 'fdvgb87df6876a8d7sf68df6gv6df7g67fda67g6df7g6adf6g7a867fd6g7df6fg7',
  'NodeID': 'worker02',
  'Ports': [{'Port': 30650, 'Protocol': 'tcp'},
            {'Port': 30651, 'Protocol': 'tcp'}],
  'ServiceID': 'service2',
  'TaskID': 'fgadfgdfgdfgsdgdsfgsfdg'},
 {'ContainerID': 'adrfgdfg78df87g6df87g6d78f6ga876df78g6a7df6g78d6d78f6g7dfd7g7fd6gfd',
  'NodeID': 'worker02',
  'Ports': [{'Port': 31010, 'Protocol': 'tcp'},
            {'Port': 31011, 'Protocol': 'tcp'}],
  'ServiceID': 'service3',
  'TaskID': 'sgfhftghfghfghgfghshgfshfgh'},
 {'ContainerID': '3d341ab586a072cc1034cc85b97c8a45154f0a18ba1c16ba02db36954400b7c3',
  'NodeID': 'worker03',
  'ServiceID': 'service4',
  'TaskID': '62kza6a2ylyf3sm2v3wb8ieey'},
 {'ContainerID': 'fa8b6e2cb22049ed498bae27907fbb03ac096aee8c21c7005d28b2aa3bad69d0',
  'NodeID': 'worker02',
  'Ports': [{'Port': 30910, 'Protocol': 'tcp'},
            {'Port': 30911, 'Protocol': 'tcp'}],
  'ServiceID': 'service5',
  'TaskID': '6vq6ck2fb106xs8nv3kdhf2d3'},
 {'ContainerID': 'sthfthfthtfhfhfsthshtfshftrfhtfstrtttstgf78fd6g77fg6d7fg787fdf78d',
  'NodeID': 'worker02',
  'Ports': [{'Port': 31090, 'Protocol': 'tcp'}],
  'ServiceID': 'helloworld-http',
  'TaskID': '9t2fhl99o1e92p86hyhs1rjl6'}]

And prints output such as the following:

Task 4btbm94dd4xqad72ftpi9herv of Service service1 should have Ports [{'Port': 30230, 'Protocol': 'tcp'}, {'Port': 30231, 'Protocol': 'tcp'}] open on Worker worker02
Task 4qadgitrhranrs52zqujb0jdt of Service service2 should have Ports [{'Port': 30650, 'Protocol': 'tcp'}, {'Port': 30651, 'Protocol': 'tcp'}] open on Worker worker02
Task 5yzf2if8bf0qnsiomho3h4qtj of Service service3 should have Ports [{'Port': 31010, 'Protocol': 'tcp'}, {'Port': 31011, 'Protocol': 'tcp'}] open on Worker worker02
Task 6vq6ck2fb106xs8nv3kdhf2d3 of Service service4 should have Ports [{'Port': 30910, 'Protocol': 'tcp'}, {'Port': 30911, 'Protocol': 'tcp'}] open on Worker worker02
Task 9t2fhl99o1e92p86hyhs1rjl6 of Service helloworld-http should have Ports [{'Port': 31090, 'Protocol': 'tcp'}] open on Worker worker02
Task c3w7dkey89nb4s1lfjlkm8rad of Service service5 should have Ports [{'Port': 31500, 'Protocol': 'tcp'}] open on Worker worker03

At this point, I'm a bit lost and my comment

# Check that each port for the task is open on the worker which the task is running on

Is as far as I can get, having tried some super confusing loops and failed horribly. How can I "simply" check each port for a service is open on the worker(s) for which the container(s) are running on?

This is to get a handle on a pretty nasty Docker UCP/MKE bug where swarm ports that should be open on workers where containers are running suddenly aren't, so we can redeploy those services, which seems to fix the problem.

docker-swarm python loops
2 Answers

The main problem is that your data is organized in an extremely inconvenient way; when this is the case, you should always consider creating new data structures to make it more convenient. (Creating a new structure is usually better than changing the shape of an existing structure; if nothing else it's less confusing.) Nested loops like this:

    for task in tasks_simple:
        for node in nodes_simple:
            if task['NodeID'] == node['NodeID']:
                task['NodeID'] = node['Hostname']
        for service in services_simple:
            if task['ServiceID'] == service['ServiceID']:
                task['ServiceID'] = service['Name']
                task['Ports'] = service['Ports']

can often be made easier (and more efficient) by creating lookup tables first:

nodes_by_id = {node['NodeID']: node for node in nodes_simple}
services_by_id = {service['ServiceID']: for service in services_simple}
workers_by_host = {worker['host']: worker for worker in worker_iptables}

and then using those in a simpler loop:

for task in tasks_simple:
    node = nodes_by_id[task['NodeID']]
    service = services_by_id[task['ServiceID']]
    worker = workers_by_host[node['Hostname']]

Now that we know the service and worker for the task, we can tackle the thing in your comment, easy-peasy. Again, we'll start with creating data structures that will make our life easy; in this case we'll create sets of the ports we're interested in:

    # Check that each port for the task is open 
    # on the worker which the task is running on
    open_ports = {port['Port'] for port in worker['ports']}
    service_ports = {port['Port'] for port in service['Ports']}

and then the check is just:

    if service_ports.issubset(open_ports):
        print("yarp")
user_37997590

So in the end, with help from @Samwise, this is what met the need:

def check_ports(tasks_simple, nodes_by_id, services_by_id, nodes_by_host):
    """Check swarm ports are open on each node for each task (container)"""

    for task in tasks_simple:
        task_node = nodes_by_id[task['NodeID']]
        service = services_by_id[task['ServiceID']]
        node = nodes_by_host[task_node['Hostname']]

        # Check that each port for the task is open
        # on the node which the task is running on
        open_ports = {port['Port'] for port in node['ports']}

        # Ignore services without ports
        if service['Ports'] is not None:
            service_ports = {str(port['Port']) for port in service['Ports']}
            if service_ports.issubset(open_ports):
                # Then the ports for the task are open on the node
                pass
            else:
                # Some ports aren't open where they should be
                print(
                    f"Service {service['Name']} "
                    f"has container {task['ContainerID']} "
                    f"running on {task_node['Hostname']} "
                    f"but at least one of the ports {service_ports} "
                    f"isn't open on that node"
                )


def main():

[SNIP]

    # Simplify nodes and services for simpler lookups
    nodes_by_id = {node['NodeID']: node for node in nodes_simple}
    services_by_id = {service['ServiceID']: service
                      for service in services_simple}
    nodes_by_host = {node['host']: node for node in nodes_iptables}

    # Check if swarm ports for each running
    # container are open on the nodes they run on
    check_ports(tasks_simple, nodes_by_id, services_by_id, nodes_by_host)

    sys.exit(0)


if __name__ == "__main__":
    main()

The reasons for the added complexity were mostly around some tasks not having ports, and therefore needing to avoid an not-indexable NoneType. Forgive me also, for having changed some variable names.

Additionally, in the suggested fix above from @Samwise, I needed to cast port['Port'] in task in tasks_simple to str as it was originally created as int. I also needed to widen my tested nodes from just swarm workers to swarm workers + swarm managers, as some tasks run containers on the manager nodes.

user_178642220
Related