SyncCluster: Skip empty cluster updates.

[aluzzardi]

This change prevents doing an "empty" cluster update.

When etcd is not yet fully initialized, it tends to return an empty list of members.

Since internalSyncCluster uses the result of the previous run (it iterates over cluster.Machines which is also filled by internalSyncCluster), it means that if we do an empty update the client is forever broken: c.cluster.Machines will be empty therefore further calls to SyncCluster will be a no-op.

Signed-off-by: Andrea Luzzardi aluzzardi@gmail.com

[abronan]

This allows us to connect to the cluster without waiting for it to be properly started (although we have an incomplete list of machines, but a periodic SyncCluster should fix this).

Tested and works fine in CI.

related to #213 (last comment)

/cc @yichengq Is there anything else that we missed about the issue and the logic behind SyncCluster?

[yichengq]

remove this comment

[yichengq]

Could you update the comment for SyncCluster func too?
I think you have gotten all points in this process.

This change is reasonable IMO because nobody wants to update to a empty client urls. A better solution may be let etcd not return member info before it has been fully initialized.

lgtm

/cc @xiang90

[aluzzardi]

@yichengq Thanks, PTAL.

I agree that fixing etcd is a better solution - after all, it should never return an empty list (at the very least it should return its own address).

However, even if we did fix that, I believe we should still push this client change to handle backward compatibility.

[aluzzardi]

@yichengq @xiang90: Friendly ping

[yichengq]

lgtm. Thanks for the contribution! 🚢

[aluzzardi]

Thank you!

[abronan]

Thanks @yichengq! 🎈