Skip to content

Add the ability to rename objects #1447

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.

Sign up for GitHub

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

Jump to bottom
Merged
pyrodie18 merged 7 commits into from
Feb 23, 2025
Merged

Add the ability to rename objects #1447

pyrodie18 merged 7 commits into from
Feb 23, 2025

Conversation

@aplathrop
Copy link
Contributor

@aplathrop aplathrop commented Jan 24, 2025

SUMMARY

Adds the ability to rename objects in Zabbix

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • zabbix_item
  • zabbix_trigger
  • zabbix_discoveryrule
  • zabbix_itemprototype
  • zabbix_triggerprototype
ADDITIONAL INFORMATION

Currently the only way to rename resources using the Ansible modules is to delete the resource under the old name and recreate it under the new name, despite that the Zabbix API supports renaming these types of resources.
The UID for these resources from the API's point of view is a number, not the name. As that's not very user friendly, when I wrote the modules, I use the resource name and the host/template name to lookup the ID. Unfortunately, the side effect of this is if you try to rename a resource, the module will try to create a duplicate resource with the new name
I've been using the above work around of deleting and recreating, but that has its own unfortunate side-effect that you lose the historical data
This PR adds a new_name parameter like below:

- name: Rename Zabbix item
  community.zabbix.zabbix_item:
    name: agent_ping
    template_name: example_template
    params:
      new_name: new_agent_ping
    state: present

In this example, the module lookup the itemid for a Zabbix item with the name agent_ping in the template example_template. It will then set the name field for the item with the id to new_agent_ping

@codecov
Copy link

codecov bot commented Jan 24, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.99%. Comparing base (2a88fcf) to head (d05872e).
Report is 2 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1447      +/-   ##
==========================================
+ Coverage   78.78%   78.99%   +0.21%     
==========================================
  Files          47       47              
  Lines        5901     5946      +45     
  Branches     1515     1535      +20     
==========================================
+ Hits         4649     4697      +48     
  Misses        769      769              
+ Partials      483      480       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aplathrop aplathrop marked this pull request as ready for review January 24, 2025 20:06
@pyrodie18
Copy link
Collaborator

pyrodie18 commented Feb 12, 2025

I don't understand how this can be idempotent. If I run the module, it will change the name the first time. If I rerun the module it will fail because the original name that it's looking for is no longer there. How often do you rename things? It just seems like its bad ansible practice in general.

@aplathrop
Copy link
Contributor Author

aplathrop commented Feb 12, 2025

I agree it kind of leaves a bad taste in my mouth, but it seems like we should be able rename an object without deleting it and recreating it, especially because the Zabbix API supports it. I don't rename things every day, but I probably have renamed things a couple dozen times over the last year
I could have it check if object already exists with the new name and if it does, don't try to find the old name, assume it's already been renamed and set the state to ok rather than changed. Not sure this is a better solution, but would make it idempotent

@pyrodie18
Copy link
Collaborator

pyrodie18 commented Feb 13, 2025 •
edited
Loading

I agree it kind of leaves a bad taste in my mouth, but it seems like we should be able rename an object without deleting it and recreating it, especially because the Zabbix API supports it. I don't rename things every day, but I probably have renamed things a couple dozen times over the last year I could have it check if object already exists with the new name and if it does, don't try to find the old name, assume it's already been renamed and set the state to ok rather than changed. Not sure this is a better solution, but would make it idempotent

It seems like an edge case to me where I would normally say leave it out but I like your idea. It's similar to how Ansible handles the cmd module. My suggestion would be check and see if the new name is there, and if so no change. If it's not, then look for the old name and then change it/or fail. Please add a test case into the module integration tests as well.

@aplathrop
Copy link
Contributor Author

aplathrop commented Feb 19, 2025

Okay, I've changed it so if the new_name is already used, the module will return OK rather than failing and added unit tests to verify that
It says merging is blocked because the branch contains merge commits. I did have to update the branch with the changes to main, but it was showing that message before I did that

@pyrodie18 pyrodie18 merged commit 8950ea0 into ansible-collections:main Feb 23, 2025
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pyrodie18 pyrodie18 pyrodie18 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aplathrop @pyrodie18