Skip to content

Commit

Permalink
Move master-host to an annotation (#131)
Browse files Browse the repository at this point in the history 
This allows for longer hostnames, in particular those of GCE instances.
This doesn't delete the old label, because that's hard with the default
"merge labels" behavior of CreateOrUpdateRobot(), so it's easier to
manually migrate the labels.

@Ongy
  • Loading branch information
@drigz
drigz authored Apr 11, 2023
1 parent 14ae831 commit 13d8340
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 54 deletions.
6 changes: 1 addition & 5 deletions scripts/robot-sim.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,9 @@ function create {
gcloud container clusters get-credentials "${ROBOT_NAME}" \
--zone=${GCP_ZONE} --project=${GCP_PROJECT_ID}

# This ensures the 'cloudrobotics.com/master-host' label will point to the VM
local CLUSTER_IP
CLUSTER_IP=$(gcloud compute instances list --project="${GCP_PROJECT_ID}" --filter "name~gke-${ROBOT_NAME}-default-pool.*" --format='get(networkInterfaces[0].accessConfigs[0].natIP)')

# shellcheck disable=2097 disable=2098
KUBE_CONTEXT=${GKE_SIM_CONTEXT} \
HOST_HOSTNAME=${CLUSTER_IP} \
HOST_HOSTNAME="nic0.${ROBOT_NAME}${GCP_ZONE}.c.${GCP_PROJECT_ID}.internal.gcpnode.com" \
ACCESS_TOKEN=$(gcloud auth application-default print-access-token) \
$DIR/../src/bootstrap/robot/setup_robot.sh \
${ROBOT_NAME} \
Expand Down
5 changes: 3 additions & 2 deletions src/go/cmd/setup-robot/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,10 +351,11 @@ func installChartOrDie(ctx context.Context, cs *kubernetes.Clientset, domain, re
}

func createOrUpdateRobot(ctx context.Context, k8sDynamicClient dynamic.Interface, labels map[string]string, annotations map[string]string) error {
const masterHost = "cloudrobotics.com/master-host"
labels["cloudrobotics.com/robot-name"] = *robotName
host := os.Getenv("HOST_HOSTNAME")
if host != "" && labels["cloudrobotics.com/master-host"] == "" {
labels["cloudrobotics.com/master-host"] = host
if host != "" && annotations[masterHost] == "" {
annotations[masterHost] = host
}
crc_version := os.Getenv("CRC_VERSION")
if crc_version != "" {
Expand Down
123 changes: 76 additions & 47 deletions src/go/cmd/setup-robot/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,108 +210,127 @@ func TestCreateOrUpdateRobot_Succeeds(t *testing.T) {
*robotName = "robot_name"

tests := []struct {
desc string
labels map[string]string
robot *registry.Robot
wantLabels map[string]string
desc string
labels map[string]string
annotations map[string]string
robot *registry.Robot
wantLabels map[string]string
wantAnnotations map[string]string
}{
{
"other robot",
map[string]string{},
&registry.Robot{
desc: "other robot",
labels: map[string]string{},
annotations: map[string]string{},
robot: &registry.Robot{
ObjectMeta: metav1.ObjectMeta{
Name: "other_robot",
Namespace: "default",
},
},
map[string]string{
wantLabels: map[string]string{
"cloudrobotics.com/robot-name": "robot_name",
},
wantAnnotations: map[string]string{
"cloudrobotics.com/master-host": hostname,
"cloudrobotics.com/robot-name": "robot_name",
},
},
{
"robot without label",
map[string]string{},
&registry.Robot{
desc: "robot without label",
labels: map[string]string{},
annotations: map[string]string{},
robot: &registry.Robot{
ObjectMeta: metav1.ObjectMeta{
Name: *robotName,
Namespace: "default",
},
},
map[string]string{
wantLabels: map[string]string{
"cloudrobotics.com/robot-name": "robot_name",
},
wantAnnotations: map[string]string{
"cloudrobotics.com/master-host": hostname,
"cloudrobotics.com/robot-name": "robot_name",
},
},
{
"robot with other label",
map[string]string{},
&registry.Robot{
desc: "robot with other label",
labels: map[string]string{},
annotations: map[string]string{},
robot: &registry.Robot{
ObjectMeta: metav1.ObjectMeta{
Name: *robotName,
Namespace: "default",
Labels: map[string]string{"cloudrobotics.com/ssh-port": "22"},
},
},
map[string]string{
wantLabels: map[string]string{
"cloudrobotics.com/robot-name": "robot_name",
"cloudrobotics.com/ssh-port": "22",
},
wantAnnotations: map[string]string{
"cloudrobotics.com/master-host": hostname,
"cloudrobotics.com/robot-name": "robot_name",
"cloudrobotics.com/ssh-port": "22",
},
},
{
"robot with same hostname",
map[string]string{},
&registry.Robot{
desc: "robot with same hostname",
labels: map[string]string{},
annotations: map[string]string{},
robot: &registry.Robot{
ObjectMeta: metav1.ObjectMeta{
Name: *robotName,
Namespace: "default",
Labels: map[string]string{"cloudrobotics.com/master-host": hostname},
Name: *robotName,
Namespace: "default",
Annotations: map[string]string{"cloudrobotics.com/master-host": hostname},
},
},
map[string]string{
wantLabels: map[string]string{
"cloudrobotics.com/robot-name": "robot_name",
},
wantAnnotations: map[string]string{
"cloudrobotics.com/master-host": hostname,
"cloudrobotics.com/robot-name": "robot_name",
},
},
{
"robot with different hostname",
map[string]string{},
&registry.Robot{
desc: "robot with different hostname",
labels: map[string]string{},
annotations: map[string]string{},
robot: &registry.Robot{
ObjectMeta: metav1.ObjectMeta{
Name: *robotName,
Namespace: "default",
Labels: map[string]string{"cloudrobotics.com/master-host": "other-host"},
Name: *robotName,
Namespace: "default",
Annotations: map[string]string{"cloudrobotics.com/master-host": "other-host"},
},
},
map[string]string{
wantLabels: map[string]string{
"cloudrobotics.com/robot-name": "robot_name",
},
wantAnnotations: map[string]string{
"cloudrobotics.com/master-host": hostname,
"cloudrobotics.com/robot-name": "robot_name",
},
},
{
"master-host given as input",
map[string]string{
desc: "master-host given as input",
labels: map[string]string{},
annotations: map[string]string{
"cloudrobotics.com/master-host": "correct-host",
},
&registry.Robot{
robot: &registry.Robot{
ObjectMeta: metav1.ObjectMeta{
Name: *robotName,
Namespace: "default",
Labels: map[string]string{"cloudrobotics.com/master-host": "other-host"},
Name: *robotName,
Namespace: "default",
Annotations: map[string]string{"cloudrobotics.com/master-host": "other-host"},
},
},
map[string]string{
wantLabels: map[string]string{
"cloudrobotics.com/robot-name": "robot_name",
},
wantAnnotations: map[string]string{
"cloudrobotics.com/master-host": "correct-host",
"cloudrobotics.com/robot-name": "robot_name",
},
},
}
for _, tc := range tests {
t.Run(tc.desc, func(t *testing.T) {
c := dynfake.NewSimpleDynamicClient(sc, tc.robot)
annotations := map[string]string{}
if err := createOrUpdateRobot(ctx, c, tc.labels, annotations); err != nil {
if err := createOrUpdateRobot(ctx, c, tc.labels, tc.annotations); err != nil {
t.Fatalf("createOrUpdateRobot() failed unexpectedly: %v", err)
}

Expand All @@ -325,11 +344,21 @@ func TestCreateOrUpdateRobot_Succeeds(t *testing.T) {
t.Fatalf("failed parsing robot labels: %v", err)
}
if !ok {
t.Fatalf("robot %q is missing the master host label", *robotName)
t.Fatalf("robot %q is missing the label map", *robotName)
}
if !reflect.DeepEqual(got, tc.wantLabels) {
t.Errorf("labels:\n%q\nwant:\n%q", got, tc.wantLabels)
}
got, ok, err = unstructured.NestedStringMap(robot.Object, "metadata", "annotations")
if err != nil {
t.Fatalf("failed parsing robot labels: %v", err)
}
if !ok {
t.Fatalf("robot %q is missing the annotation map", *robotName)
}
if !reflect.DeepEqual(got, tc.wantAnnotations) {
t.Errorf("annotations:\n%q\nwant:\n%q", got, tc.wantAnnotations)
}
})
}
}

0 comments on commit 13d8340

Please sign in to comment.