bb-Ricardo / netbox-sync

Sync objects from VMware or redfish sources to NetBox
MIT License
290 stars 65 forks source link

Netbox sync does not remove tags from object #359

Closed Daviran closed 9 months ago

Daviran commented 1 year ago

Hello @bb-Ricardo , amazing job with your script it saved me a lot of time at work.

I wonder if the script removes the tags from Netbox after it was deleted on Vcenter.

I've tried to : create a custom tag on V-center, applied it to a VM, then run the netbox-sync. It works like a charm. However, if I erase the tag in V-center from the VM, when I run back netbox-sync the tag is not removed in Netbox.

If you have already implemented this feature, can you point the direction?

Again, thanks with your amazing work

bb-Ricardo commented 1 year ago

Hi @Daviran,

Thank you very much.

Very good point. This is not implemented due to following reasons: Tags are a sort of shared resource. If a tag is added from a vCenter it does not define where it comes from in NetBox. Even though another user just added a contribution to add the tag source in the comment of that tag. But if this tag is then used further on with different resources which might not be related with the vCenter the tag came from then it exceeded its former scope. This is all fine. It only gets difficult if we want to delete it. If the tag is now used for more objects then the once where it came from then we would cause some people quite some headaches if we would straight delete it because it is not present in the vCenter anymore. That's why it is currently left to the users to cleanup old tags as they will know best if it's still relevant or not.

What do you think?

rizlas commented 1 year ago

Hi, good point Ricardo. Maybe an additional setting True/False is the best option and let netbox-sync admin to choose the behavior. Even tough you should apply the following: "if a thing is automated, do not touch it if you don't know what you are doing"

Of course, deletion should occur only when tag has 0 associated resources, even if the setting is True.

With your explanation you already have the comment for the setting 🤣

bb-Ricardo commented 11 months ago

Hi @rizlas,

This is a great idea. I added pruning for unused tags in the development branch. Would you be able to test this?

Thank you.

rizlas commented 11 months ago

On it during this week

rizlas commented 11 months ago

Hi, pruning is ok. There still a thing tho. Tags could be shared between instances. E.g.: "storage" tag in vCenterA and vCenterB.

With this description:

tag_description = f"{primary_tag_name} {self.name}: "\
f"{self.tag_session.tagging.Tag.get(tag_id).description}"

When vCenterA sync starts, "storage" tag description will be:

"NetBox-synced vCenterA: "

When vCenterB sync starts, "storage" tag description will be:

"NetBox-synced vCenterB: "

I think we can find a solution to handle multiple self.name or add a simple "NetBox-synced" as prefix. Also i don't like the semicolon without anything after that, but maybe it's just me 😄

rizlas commented 11 months ago

I forgot the main thing!

However, if I erase the tag in V-center from the VM, when I run back netbox-sync the tag is not removed in Netbox.

This is still happening. Tag was not removed from VirtualMachine object.

bb-Ricardo commented 11 months ago

I forgot the main thing!

However, if I erase the tag in V-center from the VM, when I run back netbox-sync the tag is not removed in Netbox.

This is still happening. Tag was not removed from VirtualMachine object.

Good point, currently tags are not compared backwards, will look into it.

bb-Ricardo commented 11 months ago

Hi, pruning is ok. There still a thing tho. Tags could be shared between instances. E.g.: "storage" tag in vCenterA and vCenterB.

With this description:

tag_description = f"{primary_tag_name} {self.name}: "\
f"{self.tag_session.tagging.Tag.get(tag_id).description}"

When vCenterA sync starts, "storage" tag description will be:

"NetBox-synced vCenterA: "

When vCenterB sync starts, "storage" tag description will be:

"NetBox-synced vCenterB: "

I think we can find a solution to handle multiple self.name or add a simple "NetBox-synced" as prefix. Also i don't like the semicolon without anything after that, but maybe it's just me 😄

good point as well, we should just remove the source name and only state that these tags are handled by netbox-sync. What do you think?

rizlas commented 11 months ago

good point as well, we should just remove the source name and only state that these tags are handled by netbox-sync. What do you think?

Yeah, seems reasonable enough. Thank you

bb-Ricardo commented 9 months ago

Hi @rizlas,

I fixed the issue with the tag description and that it was not remove. Can you check if it works properly now.

Also the tag might only be removed on the second run:

The script is querying the associations, but this only at the beginning. At the beginning of the first run the tag still has assigned objects, on the second run the counter should be 0 and therefore the tag should be removed from NetBox.

If this all works then feel free to close this issue.

Thank you very much again for your support

rizlas commented 9 months ago

Hi, I tested it again but the same problem persist.

eg: VM lorem with tags: ipsum dolor sit amet

  1. Add anoter tag (consectetur) in vsphere to lorem
  2. Run netbox sync
  3. consectetur is added to tags and netbox vm object
  4. Remove consectetur in vsphere from lorem
  5. Run netbox sync
  6. consectetur is removed from netbox vm object and tag counter decreased
  7. Run netbox sync again, if consectetur counter is 0, remove it

Step six is not happening.

Also, consider this to avoid semicolon when description is empty:

tag_name = self.tag_session.tagging.Tag.get(tag_id).name
vmware_tag_description = self.tag_session.tagging.Tag.get(tag_id).description
tag_description = primary_tag_name

if vmware_tag_description:
    tag_description = f"{primary_tag_name}: {vmware_tag_description}"
bb-Ricardo commented 9 months ago

Hi @rizlas,

Thank you for bearing with me and this detailed explanation. I finally got around to look into this again and I believe I found the issue. I just pushed another commit to development branch which removes the tags if they have not been found in the VMware source.

Can you please test it and let me know if it works now. Feel free to close the issue if your tests were successful.

Thank you.

rizlas commented 9 months ago

Hi, thanks to you. I'm only reporting.

Unfortunately tags are not yet deleted. Do I need to check any logs in particular?

bb-Ricardo commented 9 months ago

Hi,

so you checked out the latest development version and it is not removing the tags like you described in point 6.

the logs should mention the removal of a tag when parsing the object during the sync.

https://github.com/bb-Ricardo/netbox-sync/commit/b7388d97a908a0d2674f9ce0ecd78bbc0fee1137#diff-e7a495959928cbb6f10cb4c5343bbcae7a58acda168716b8db7690442f8e8a7eR1108-R1118

Here you could add debug lines like these to see which tags are found and which tags are currently assigned:

print(new_object_tags)
print(device_vm_object.data.get("tags", list()))
rizlas commented 9 months ago

Sorry, rush answer. The tag is removed from the VM now. (step 6) [Fixed] Tag object with count 0 is not removed (step 7)

bb-Ricardo commented 9 months ago

Hi @rizlas,

thank you for testing. I just tested it and it seems to work in my setup.

I created a new Tag with NetBox-synced: test tag as description. It had 0 tagged_items and it got deleted.

{'_original_data': {},
 'data': {'color': '9e9e9e',
          'created': '2024-02-15T19:17:13.736933Z',
          'description': 'NetBox-synced: test tag',
          'display': 'NetBox-synced Test123',
          'id': 14,
          'last_updated': '2024-02-15T19:17:13.736986Z',
          'name': 'NetBox-synced Test123',
          'object_types': [],
          'slug': 'netbox-synced-test123',
          'tagged_items': 0,
          'url': 'http://netbox.test/api/extras/tags/14/'},
 'data_model': {'color': 6,
                'description': 200,
                'name': 100,
                'slug': 100,
                'tagged_items': <class 'int'>},
 'deleted': False,
 'inventory': <module.netbox.inventory.NetBoxInventory object at 0x10ecb92a0>,
 'is_new': False,
 'nb_id': 14,
 'source': None,
 'unset_items': [],
 'updated_items': []}
2024-02-15 20:17:38,200 - INFO: Deleting unused tag NetBox-synced Test123
2024-02-15 20:17:38,252 - DEBUG2: Received HTTP Status 204.
2024-02-15 20:17:38,253 - INFO: NetBox successfully deleted tag object 'NetBox-synced Test123'.

Can you please have another look? Does your tag description differ?

Thank you

rizlas commented 9 months ago
{
    "id": 416,
    "url": "https://netbox/api/extras/tags/416/",
    "display": "test_netbox_sync",
    "name": "test_netbox_sync",
    "slug": "test_netbox_sync",
    "color": "9e9e9e",
    "description": "NetBox-synced: This is a nice test",
    "object_types": [],
    "tagged_items": 1,
    "created": "2024-02-16T09:47:07.835749Z",
    "last_updated": "2024-02-16T09:47:07.835765Z"
}

Yeah it's working. Dunno why yesterday wasn't.

Good, I had a look to deletion process, is really simple. Well done.

I'm not the author of the issue, so I can't close it.

Thank you!

bb-Ricardo commented 9 months ago

great, thank you for testing it again. Then I can finally release the next version

rizlas commented 9 months ago

On it immediately 🤣

bb-Ricardo commented 9 months ago

Yes 😬,

Was just waiting for your positive feedback. Now we got some neat bug fixes and great new features.

Thanks again for your great help here.

Now I need to tackle the next 🐞.

rizlas commented 9 months ago

You've missed to build docker image I guess 😄

bb-Ricardo commented 9 months ago

FFFFUUUUUUU*****

And just realised that the docker build is broken.

thank you. will take care of it.

rizlas commented 9 months ago

You sure? I've build it manually from main branch. All fine.

bb-Ricardo commented 9 months ago

So, now the image should be uptodate. Had a local issue with my docker setup. All fixed now.