do not delegate facts on client nodes

[guits]

This commit is a workaround for
https://bugzilla.redhat.com/show_bug.cgi?id=1550977

We iterate over all nodes on each node and we delegate the facts gathering.
This is high memory consuming when having a large number of nodes in the
inventory.
That way of gathering is not necessary for clients node so we can simply
gather local facts for these nodes.

Signed-off-by: Guillaume Abrioux gabrioux@redhat.com

[guits]

jenkins test luminous-ansible2.4-purge_bluestore_osds_container

[leseb]

I'm ok with the patch but last time I discussed this with @fultonj he said disabling fact gathering didn't change anything. What has changed since?

[guits]

@leseb this is what I've raised yesterday in a discussion with @andrewschoen
The thing is that we are not sure how they exactly tested. The context is still unclear.
I'm waiting for an env with that reproduced issue so I can test this patch myself.

[guits]

jenkins test luminous-ansible2.4-purge_bluestore_osds_container

[andymcc]

2 things:

1. The logic seems wrong:
   This will delegate facts if delegate_facts_host is true or the host isnt in the clients group. E,g, if you set "delegate_facts_host = false" it will still delegate on non-client hosts.
   The title makes it sound like you want delegate_facts_host and inventory_hostname not in groups.get('clients') - which should delegate if delegate_facts_host is true and the host isn't in the client group.

2. This has a consequence because if you have a host that is in the clients group and another group (for whatever reason) it won't delegate facts. You may want to check if it is in all the other groups, but not in the clients group, but that makes a long statement.

Maybe a better way would be to remove the "clients" from the hosts: for this task (line 13), then the gather facts task wont run against clients though, you could run that first against clients.

[guits]

that's correct, i'll update this commit.
thanks @andymcc

[andrewschoen]

I like the idea of removing clients from the hosts line for this play. Perhaps we should make this fact delegation it's own play entirely, not add clients there but then keep them here so that ceph-defaults and ceph-docker-common can be run against them?

I think that would take care of the second concern @andymcc had.

[guits]

what about using with_items: "{{ groups['all'] | difference(groups['clients']) }}" ?

here is the result.
We can see client0 isn't gathering any facts and nobody is gathering facts about client0
later client0 will gather his facts here

@andymcc @leseb @andrewschoen

[andrewschoen]

what about using with_items: "{{ groups['all'] | difference(groups['clients']) }}" ?

I think this is getting closer but I have a couple questions/concerns:

• if clients is not included in the play that delegates facts and a user provides--limit clients the playbook will fail because the client nodes will not know about the rest of the cluster to create a ceph.conf

• I think the concern about a node being a client and for example a mon might still exist here. If the hostname for a node exists in both client and mon groups then it would not be included in groups['all'] | difference(groups['clients']) right? This means it's facts would not be delegated and it would not be included in the ceph.conf if --limit clients was used. I'm not sure of a good way to fix this problem without big changes to how we create configuration files though.

This does certainly avoid the issue of delegating facts from clients to all other nodes in the cluster though.

[guits]

jenkins test luminous-ansible2.4-purge_bluestore_osds_container

[guits]

jenkins test luminous-ansible2.4-purge_bluestore_osds_container

[guits]

if clients is not included in the play that delegates facts and a user provides--limit clients the playbook will fail because the client nodes will not know about the rest of the cluster to create a ceph.conf

as discussed with you on IRC (btw, thanks for your help), this has been addressed by the last commit I've pushed.

I think the concern about a node being a client and for example a mon might still exist here. If the hostname for a node exists in both client and mon groups then it would not be included in groups['all'] | difference(groups['clients']) right? This means it's facts would not be delegated and it would not be included in the ceph.conf if --limit clients was used. I'm not sure of a good way to fix this problem without big changes to how we create configuration files though.

yes, this concern is still present and yes, I'm not sure we can find a fix quickly and without bringing a huge change.

[guits]

jenkins test luminous-ansible2.4-purge_bluestore_osds_container

[fultonj]

Preliminary testing shows that this patch plus moving from ansible 2.4.2 to 2.4.3 seems to reduce the memory consumed by ceph-ansible. There have only been 3 test runs (2.4.2, 2.4.3 and 2.4.3 + this patch) so we should probably do more runs before we conclude it reduces memory by x%.