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

Fixed deprecation warning when installing multiple pip with "with_items" #116

Merged
merged 1 commit into from
Nov 5, 2020
Merged

Fixed deprecation warning when installing multiple pip with "with_items" #116

merged 1 commit into from
Nov 5, 2020

Conversation

ironiq
Copy link

@ironiq ironiq commented Nov 2, 2020

The below deprecation warning comes at task 'Install psycopg2/selinux via pip on Red Hat-based distros':
'Invoking "pip" only once while using a loop via squash_actions is deprecated.'
Also there was a small problem when systemd units are being started: the daemon_reload was missing.

The below deprecation warning comes at task 'Install psycopg2/selinux via pip on Red Hat-based distros'
'Invoking "pip" only once while using a loop via squash_actions is deprecated.'
@ironiq ironiq changed the title Fixed deprecation warning when installing multiple pip with "with_items" Fixed deprecation warning when installing multiple pip with "with_items" and add daemon_reload when restarting Netbox systemd units Nov 2, 2020
@ironiq
Copy link
Author

ironiq commented Nov 2, 2020

This PR fixes #117

@lae
Copy link
Owner

lae commented Nov 2, 2020

The daemon_reload attribute for those two services is already within the handlers:

https://github.com/lae/ansible-role-netbox/blob/master/handlers/main.yml

Are you noting something otherwise?

@ironiq
Copy link
Author

ironiq commented Nov 2, 2020

Yes, i realized that, but ansible gave an error for me before adding daemon_reload. I will try to reproduce it tomorrow.

@ironiq
Copy link
Author

ironiq commented Nov 4, 2020

I tried it once again the original version, here is the output of that task:

TASK [lae.netbox : Start and enable NetBox' socket and service] ********************************************************************************************
failed: [netbox.company.local] (item=netbox.socket) => {"ansible_loop_var": "item", "changed": false, "item": "netbox.socket", "msg": "Could not find the requested service netbox.socket: host"}
failed: [netbox.company.local] (item=netbox.service) => {"ansible_loop_var": "item", "changed": false, "item": "netbox.service", "msg": "Could not find the requested service netbox.service: host"}

@ironiq
Copy link
Author

ironiq commented Nov 4, 2020

Hmm... It seems that if the unit file is deployed but the daemon was not reloaded, the next run will not change the file so the handler will not be called -> daemon_reload will not occur. I don't really know what happened here...

@lae
Copy link
Owner

lae commented Nov 4, 2020

Yeah, I realised that it might be a possibility that the daemon reload does not occur in the start & enable tasks while looking through, but it doesn't seem to break in CI - so I'm not sure if there's something about the environment you're deploying in or not that causes an issue.

I believe the fact that there is a restart handler on the configuration files that should be triggered on create means that we don't necessarily need to start the service. Can you try just removing the state: started attribute from those tasks and see if your issue persists? (on a fresh install, of course)

@ironiq
Copy link
Author

ironiq commented Nov 4, 2020

Actually i tried several times after i realized this and the original code worked perfectly. I think the issue for me was a leftover/not enough cleanup. Shall i close this pull request and open a brand new request, but with only the fix of the deprecation warning?

@ironiq
Copy link
Author

ironiq commented Nov 4, 2020

Seems i've found the problem: the unit is installed via template. This notifies the "restart $unit" handler. This handler will do the daemon_reload, set the state, but will not enable the unit. The next task is to enable and start the unit, but on CentOS7 i don't know why, but it cannot start even if the "enabled: yes" is there. When i add the "enabled: yes" to the handler, it will do its job.

@lae
Copy link
Owner

lae commented Nov 5, 2020

@ironiq You can just amend this PR and force push the new commit to overwrite what's currently in this PR.

Please see Option 2 here: https://www.burntfen.com/2015-10-30/how-to-amend-a-commit-on-a-github-pull-request

I will move any further discussion of the deploy issue to the linked issue.

@ironiq ironiq changed the title Fixed deprecation warning when installing multiple pip with "with_items" and add daemon_reload when restarting Netbox systemd units Fixed deprecation warning when installing multiple pip with "with_items" Nov 5, 2020
@ironiq
Copy link
Author

ironiq commented Nov 5, 2020

Can you please check it now?

@lae lae merged commit be1575d into lae:master Nov 5, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants