baremetal: sets hostname controller_names/worker_names

[ipochi]

Sets the hostname from the <cluster_name>-worker-<count_index> which
is set when set_standard_hostname is true, to
controller_names<count_index> for controllers and
worker_names<count_index> for workers

[pothos]

To add, the hostname change got introduced by #1477 where it was set from the template variable domain_name coming from var.controller_domains[count.index].

[pothos]

Can you add my comment to the commit message?

[invidian]

We could have some tests for this.

In general, I'd like #441 to be resolved in the first place, as it seems to me recently done changes more unify things, rather then retain old behavior, so I'm not sure about this PR. Perhaps someone more involved should decide.

[invidian]

We should make sure user can still override that.

[pothos]

You mean the user would use a different name for the machine when defining it in the configuration and setting up a domain name than the hostname would be? I think it's good to check first what the behavior of CLC snippets is when the same file gets specified by the user again and if it doesn't work, document that the user has to set the hostname through a one-time service unit.

[pothos]

Since this is a bug fix and does not change something related to how the user could overwrite the hostname, I think we should merge.

[pothos]

The mercury-controller-0 name in Test_Baremetal_NodeSpecificLabels is mismatching because the K8s node name is node1 based on the hostname. I gues the K8s node name used to be overwritten and now isn't anymore, or this node-specific label test was implemented after the accidental name change. I think it's good to just use the user-provided name and later change the CI node naming to be more specific than node1, node2, node3`.

[pothos]

The test error message is not specific enough:

                if len(nodes.Items) == 1 && nodes.Items[0].Name != nodeName {
                        t.Errorf("expected node %q to have the labels %s", nodes.Items[0].Name, labels)
                }

I think it should be

                if len(nodes.Items) == 1 {
                                if nodes.Items[0].Name != nodeName  {
                                        t.Errorf("expected node %q but found %q to have the labels %s", nodeName, nodes.Items[0].Name, labels)
                                }
                } else {
                        t.Errorf("expected a single node, got %d", len(nodes.Items))
                }

[invidian]

                if len(nodes.Items) == 1 {
                                if nodes.Items[0].Name != nodeName  {
                                        t.Errorf("expected node %q but found %q to have the labels %s", nodeName, nodes.Items[0].Name, labels)
                                }
                } else {
                        t.Errorf("expected a single node, got %d", len(nodes.Items))
                }

I'd make that two independent asserts with t.Fatal.

[pothos]

Will merge now and start a new PR based on this