m/test/launch: order nodes in NodeIDs correctly
This ensures that referring to node by number via NodeIDs is possible -
ie. that NodeIDs[n] is the ID of the nth node as defined in NodeOpts or
as used as the 'idx' argument in RebootNode.
This was never and is still not correctly formalized, even in comments,
and is yet another proof that the cluster launch code deserves to be
rewritten from first principles.
Fixes #301.
Change-Id: I1ad13dc3bdd35a0c34f86e5d051ca9378491c876
Reviewed-on: https://review.monogon.dev/c/monogon/+/3168
Reviewed-by: Jan Schär <jan@monogon.tech>
Tested-by: Jenkins CI
diff --git a/metropolis/node/BUILD.bazel b/metropolis/node/BUILD.bazel
index 876dbfc..6354edc 100644
--- a/metropolis/node/BUILD.bazel
+++ b/metropolis/node/BUILD.bazel
@@ -18,7 +18,10 @@
"//metropolis:__subpackages__",
"@io_k8s_kubernetes//pkg/registry:__subpackages__",
],
- deps = ["@com_github_vishvananda_netlink//:netlink"],
+ deps = [
+ "//metropolis/proto/common",
+ "@com_github_vishvananda_netlink//:netlink",
+ ],
)
# debug_build checks if we're building in debug mode and enables various debug features for the image.
diff --git a/metropolis/node/labels.go b/metropolis/node/labels.go
index 08d0c6e..3e51aae 100644
--- a/metropolis/node/labels.go
+++ b/metropolis/node/labels.go
@@ -3,6 +3,8 @@
import (
"fmt"
"regexp"
+
+ cpb "source.monogon.dev/metropolis/proto/common"
)
var (
@@ -62,3 +64,14 @@
}
return nil
}
+
+// GetNodeLabel retrieves a node label by key, returning its value or an empty
+// string if no labels with this key is set on the node.
+func GetNodeLabel(labels *cpb.NodeLabels, key string) string {
+ for _, pair := range labels.Pairs {
+ if pair.Key == key {
+ return pair.Value
+ }
+ }
+ return ""
+}
diff --git a/metropolis/test/launch/cluster/cluster.go b/metropolis/test/launch/cluster/cluster.go
index 4cbfede..dfeb457 100644
--- a/metropolis/test/launch/cluster/cluster.go
+++ b/metropolis/test/launch/cluster/cluster.go
@@ -21,6 +21,7 @@
"os/exec"
"path"
"path/filepath"
+ "strconv"
"strings"
"syscall"
"time"
@@ -51,6 +52,12 @@
"source.monogon.dev/metropolis/test/launch"
)
+const (
+ // nodeNumberKey is the key of the node label used to carry a node's numerical
+ // index in the test system.
+ nodeNumberKey string = "test-node-number"
+)
+
// NodeOptions contains all options that can be passed to Launch()
type NodeOptions struct {
// Name is a human-readable identifier to be used in debug output.
@@ -776,7 +783,7 @@
InitialClusterConfiguration: opts.InitialClusterConfiguration,
Labels: &cpb.NodeLabels{
Pairs: []*cpb.NodeLabels_Pair{
- {Key: "test-node-id", Value: "0"},
+ {Key: nodeNumberKey, Value: "0"},
},
},
},
@@ -896,7 +903,9 @@
return nil, fmt.Errorf("could not write owner certificate: %w", err)
}
- // Set up a partially initialized cluster instance, to be filled in in the
+ launch.Log("Cluster: Node %d is %s", 0, firstNode.ID)
+
+ // Set up a partially initialized cluster instance, to be filled in the
// later steps.
cluster := &Cluster{
Owner: *cert,
@@ -969,7 +978,7 @@
CaCertificate: resI.CaCertificate,
Labels: &cpb.NodeLabels{
Pairs: []*cpb.NodeLabels_Pair{
- {Key: "test-node-id", Value: fmt.Sprintf("%d", i)},
+ {Key: nodeNumberKey, Value: fmt.Sprintf("%d", i)},
},
},
},
@@ -997,7 +1006,10 @@
}
}
+ // Wait for nodes to appear as NEW, populate a map from node number (index into
+ // NodeOpts, etc.) to Metropolis Node ID.
seenNodes := make(map[string]bool)
+ nodeNumberToID := make(map[int]string)
launch.Log("Cluster: waiting for nodes to appear as NEW...")
for i := 1; i < opts.NumNodes; i++ {
for {
@@ -1018,7 +1030,13 @@
ID: n.Id,
Pubkey: n.Pubkey,
}
- cluster.NodeIDs = append(cluster.NodeIDs, n.Id)
+
+ num, err := strconv.Atoi(node.GetNodeLabel(n.Labels, nodeNumberKey))
+ if err != nil {
+ return nil, fmt.Errorf("node %s has undecodable number label: %w", n.Id, err)
+ }
+ launch.Log("Cluster: Node %d is %s", num, n.Id)
+ nodeNumberToID[num] = n.Id
}
if len(seenNodes) == opts.NumNodes-1 {
@@ -1029,6 +1047,11 @@
}
launch.Log("Found all expected nodes")
+ // Build the rest of NodeIDs from map.
+ for i := 1; i < opts.NumNodes; i++ {
+ cluster.NodeIDs = append(cluster.NodeIDs, nodeNumberToID[i])
+ }
+
approvedNodes := make(map[string]bool)
upNodes := make(map[string]bool)
if !opts.LeaveNodesNew {