Bulk Power fails with multiple nodes

[Hwiggy]

Background (please complete the following information):

• Panel or Daemon: Panel
• Version of Panel/Daemon: 0.7.13
• Server's OS: Ubuntu 18.04.2 LTS 4.15.0-45-generic
• Your Computer's OS & Browser: Unrelated (Artisan issue)

Describe the bug
Bulk power command failing with multiple nodes
Please provide additional information too, depending on what you have issues with:
PHP 7.2.15-1+ubuntu16.04.1+deb.sury.org+1

To Reproduce
Steps to reproduce the behavior:

1. Have multiple daemon nodes
2. Have at least 1 server on each
3. Try to bulk power with artisan with no nodes / servers specified, i.e p:bulk-power:restart
4. Observe as the first node is contacted for all servers, resulting in a few 404s

Expected behavior
The bulk power command should properly determine which node a server belongs to and use the correct URL for API calls

[MatthewSH]

Am I missing something here? So you're saying to not specify any nodes and not specify any servers?
Based on what I'm looking at I don't see an option for that. It looks like you at least have something.

The conditions I see for getServersForPowerAction are:

1. nodes and server are populated
2. nodes is empty, while servers is populated
3. nodes is populated, while servers is empty

So...unless I'm misreading something it looks like it should fail? I mean...there could be another check for if both are empty just to return all. Again, this is just a quick once over in the code..I could be wrong.

[Hwiggy]

The command states empty parameters as a possible execution condition. Empty parameters in fact does not work for multiple servers across multiple nodes.

[MatthewSH]

@Hwiggy although servers or nodes can be empty, it doesn't look like there's a condition when they're both empty. Where are you seeing this stated on the command? I'm looking through the command itself and nothing in the description or signature says it's able to be empty.

[Hwiggy]

Run the command

…

On Sat, Jun 1, 2019, 9:50 PM Matthew Hatcher ***@***.***> wrote: @Hwiggy <https://github.com/Hwiggy> although servers or nodes can be empty, it doesn't look like there's a condition when they're both empty. Where are you seeing this stated on the command? I'm looking through the command itself and nothing in the description or signature says it's able to be empty. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1526?email_source=notifications&email_token=AB6LLWFO3RPHW7AH2IUK3VTPYMRPNA5CNFSM4G7CFZ22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWXL6GA#issuecomment-497991448>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB6LLWEEC75B3JVABISHZDLPYMRPNANCNFSM4G7CFZ2Q> .

[MatthewSH]

I'll look into it soon. I think I know what's happening. Thanks.

[MatthewSH]

So, funny story about this and I'll give enough detail and background. You can see the history of this in #support-two in the Discord between me and 𝓓𝓸𝓶𝓲𝓲𝓑𝓾𝓷𝓷#0898.

Basically, where this started...they ran the bulk power for starting. Random server failed as a 404, unknown server. Through a variety of different commands, I was able to get a variety of different errors and eventually saw a pattern.

To know this...you have to know their nodes.
Node 1: nd2eu
Node 2: pb50
Node 3: nd1us

Basically, what seems to be happening is random fqdn's are being applied to random servers.

Here's a map of some of the errors:

Blank
Server 32 (nd1us) -> Ran on nd2eu node (404)
Server 9 (pb50) -> Ran on nd2eu node (404)
Server 11 (pb50) -> Ran on nd2eu node (404)
Server 12 (pb50) -> Ran on nd2eu node (404)
Server 13 (pb50) -> Ran on nd2eu node (404)
Server 14 (pb50) -> Ran on nd2eu node (404)

--nodes 1,2,3
Server 9 (pb50) -> Ran on nd2eu node (404)

--nodes 3,1
Server 32 (nd1us) -> Ran on pb50 node (404)
Server 33 (nd1us) -> Ran on pb50 node (404)

It's something about multiple nodes that the code is getting confused with. Specifying 1 single node is fine. Obviously the order changes depending on what servers were restarted first...but now we have something to help track this down.

[TrixterTheTux]

Caused by the piece of code here: (which affects all bulk commands)

panel/app/Repositories/Daemon/BaseRepository.php

Lines 131 to 138 in 96699b1

	if (! $this->getNode() instanceof Node) {
	if (! $this->getServer() instanceof Server) {
	throw new RuntimeException('An instance of ' . Node::class . ' or ' . Server::class . ' must be set on this repository in order to return a client.');
	}
	$this->getServer()->loadMissing('node');
	$this->setNode($this->getServer()->getRelation('node'));
	}

When you initially set the server (without setting the node), it'll fetch the node and save it for use. But if you update the server that has different node, it'll never update which node it does the request to (and ends up doing it on the first server's node and failing).
Easiest fix to test $repository->setNode($server->node)->setServer($server)->...; fixes it for me locally.