Repository: vmware-tanzu/octant
Pull request #513 (2019-12-29 12:30:09)

pods with missing owners should not cause workload viewer to break
If a pod has a missing ancestor, i.e, it could not be found in the object store, skip finding the ancestor instead of returning an error.

#510


Patch:
diff --git a/internal/modules/workloads/home_describer.go b/internal/modules/workloads/home_describer.go
index d7d188ba..b0478232 100644
--- a/internal/modules/workloads/home_describer.go
+++ b/internal/modules/workloads/home_describer.go
@@ -52,7 +52,7 @@ func (h *HomeDescriber) Describe(ctx context.Context, namespace string, options
 
 	cards, fullMetrics, err := collector.Collect(ctx, namespace)
 	if err != nil {
-		return component.EmptyContentResponse, fmt.Errorf("collect workload cards")
+		return component.EmptyContentResponse, fmt.Errorf("collect workload cards: %w", err)
 	}
 
 	cardWidth := component.WidthHalf
diff --git a/internal/octant/workload.go b/internal/octant/workload.go
index c19f45eb..c26bbb19 100644
--- a/internal/octant/workload.go
+++ b/internal/octant/workload.go
@@ -171,8 +171,7 @@ func objectOwner(ctx context.Context, objectStore store.Store, object *unstructu
 	}
 
 	if !found {
-		// TODO: does this need a better error? (GH#510)
-		return nil, fmt.Errorf("unable to find %s: %w", ownerRefKey, err)
+		return object, nil
 	}
 
 	return objectOwner(ctx, objectStore, owner)


Test patch:
diff --git a/internal/octant/workload_test.go b/internal/octant/workload_test.go
index 033b2a84..faf8bda9 100644
--- a/internal/octant/workload_test.go
+++ b/internal/octant/workload_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	metricsv1beta1 "k8s.io/metrics/pkg/apis/metrics/v1beta1"
@@ -66,6 +67,17 @@ func TestClusterWorkloadLoader(t *testing.T) {
 		corev1.ResourceMemory: resource.MustParse("3Mi"),
 	}
 
+	pod3 := testutil.ToUnstructured(t, testutil.CreatePod("pod-3", func(pod *corev1.Pod) {
+		pod.OwnerReferences = []metav1.OwnerReference{
+			{
+				APIVersion: "apps/v1",
+				Kind:       "ReplicaSet",
+				Name:       "missing",
+				UID:        "12345",
+			},
+		}
+	}))
+
 	objectStore := storeFake.NewMockStore(controller)
 	objectStore.EXPECT().
 		List(gomock.Any(), store.Key{
@@ -86,6 +98,25 @@ func TestClusterWorkloadLoader(t *testing.T) {
 		Return(replicaSet, true, nil).
 		AnyTimes()
 
+	noOwnerObjectStore := storeFake.NewMockStore(controller)
+	noOwnerObjectStore.EXPECT().
+		List(gomock.Any(), store.Key{
+			Namespace:  "namespace",
+			APIVersion: "v1",
+			Kind:       "Pod",
+		}).
+		Return(testutil.ToUnstructuredList(t, pod3), true, nil).
+		AnyTimes()
+	noOwnerObjectStore.EXPECT().
+		Get(gomock.Any(), store.Key{
+			Namespace:  "namespace",
+			APIVersion: "apps/v1",
+			Kind:       "ReplicaSet",
+			Name:       "missing",
+		}).
+		Return(nil, false, nil).
+		AnyTimes()
+
 	cases := []struct {
 		name            string
 		namespace       string
@@ -158,6 +189,26 @@ func TestClusterWorkloadLoader(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:            "pod with missing owner",
+			objectStore:     noOwnerObjectStore,
+			podMetricLoader: podMetricsLoader(controller, pm, false, false),
+			namespace:       "namespace",
+			expected: []octant.Workload{
+				{
+					Name:     "pod-3",
+					IconName: "application",
+					SegmentCounter: map[component.NodeStatus][]octant.PodWithMetric{
+						component.NodeStatusOK: {
+							{
+								Pod:          pod3,
+								ResourceList: corev1.ResourceList{},
+							},
+						},
+					},
+				},
+			},
+		},
 		{
 			name:            "namespace is blank",
 			objectStore:     objectStore,