Periodic warning for 1-node cluster w/ seed hosts

[kingherc]

For fully-formed single-node clusters, emit a periodic warning if
seed hosts have been configured.

Closes #85222

[elasticmachine]

Pinging @elastic/es-distributed (Team:Distributed)

[DaveCTurner]

Looks good, I left some suggestions/comments but nothing major.

[DaveCTurner]

Also I've added the cloud-deploy label, it would be good to confirm (manually) that this doesn't trigger any warnings on single-node Cloud clusters.

[kingherc]

Hi @DaveCTurner , I've manually checked that the warning doesn't appear on a local distribution I built. How can I manually test it on a single-node cloud cluster? Does the cloud-deploy label somehow automatically spin a cloud cluster so I can check the logs?

[kingherc]

Hi @DaveCTurner , also pushed suggested changes, please check once more.

[DaveCTurner]

How can I manually test it on a single-node cloud cluster?

I shared a link to the internal docs in another channel.

[kingherc]

@elasticmachine test this

[kingherc]

@DaveCTurner please check once more and review.

[kingherc]

@elasticmachine run elasticsearch-ci/packaging-tests-unix-sample

[kingherc]

@elasticmachine run elasticsearch-ci/packaging-tests-unix-sample

[kingherc]

Hi @DaveCTurner , PR is ready for your review.

[DaveCTurner]

👍 sorry I'm unlikely to get to this today now, but it's on my list.

[DaveCTurner]

Changes all look good except for one tiny comment about naming/docs. However I think we should consider this blocked on elastic/cloud-on-k8s#5834. Not sure if you should (or want to) fix that too.

[kingherc]

@DaveCTurner I was eager to merge this but I understand we want to make sure cloud sets the settings correctly :)
By the way, when I tested it, I did see that seed_hosts was set to []. So the elastic/cloud-on-k8s#5834 is to make sure it truly happens and it's not by chance?

[kingherc]

@DaveCTurner thinking more about this: The current condition for the warning is if the seed_hosts is set, and it is non-empty. I think the cloud uses either the file method (without the seed_hosts at all) or might set it to an empty list. So I think the current code should be OK for the cloud. Or do they populate seed_hosts for some reason for single-node clusters? (I remind we don't look now at all the last discovered hosts)

[DaveCTurner]

I think this works fine as-is in Cloud (i.e. ESS) but not in ECK.

[kingherc]

@DaveCTurner , ah I see! I think ECK does not set seed_hosts at all, so they should not get the warning (as we do not look now for last discovered hosts -- which would include the unicast_hosts.txt file). I can test ECK with a single-node to confirm. Will try to see if they have instructions on testing.

[DaveCTurner]

Aha, right, thanks for the correction. I lost track of this in all the iterations. You're right, this should be fine for both ECK and ESS.

[kingherc]

Thanks @DaveCTurner ! By the way, I believe the issue elastic/cloud-on-k8s#5834 might not be needed anymore with our new approach. Should we propose to close it?

[DaveCTurner]

No I think the ECK issue still has value even now. If a node cannot find the master then it will (eventually) emit log messages about all the addresses it's trying, which will include the default seed addresses if this setting isn't set, and that just causes confusing noise.

[kingherc]

Ah, that's another message that gets emitted while trying to find masters. OK, thanks for the explanation! Will merge this when it passes the tests.