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

TypeError when deleting a Rocknet #2814

Closed
phillxnet opened this issue Mar 22, 2024 · 7 comments
Closed

TypeError when deleting a Rocknet #2814

phillxnet opened this issue Mar 22, 2024 · 7 comments
Assignees

Comments

@phillxnet
Copy link
Member

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
Copy link
Member

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

@Hooverdan96
Copy link
Member

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
Copy link
Member Author

@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
Copy link
Member

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 phillxnet self-assigned this Mar 26, 2024
@phillxnet
Copy link
Member Author

Reproducer Rocknet expanded:
undeletable-rocknet

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Mar 26, 2024
Check for db query _set existence using build-in .exists().
@phillxnet
Copy link
Member Author

@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
Copy link
Member Author

Closing as:
Fixed by #2817

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants