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

os_nova_host_aggregate - Fix aggregate delete with hosts #53166

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@MaxBab
Copy link
Contributor

MaxBab commented Mar 1, 2019

SUMMARY

Aggregate delete task will fail in case it has hosts within the aggregate.
As by the OpenStack, the hosts should be removed from the aggregate
prior aggregate delete.

Add remove host in case provided.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

os_nova_host_aggregate

os_nova_host_aggregate - Fix aggregate delete with hosts
Aggregate delete task will fail in case it has hosts within the aggregate.
As by the OpenStack, the hosts should be removed from the aggregate
prior aggregate delete.

Add remove host in case provided.
@openstack-zuul

This comment has been minimized.

Copy link

openstack-zuul bot commented Mar 1, 2019

Build succeeded (third-party-check pipeline).

@Yarboa

This comment has been minimized.

Copy link

Yarboa commented Mar 3, 2019

+1 There is logic, in that,
Need to remove all hosts part of aggregation, and than delete it,

@@ -167,6 +167,9 @@ def main():
if aggregate is None:
changed = False
else:
if hosts:

This comment has been minimized.

@krsacme

krsacme Mar 4, 2019

Contributor

Does it require to provide the list of hosts explicitly? The example does not provide the hosts input for the deleting the aggregate. I would expect to ansible to remove all the nodes from the aggregate by itself, instead of user input.

This comment has been minimized.

@MaxBab

MaxBab Mar 4, 2019

Author Contributor

Yes, the list of the hosts should be provided.
When you are using variables for the hosts, the hosts could be added or deleted depending by the state provided.

This is the default behavior of openstack.
It can't remove the hosts from the aggregate without getting the hosts provided.

I don't think ansible module should override this behavior.
It supports the openstack api capabilities and should not change the logic of openstack.

This comment has been minimized.

@krsacme

krsacme Mar 4, 2019

Contributor

It supports the openstack api capabilities and should not change the logic of openstack.

IMO, ansible enables to use the openstack APIs in a simplified manner, rather than providing a one-to-one mapping of all openstack cli commands. I may be wrong, i would love to hear from experts. If it is approved with the current approach, it would be good to update example with 'hosts' mentioned for 'absent' scenario too.

Yarboa pushed a commit to redhat-openstack/ansible-nfv that referenced this pull request Mar 4, 2019

Aggregate delete fix
Aggregate delete task will fail in case it has hosts within the aggregate.
As by the OpenStack, the hosts should be removed from the aggregate
prior aggregate delete.

Add remove host in case provided.

The upstream pull request could be tracked by the following link:
ansible/ansible#53166

Meanwhile, the fixed module will be placed under the 'library'
directory.

Change-Id: I518a1f8440338a0090b1bc2ee96d15bab3684a67
@MaxBab

This comment has been minimized.

Copy link
Contributor Author

MaxBab commented Mar 10, 2019

Hello @Shrews @cloudnull @dagnello @emonty @evrardjp @juliakreger @kuboj @mnaser @odyssey4me @rcarrillocruz,

Could you provide your input on the patch, please?

Thank you.

@ansibot ansibot added the stale_ci label Mar 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.