travisghansen / hass-opnsense

OPNsense integration with Home Assistant
Apache License 2.0
227 stars 29 forks source link

Upgraded, Now I Have Two Routers? #215

Closed wormuths closed 1 month ago

wormuths commented 1 month ago

I upgraded the plugin, and it created a duplicate device. Now I have my OPNSense router twice, with a ton of unavailable entities.

alexdelprete commented 1 month ago

Yes, happened to me too, sorry about that.

Delete both instances and readd the integration. Many changes are happening, @Snuffy2 is doing a hard work and this project is getting all the love it deserves.

wormuths commented 1 month ago

Hey there...

I removed both, uninstalled, rebooted, re-installed, rebooted and added it back. It left me with only 22 entities being provided. So many things were missing that I restored to before the re-install and I'm back where I was.

I have to leave it like this until it's providing everything I used to have. I have graphs for interface stats, automations that run depending on traffic activity, etc... I appreciate the work going into it, but I need to keep upgrading in place and hope this irons out.

wormuths commented 1 month ago

If you all need help with any information along the way, of course reach out. Between the duplicate instances right now I have 262 entities. I'm sure some are duplicates. Right now the second (duplicate) instance has 96 Controls, Sensors and Diagnostics...

If I had one comment right now, I'd say that it's VERY important to try and name the entities the same on rewrite. This way, I (and others) can just remove/reinstall and be back in business...

alexdelprete commented 1 month ago

It left me with only 22 entities being provided. So many things were missing that I restored to before the re-install and I'm back where I was.

Many entities are disabled by default, you need to enable them. I only enable what's needed for my use-case, and it makes sense to have most of them disabled.

Controls: image

Sensors: image

Snuffy2 commented 1 month ago

I've also had the issue a couple of times where the integration has created a second instance. This has happened in versions before and after v0.2.0 to me. I suspect it has to do with the way the device id is created and used. I'd like to improve this but I'm still brainstorming how best to do it without breaking existing installs.

Totally agree that with all of these updates that we don't rename the entities when possible. It was unavoidable for the Service switches and OpenVPN sensors. The rest should have maintained their same names with all of the updates from v0.2.0+

It is interesting that you noted on reinstall you ended up with only 22 entities. I had the same issue last night (see #220). At least in my case it was because of a bad URL for OPNsense, but I can't reproduce it. Either way, I'll improve the validation in config_flow to try to prevent that from happening again. If you have logs from the reinstall with 22 entities, please pass it along.

alexdelprete commented 1 month ago

Weird. This afternoon a second device with only sensors was created. Obviously all duplicated sensors. I agree it's something related to how the device_id is created. I have an idea: since device_id has to be unique, if OPNsense API doesn't provide a unique sn for the device, we could use the MAC address or something related to a hw unique s/n (CPU?). Just brainstorming...I'll think about it...

I must also say that this didn't happen until last week.

This is one of the duplicated sensors (sensor.opnsense_cpu_usage_2) that was created with the second device this afternoon...nothing was changed and nobody home. :)

image

wormuths commented 1 month ago

Yeah, I got two instances now. Deleted the "unavailable" entities, and the rest are remaining disabled. I'm still using the instance from the original, pre-upgrade just fine with the duplicate in place. I'm just not touching anything at the moment because the original is still working. As for logs, I don't have any errors relating to this that I can see.

And it looks like I just noticed that HACS doesn't let you see/do anything else until you perform the available upgrade. So leaving this un-upgraded disables HACS? Is that new? It's not the fault of this, but that's a terrible design change in HA unless I'm missing something...

c3p0vsr2d2 commented 1 month ago

I removed the integration, rebooted, and then readded it. I see some switches which were provided by the older integration version are missing (I did check disabled switches as well). For example, Wireguard VPN service switch is missing, even though there are sensor entities indicating state. What gives?

oblivioncth commented 1 month ago

Just wanted to note that this still happened to me after updating to 0.3.5, as the above PR implies it was resolved, in case more work should be done.

Logger: homeassistant.config_entries
Source: config_entries.py:933
First occurred: 12:55:29 PM (1 occurrences)
Last logged: 12:55:29 PM

Error migrating entry Router for opnsense
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 933, in async_migrate
    result = await component.async_migrate_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/opnsense/__init__.py", line 315, in async_migrate_entry
    new_dev = device_registry.async_update_device(
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/device_registry.py", line 1027, in async_update_device
    new_values["identifiers"] = self._validate_identifiers(
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/device_registry.py", line 1133, in _validate_identifiers
    raise DeviceIdentifierCollisionError(identifiers, existing_device)
homeassistant.helpers.device_registry.DeviceIdentifierCollisionError: Identifiers {('opnsense', '00_d0_4c_10_27_c0')} already registered with DeviceEntry(area_id='office', config_entries={'37a1a3eb0b3f61041c8d92eb4be590f1'}, configuration_url='https://192.168.1.1', connections=set(), created_at=datetime.datetime(1970, 1, 1, 0, 0, tzinfo=datetime.timezone.utc), disabled_by=None, entry_type=None, hw_version=None, id='2032880a78e66bf6bdaa7c17fcc38e16', identifiers={('opnsense', '00_d0_4c_10_27_c0')}, labels=set(), manufacturer='Deciso B.V.', model='OPNsense', model_id=None, modified_at=datetime.datetime(2024, 10, 13, 16, 55, 29, 78952, tzinfo=datetime.timezone.utc), name_by_user=None, name='Router', primary_config_entry=None, serial_number=None, suggested_area=None, sw_version='23.1.5_4', via_device_id=None, is_new=False)

I'll just reinstall the integration.

wormuths commented 1 month ago

So I had to uninstall/reinstall the integration as well to get rid of the duplicated servers. Once I did, it correctly only created one server, and seems to be running fine.

The only thing I've noticed so far was that the interface entity names now use what I labeled them, instead of "OPT1", "OPT2", etc... I need to find everywhere I used those names and correct the elements in home assistant, but a small price to pay for some maintenance on this critical integration.

Thanks for all the hard work @alexdelprete and @Snuffy2 ... IT IS greatly appreciated.

Snuffy2 commented 1 month ago

Glad you got everything working.

I should have clarified with v0.3.5 better. It unfortunately wasn't going to remove duplicate devices but should prevent it from happening going forward.

alexdelprete commented 1 month ago

Thanks for all the hard work @alexdelprete and @Snuffy2 ... IT IS greatly appreciated.

99.9% of credit goes to the devs: @travisghansen and @Snuffy2. I'm just helping them, and the only credit I take was convincing Travis to create an integration also for OPNsense (he made one for pfSense). Now I simply help and brainstorm with them. :)

wormuths commented 3 weeks ago

99.9% of credit goes to the devs: @travisghansen and @Snuffy2. I'm just helping them, and the only credit I take was convincing Travis to create an integration also for OPNsense (he made one for pfSense). Now I simply help and brainstorm with them. :)

Absolutely. I was using the pfSense integration @alexdelprete made. When pfSense decided to abandon the "Community Edition" I switched to OPNSense. So I whole-heartedly support most effort going into this integration. LOL

To put a bullet point on my appreciation, I use this integration not only to monitor service status and traffic, but it is used all over to enhance integrations. For example, only do a scheduled speed test when you're not streaming (traffic is quiet). This, and Home Assistant, truthfully is my network monitoring solution. I couple this with a custom integration for my Linksys Velop nodes, and with the help of smart plugs, I reboot troubled nodes that lose connectivity and such.

This integration is awesome, and I do appreciate everything that goes into it, so thank you! It keeps the wife from yelling "what's wrong with the internet". LOL

wormuths commented 3 weeks ago

Glad you got everything working.

I should have clarified with v0.3.5 better. It unfortunately wasn't going to remove duplicate devices but should prevent it from happening going forward.

Working great now, thank you!

alexdelprete commented 3 weeks ago

I was using the pfSense integration @alexdelprete made

You meant @travisghansen. Luckily I managed to convince him to do it also for OPNsense. :)

wormuths commented 3 weeks ago

I was using the pfSense integration @alexdelprete made.

You meant @travisghansen. Luckily I managed to convince him to do it also for OPNsense. :)

Oh, yeah... LOL Thanks @travisghansen !