Merge pull request #798 from GuessWhoSamFoo/issue-797

Fix tab error when service spec uses externalName
vmware-archive · Mar 27, 2020 · a9f41ce · a9f41ce
2 parents 1f042d5 + 6987677
commit a9f41ce
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 56 deletions.
diff --git a/changelogs/unreleased/798-GuessWhoSamFoo b/changelogs/unreleased/798-GuessWhoSamFoo
@@ -0,0 +1 @@
+Fix tab errors when service spec uses externalName
diff --git a/internal/objectstatus/service.go b/internal/objectstatus/service.go
@@ -28,38 +28,40 @@ func service(ctx context.Context, object runtime.Object, o store.Store) (ObjectS
 		return ObjectStatus{}, errors.Wrap(err, "convert object to service")
 	}
 
-	key := store.Key{
-		Namespace:  service.Namespace,
-		APIVersion: "v1",
-		Kind:       "Endpoints",
-		Name:       service.Name,
-	}
+	if service.Spec.ExternalName == "" {
+		key := store.Key{
+			Namespace:  service.Namespace,
+			APIVersion: "v1",
+			Kind:       "Endpoints",
+			Name:       service.Name,
+		}
 
-	endpoints := &corev1.Endpoints{}
+		endpoints := &corev1.Endpoints{}
 
-	found, err := store.GetAs(ctx, o, key, endpoints)
-	if err != nil {
-		return ObjectStatus{}, errors.Wrapf(err, "get endpoints for service %s", service.Name)
-	}
+		found, err := store.GetAs(ctx, o, key, endpoints)
+		if err != nil {
+			return ObjectStatus{}, errors.Wrapf(err, "get endpoints for service %s", service.Name)
+		}
 
-	if !found {
-		return ObjectStatus{
-			nodeStatus: component.NodeStatusWarning,
-			Details:    []component.Component{component.NewText("Service has no endpoints")},
-		}, nil
-	}
+		if !found {
+			return ObjectStatus{
+				nodeStatus: component.NodeStatusWarning,
+				Details:    []component.Component{component.NewText("Service has no endpoints")},
+			}, nil
+		}
 
-	addressCount := 0
+		addressCount := 0
 
-	for _, subset := range endpoints.Subsets {
-		addressCount += len(subset.Addresses)
-	}
+		for _, subset := range endpoints.Subsets {
+			addressCount += len(subset.Addresses)
+		}
 
-	if addressCount == 0 {
-		return ObjectStatus{
-			nodeStatus: component.NodeStatusWarning,
-			Details:    []component.Component{component.NewText("Service has no endpoint addresses")},
-		}, nil
+		if addressCount == 0 {
+			return ObjectStatus{
+				nodeStatus: component.NodeStatusWarning,
+				Details:    []component.Component{component.NewText("Service has no endpoint addresses")},
+			}, nil
+		}
 	}
 
 	return ObjectStatus{

diff --git a/internal/objectstatus/service_test.go b/internal/objectstatus/service_test.go
@@ -52,6 +52,17 @@ func Test_service(t *testing.T) {
 				Details:    []component.Component{component.NewText("Service is OK")},
 			},
 		},
+		{
+			name: "externalName",
+			init: func(t *testing.T, o *storefake.MockStore) runtime.Object {
+				objectFile := "service_external.yaml"
+				return testutil.LoadObjectFromFile(t, objectFile)
+			},
+			expected: ObjectStatus{
+				nodeStatus: component.NodeStatusOK,
+				Details:    []component.Component{component.NewText("Service is OK")},
+			},
+		},
 		{
 			name: "no endpoint subsets",
 			init: func(t *testing.T, o *storefake.MockStore) runtime.Object {

diff --git a/internal/objectstatus/testdata/service_external.yaml b/internal/objectstatus/testdata/service_external.yaml
@@ -0,0 +1,12 @@
+kind: Service
+apiVersion: v1
+metadata:
+  name: mysql
+  namespace: default
+spec:
+  ports:
+    - protocol: TCP
+      port: 3306
+      targetPort: 3306
+  type: ExternalName
+  externalName: host-to-external-mysql.com
diff --git a/internal/printer/service.go b/internal/printer/service.go
@@ -311,6 +311,10 @@ func createServiceEndpointsView(ctx context.Context, service *corev1.Service, op
 	cols := component.NewTableCols("Target", "IP", "Node Name")
 	table := component.NewTable("Endpoints", "There are no endpoints!", cols)
 
+	if service.Spec.ExternalName != "" {
+		return table, nil
+	}
+
 	object, err := o.Get(ctx, key)
 	if err != nil {
 		return nil, errors.Wrapf(err, "get endpoints for service %s", service.Name)

diff --git a/internal/printer/service_test.go b/internal/printer/service_test.go
@@ -285,12 +285,7 @@ func Test_createServiceSummaryStatus(t *testing.T) {
 }
 
 func Test_createServiceEndpointsView(t *testing.T) {
-	service := &corev1.Service{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "service",
-		},
-	}
+	cols := component.NewTableCols("Target", "IP", "Node Name")
 
 	nodeName := "node"
 	endpoints := &corev1.Endpoints{
@@ -312,36 +307,72 @@ func Test_createServiceEndpointsView(t *testing.T) {
 		},
 	}
 
-	controller := gomock.NewController(t)
-	defer controller.Finish()
+	cases := []struct {
+		name    string
+		service *corev1.Service
+		table   *component.Table
+		rows    component.TableRow
+	}{
+		{
+			name: "endpoint",
+			service: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "service",
+				},
+			},
+			table: component.NewTable("Endpoints", "There are no endpoints!", cols),
+			rows: component.TableRow{
+				"Target":    component.NewLink("", "pod", "/pod"),
+				"IP":        component.NewText("10.1.1.1"),
+				"Node Name": component.NewText("node"),
+			},
+		},
+		{
+			name: "externalName",
+			service: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "service",
+				},
+				Spec: corev1.ServiceSpec{
+					ExternalName: "test",
+				},
+			},
+			table: component.NewTable("Endpoints", "There are no endpoints!", cols),
+		},
+	}
 
-	tpo := newTestPrinterOptions(controller)
-	printOptions := tpo.ToOptions()
+	for _, tc := range cases {
+		controller := gomock.NewController(t)
+		defer controller.Finish()
 
-	key := store.Key{Namespace: "default", APIVersion: "v1", Kind: "Endpoints", Name: "service"}
-	tpo.objectStore.EXPECT().
-		Get(gomock.Any(), gomock.Eq(key)).
-		Return(toUnstructured(t, endpoints), nil)
+		tpo := newTestPrinterOptions(controller)
+		printOptions := tpo.ToOptions()
 
-	podLink := component.NewLink("", "pod", "/pod")
-	tpo.link.EXPECT().
-		ForGVK(gomock.Any(), "v1", "Pod", gomock.Any(), gomock.Any()).
-		Return(podLink, nil).
-		AnyTimes()
+		if tc.service.Spec.ExternalName == "" {
+			key := store.Key{Namespace: "default", APIVersion: "v1", Kind: "Endpoints", Name: "service"}
+			tpo.objectStore.EXPECT().
+				Get(gomock.Any(), gomock.Eq(key)).
+				Return(toUnstructured(t, endpoints), nil)
+
+			podLink := component.NewLink("", "pod", "/pod")
+			tpo.link.EXPECT().
+				ForGVK(gomock.Any(), "v1", "Pod", gomock.Any(), gomock.Any()).
+				Return(podLink, nil).
+				AnyTimes()
+		}
 
-	ctx := context.Background()
-	got, err := createServiceEndpointsView(ctx, service, printOptions)
-	require.NoError(t, err)
+		ctx := context.Background()
+		got, err := createServiceEndpointsView(ctx, tc.service, printOptions)
+		require.NoError(t, err)
 
-	cols := component.NewTableCols("Target", "IP", "Node Name")
-	expected := component.NewTable("Endpoints", "There are no endpoints!", cols)
-	expected.Add(component.TableRow{
-		"Target":    component.NewLink("", "pod", "/pod"),
-		"IP":        component.NewText("10.1.1.1"),
-		"Node Name": component.NewText("node"),
-	})
+		if tc.rows != nil {
+			tc.table.Add(tc.rows)
+		}
 
-	component.AssertEqual(t, expected, got)
+		component.AssertEqual(t, tc.table, got)
+	}
 }
 
 func Test_describePortShort(t *testing.T) {