rockstor / rockstor-core

Linux/BTRFS based Network Attached Storage(NAS)
http://rockstor.com/docs/contribute_section.html
GNU General Public License v3.0
553 stars 137 forks source link

TypeError when deleting a Rocknet #2814

Closed phillxnet closed 6 months ago

phillxnet commented 6 months ago

When deleting for example a docker network named "111", having just created it on a fresh system, using the System - Network (Rockstor 5.0.8-0 testing channel) page:

Network Connections

Name UUID Type State Docker name Connection method IP Address Gateway DNS Servers DNS Search Domains MTU
br-09b408c806a0 1f608a19-b2dc-4bff-a5fc-91a753870932 bridge activated 111 manual 172.18.0.1/16 172.18.0.1     1500

  docker0 | 9ee2ce17-bb38-4db3-b4cd-33306e3384af | bridge | activated | docker0 | manual | 172.17.0.1/16 | 172.17.0.1 |   |   | 1500   Wired connection 1 | e9c4b86c-930e-3294-b9c7-f0aff0b3481f | ethernet | activated

The following error was observed:

[22/Mar/2024 15:36:17] ERROR [storageadmin.util:45] Exception: '>' not supported between instances of 'BridgeConnection' and 'int'
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/rest_framework_custom/generic_view.py", line 41, in _handle_exception
    yield
  File "/opt/rockstor/src/rockstor/storageadmin/views/network.py", line 548, in delete
    if nco.bridgeconnection_set.first() > 0:  # If docker network
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '>' not supported between instances of 'BridgeConnection' and 'int'
FroggyFlox commented 6 months ago

Linking to #2775 as any PR closing this issue should also close #2775.

Hooverdan96 commented 6 months ago

Could this be done with another check whether the set is empty before handling the two cases?

    def delete(self, request, id):
        with self._handle_exception(request):
            nco = self._nco(request, id)
            if nco.bridgeconnection_set: # a bridgeconnection exists
                if nco.bridgeconnection_set.first() > 0:  # If docker network
                    brco = nco.bridgeconnection_set.first()
                    ... 
                    dnet_remove(network=brco.docker_name)
                else:
                    restricted = False
                    ...
                    self._delete_connection(nco)
            else: # no bridgeconnection exists, delete orphaned entry
                self._delete_connection(nco)
phillxnet commented 6 months ago

@Hooverdan96 I'm going to have a look at this and it's linked issue shortly if all goes well. Re:

Could this be done with another check whether the set is empty before handling the two cases?

It may fix the linked issue #2775

TypeError: ‘>’ not supported between instances of ‘NoneType’ and ‘int’

though we may need an .exists().

But I suspect not the failure in this issue. I'll have to brush up what was intended here in the first place. Then likely yes we just need an additional check of sorts.

FroggyFlox commented 6 months ago

If I remember correctly, this is only there to check if the NetworkConnection object has a related BridgeConnection, in which case it means it has docker networks that need to be disconnected from their containers. I remember taking this construct from elsewhere (within the same class maybe?) so I hope there isn't an issue with these other instances as well.

phillxnet commented 6 months ago

Reproducer Rocknet expanded: undeletable-rocknet

phillxnet commented 6 months ago

@FroggyFlox & @Hooverdan96 Just looking at a simple fix by way of replacing what looks like it should have been .count() > 0 with a basic query .exists().

Take a look at the associated PR. Now testing to see if this also fixes the missing network card issue originally created by @Hooverdan96, and linked to by @FroggyFlox :

"Cannot delete physically absent network card entry using WebUI" #2775

Which is a little more tricky to setup a reproducer for.

phillxnet commented 6 months ago

Closing as: Fixed by #2817