[ws-manager] Backport #4405 to use nodeName

components/ws-manager/pkg/manager/annotations.go

@@ -80,8 +80,8 @@ const (
 	// disposalStatusAnnotation contains the status of the workspace disposal process
 	disposalStatusAnnotation = "gitpod.io/disposalStatus"
 
-	// hostIPAnnotation contains the IP of the node the pod ran on. We use this to remeber the IP in case the pod gets evicted.
-	hostIPAnnotation = "gitpod.io/hostIP"
+	// nodeNameAnnotation contains the name of the node the pod ran on. We use this to remeber the name in case the pod gets evicted.
+	nodeNameAnnotation = "gitpod.io/nodeName"
 )
 
 // markWorkspaceAsReady adds annotations to a workspace pod

components/ws-manager/pkg/manager/manager.go

@@ -1078,12 +1078,11 @@ func (m *Manager) connectToWorkspaceDaemon(ctx context.Context, wso workspaceObj
 	tracing.ApplyOWI(span, wso.GetOWI())
 	defer tracing.FinishSpan(span, nil)
 
-	var hostIP string
-	if wso.Pod == nil {
-		return nil, xerrors.Errorf("cannot connect to ws-daemon: no workspace pod")
+	nodeName := wso.NodeName()
+	if nodeName == "" {
+		return nil, xerrors.Errorf("no nodeName found")
 	}
 
-	nodeName := wso.Pod.Spec.NodeName
 	// Get all the ws-daemon endpoints (headless)
 	// NOTE: we could do a DNS lookup but currently keeping it k8s-centric
 	// to allow for transitioning to the newer service topology support.
@@ -1103,6 +1102,7 @@ func (m *Manager) connectToWorkspaceDaemon(ctx context.Context, wso workspaceObj
 	}
 
 	// Find the ws-daemon endpoint on this node
+	var hostIP string
 	for _, pod := range endpointsList.Items {
 		for _, subset := range pod.Subsets {
 			for _, endpointAddress := range subset.Addresses {

components/ws-manager/pkg/manager/manager_test.go

@@ -293,55 +293,55 @@ func TestFindWorkspacePod(t *testing.T) {
 	}
 }
 
-func TestManager_connectToWorkspaceDaemon(t *testing.T) {
+func TestConnectToWorkspaceDaemon(t *testing.T) {
 	badNodeName := "not-matching-node"
 	goodNodeName := "a-matching-node"
 
-	type args struct {
-		ctx  context.Context
-		wso  workspaceObjects
-		objs []client.Object
+	type Args struct {
+		Ctx  context.Context
+		WSO  workspaceObjects
+		Objs []client.Object
 	}
 	tests := []struct {
-		name        string
-		args        args
-		wantErr     bool
-		expectedErr string
+		Name        string
+		Args        Args
+		WantErr     bool
+		ExpectedErr string
 	}{
 		{
-			name: "handles empty wso",
-			args: args{
-				ctx: context.Background(),
-				wso: workspaceObjects{},
+			Name: "handles empty wso",
+			Args: Args{
+				Ctx: context.Background(),
+				WSO: workspaceObjects{},
 			},
-			expectedErr: "no workspace pod",
+			ExpectedErr: "no nodeName found",
 		},
 		{
-			name: "handles no endpoints",
-			args: args{
-				ctx: context.Background(),
-				wso: workspaceObjects{
+			Name: "handles no endpoints",
+			Args: Args{
+				Ctx: context.Background(),
+				WSO: workspaceObjects{
 					Pod: &corev1.Pod{
 						Spec: corev1.PodSpec{
 							NodeName: "a_node_name",
 						},
 					},
 				},
 			},
-			expectedErr: "no matching endpoint",
+			ExpectedErr: "no matching endpoint",
 		},
 		{
-			name: "handles no endpoint on current node",
-			args: args{
-				ctx: context.Background(),
-				wso: workspaceObjects{
+			Name: "handles no endpoint on current node",
+			Args: Args{
+				Ctx: context.Background(),
+				WSO: workspaceObjects{
 					Pod: &corev1.Pod{
 						Spec: corev1.PodSpec{
 							NodeName: "a_node_name",
 						},
 					},
 				},
-				objs: []client.Object{
+				Objs: []client.Object{
 					&corev1.Endpoints{
 						TypeMeta: metav1.TypeMeta{
 							Kind:       "Endpoints",
@@ -375,20 +375,20 @@ func TestManager_connectToWorkspaceDaemon(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "no matching endpoint",
+			ExpectedErr: "no matching endpoint",
 		},
 		{
-			name: "finds endpoint on current node",
-			args: args{
-				ctx: context.Background(),
-				wso: workspaceObjects{
+			Name: "finds endpoint on current node",
+			Args: Args{
+				Ctx: context.Background(),
+				WSO: workspaceObjects{
 					Pod: &corev1.Pod{
 						Spec: corev1.PodSpec{
 							NodeName: goodNodeName,
 						},
 					},
 				},
-				objs: []client.Object{
+				Objs: []client.Object{
 					&corev1.Endpoints{
 						TypeMeta: metav1.TypeMeta{
 							Kind:       "Endpoints",
@@ -425,16 +425,16 @@ func TestManager_connectToWorkspaceDaemon(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		manager := forTestingOnlyGetManager(t, tt.args.objs...)
+		manager := forTestingOnlyGetManager(t, tt.Args.Objs...)
 		// Add dummy daemon pool - slightly hacky but we aren't testing the actual connectivity here
 		manager.wsdaemonPool = grpcpool.New(func(host string) (*grpc.ClientConn, error) {
 			return nil, nil
 		})
 
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := manager.connectToWorkspaceDaemon(tt.args.ctx, tt.args.wso)
-			if (err != nil) && !strings.Contains(err.Error(), tt.expectedErr) {
-				t.Errorf("Manager.connectToWorkspaceDaemon() error = %v, wantErr %v", err, tt.wantErr)
+		t.Run(tt.Name, func(t *testing.T) {
+			got, err := manager.connectToWorkspaceDaemon(tt.Args.Ctx, tt.Args.WSO)
+			if (err != nil) && !strings.Contains(err.Error(), tt.ExpectedErr) {
+				t.Errorf("Manager.connectToWorkspaceDaemon() error = %v, wantErr %v", err, tt.WantErr)
 				return
 			}
 			if err != nil && got != nil {

components/ws-manager/pkg/manager/monitor.go

@@ -330,7 +330,7 @@ func (m *Monitor) actOnPodEvent(ctx context.Context, status *api.WorkspaceStatus
 		//
 		// Also, in case the pod gets evicted we would not know the hostIP that pod ran on anymore.
 		// In preparation for those cases, we'll add it as an annotation.
-		err := m.manager.markWorkspace(ctx, workspaceID, deleteMark(wsk8s.TraceIDAnnotation), addMark(hostIPAnnotation, wso.HostIP()))
+		err := m.manager.markWorkspace(ctx, workspaceID, deleteMark(wsk8s.TraceIDAnnotation), addMark(nodeNameAnnotation, wso.NodeName()))
 		if err != nil {
 			log.WithError(err).Warn("was unable to remove traceID and/or add host IP annotation from/to workspace")
 		}
@@ -872,11 +872,11 @@ func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceOb
 		}
 
 		// Maybe the workspace never made it to a phase where we actually initialized a workspace.
-		// Assuming that once we've had a hostIP we've spoken to ws-daemon it's safe to assume that if
-		// we don't have a hostIP we don't need to dipose the workspace.
+		// Assuming that once we've had a nodeName we've spoken to ws-daemon it's safe to assume that if
+		// we don't have a nodeName we don't need to dipose the workspace.
 		// Obviously that only holds if we do not require a backup. If we do require one, we want to
 		// fail as loud as we can in this case.
-		if !doBackup && wso.HostIP() == "" {
+		if !doBackup && wso.NodeName() == "" {
 			// we don't need a backup and have never spoken to ws-daemon: we're good here.
 			m.finalizerMapLock.Unlock()
 			return true, &csapi.GitStatus{}, nil

components/ws-manager/pkg/manager/status.go

@@ -134,15 +134,15 @@ func (wso *workspaceObjects) WasEverReady() (res bool) {
 	return true
 }
 
-// HostIP returns the IP of the node this workspace is/was deployed to. If this workspace has never been deployed anywhere, HostIP returns an empty string.
-func (wso *workspaceObjects) HostIP() string {
+// HostName returns the name of the node this workspace is/was deployed to. If this workspace has never been deployed anywhere, HostName returns an empty string.
+func (wso *workspaceObjects) NodeName() string {
 	if wso.Pod == nil {
 		return ""
 	}
-	if wso.Pod.Status.HostIP != "" {
-		return wso.Pod.Status.HostIP
+	if wso.Pod.Spec.NodeName != "" {
+		return wso.Pod.Spec.NodeName
 	}
-	if res, ok := wso.Pod.Annotations[hostIPAnnotation]; ok {
+	if res, ok := wso.Pod.Annotations[nodeNameAnnotation]; ok {
 		return res
 	}
 

components/ws-manager/pkg/manager/status_test.go

@@ -155,21 +155,21 @@ func BenchmarkGetStatus(b *testing.B) {
 	}
 }
 
-func TestGetHostIP(t *testing.T) {
+func TestGetNodeName(t *testing.T) {
 	tests := []struct {
 		Name        string
 		WSO         workspaceObjects
 		Expectation string
 	}{
 		{
-			Name: "no hostIP",
+			Name: "no nodeName",
 		},
 		{
 			Name: "spec",
 			WSO: workspaceObjects{
 				Pod: &v1.Pod{
-					Status: v1.PodStatus{
-						HostIP: "foobar",
+					Spec: v1.PodSpec{
+						NodeName: "foobar",
 					},
 				},
 			},
@@ -181,23 +181,7 @@ func TestGetHostIP(t *testing.T) {
 				Pod: &v1.Pod{
 					ObjectMeta: metav1.ObjectMeta{
 						Annotations: map[string]string{
-							hostIPAnnotation: "from-anno",
-						},
-					},
-					Status: v1.PodStatus{
-						HostIP: "",
-					},
-				},
-			},
-			Expectation: "from-anno",
-		},
-		{
-			Name: "annotation pod no status",
-			WSO: workspaceObjects{
-				Pod: &v1.Pod{
-					ObjectMeta: metav1.ObjectMeta{
-						Annotations: map[string]string{
-							hostIPAnnotation: "from-anno",
+							nodeNameAnnotation: "from-anno",
 						},
 					},
 				},
@@ -210,11 +194,11 @@ func TestGetHostIP(t *testing.T) {
 				Pod: &v1.Pod{
 					ObjectMeta: metav1.ObjectMeta{
 						Annotations: map[string]string{
-							hostIPAnnotation: "from-anno",
+							nodeNameAnnotation: "from-anno",
 						},
 					},
-					Status: v1.PodStatus{
-						HostIP: "from-spec",
+					Spec: v1.PodSpec{
+						NodeName: "from-spec",
 					},
 				},
 			},
@@ -224,9 +208,9 @@ func TestGetHostIP(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.Name, func(t *testing.T) {
-			act := test.WSO.HostIP()
+			act := test.WSO.NodeName()
 			if diff := cmp.Diff(test.Expectation, act); diff != "" {
-				t.Errorf("unexpected hostIP (-want +got):\n%s", diff)
+				t.Errorf("unexpected nodeName (-want +got):\n%s", diff)
 			}
 		})
 	}