Repository: vmware-tanzu/octant
Pull request #831 (2020-04-09 19:45:45)

load summary even if actions fail to load
Signed-off-by: Wayne Witzel III <wayne@riotousliving.com>


**What this PR does / why we need it**:

Summary tab fails to load because generating actions will can the object path function which produces an error for disconnected pods.

https://github.com/vmware-tanzu/octant/blob/master/internal/printer/container.go#L508

Log and continue.



GuessWhoSamFoo: This can end up spamming the console. Can it check for the parent pod status and decide to add the action func instead?
wwitzel3: Yes.
wwitzel3: @GuessWhoSamFoo so an issue I run in to when doing that.

Pod Templates which are part of a Deployment summary also use the container configuration printer.
I'm still trying to decide the best way to handle this.
wwitzel3: I pushed up a new effort. Now the pod template is explicitly adding the action. No if/else song and dance. I left the error message in for those edge cases, but it no longer spams.
wwitzel3: @vmware-tanzu/octant-reviewers PTAL

Patch:
diff --git a/README.md b/README.md
index e335dbb9..65730515 100644
--- a/README.md
+++ b/README.md
@@ -131,8 +131,6 @@ Plugins are a core part of Octant in the Kubernetes ecosystem. A plugin can read
 
 An example plugin can be found in [`cmd/octant-sample-plugin`](cmd/octant-sample-plugin) and installed to the default plugin path with `go run build.go install-test-plugin`.
 
-Some plugins can be found on GitHub in the [`#octant-plugin`](https://github.com/topics/octant-plugin) topic (tag).
-
 Documentation for plugin components can be found in the [Plugins section](https://octant.dev/docs/master/plugins/) of the documentation.
 
 ## Discussion
diff --git a/changelogs/unreleased/831-wwitzel3 b/changelogs/unreleased/831-wwitzel3
new file mode 100644
index 00000000..1717a32c
--- /dev/null
+++ b/changelogs/unreleased/831-wwitzel3
@@ -0,0 +1,1 @@
+Fix bug that prevented Pod summary loading if status was nil/unknown.
diff --git a/internal/printer/container.go b/internal/printer/container.go
index 92b3653f..47f38373 100644
--- a/internal/printer/container.go
+++ b/internal/printer/container.go
@@ -9,6 +9,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"github.com/vmware-tanzu/octant/internal/log"
 	"path"
 	"sort"
 	"strings"
@@ -28,8 +29,32 @@ import (
 	"github.com/vmware-tanzu/octant/pkg/view/component"
 )
 
+
 type containerActionFunc func(runtime.Object, *corev1.Container) (component.Action, error)
 
+// ContainerConfigurationOption
+type ContainerConfigurationOption func(*ContainerConfiguration)
+
+func IsInit(b bool) ContainerConfigurationOption {
+	return func(cc *ContainerConfiguration) {
+		cc.isInit = b
+	}
+}
+
+func WithPrintOptions(options Options) ContainerConfigurationOption {
+	return func(cc *ContainerConfiguration) {
+		cc.options = options
+	}
+}
+
+func WithActions(actions ...containerActionFunc) ContainerConfigurationOption {
+	return func(cc *ContainerConfiguration) {
+		for _, action := range actions {
+			cc.actionGenerators = append(cc.actionGenerators, action)
+		}
+	}
+}
+
 // ContainerConfiguration generates container configuration.
 type ContainerConfiguration struct {
 	parent             runtime.Object
@@ -42,17 +67,18 @@ type ContainerConfiguration struct {
 }
 
 // NewContainerConfiguration creates an instance of ContainerConfiguration.
-func NewContainerConfiguration(ctx context.Context, parent runtime.Object, c *corev1.Container, pfs portforward.PortForwarder, isInit bool, options Options) *ContainerConfiguration {
-	actionGenerators := []containerActionFunc{}
-	return &ContainerConfiguration{
+func NewContainerConfiguration(ctx context.Context, parent runtime.Object, c *corev1.Container, pfs portforward.PortForwarder, opts ...ContainerConfigurationOption) *ContainerConfiguration {
+	cc := &ContainerConfiguration{
 		parent:             parent,
 		container:          c,
-		isInit:             isInit,
 		portForwardService: pfs,
-		options:            options,
 		context:            ctx,
-		actionGenerators:   actionGenerators,
+		actionGenerators:   []containerActionFunc{},
 	}
+	for _, o := range opts {
+		o(cc)
+	}
+	return cc
 }
 
 // Create creates a deployment configuration summary.
@@ -107,10 +133,6 @@ func (cc *ContainerConfiguration) Create() (*component.Summary, error) {
 
 		sections.AddText("Ready", fmt.Sprintf("%t", containerStatus.Ready))
 		sections.AddText("Restart Count", fmt.Sprintf("%d", containerStatus.RestartCount))
-	} else {
-		if !cc.isInit {
-			cc.actionGenerators = append(cc.actionGenerators, editContainerAction)
-		}
 	}
 
 	envTbl, err := describeContainerEnv(cc.context, cc.parent, c, cc.options)
@@ -145,7 +167,9 @@ func (cc *ContainerConfiguration) Create() (*component.Summary, error) {
 	for _, actionFunc := range cc.actionGenerators {
 		action, err := actionFunc(cc.parent, c)
 		if err != nil {
-			return nil, errors.Wrap(err, "loading actions")
+			logger := log.From(cc.context)
+			logger.Errorf("failed loading actions: %w", err)
+			continue
 		}
 		summary.AddAction(action)
 	}
@@ -530,6 +554,6 @@ func containersPathForObject(object runtime.Object) ([]string, error) {
 	case g.Group == "apps" && g.Kind == "StatefulSet":
 		return []string{"spec", "template", "spec", "containers"}, nil
 	default:
-		return nil, errors.Errorf("unable to find containers location for %s", g)
+		return nil, errors.Errorf("unable to find containers location for %+v, %s, %s", g, g.Group, g.Kind)
 	}
 }
diff --git a/internal/printer/job_template.go b/internal/printer/job_template.go
index 60094b42..e1ee0f44 100644
--- a/internal/printer/job_template.go
+++ b/internal/printer/job_template.go
@@ -51,7 +51,7 @@ func (jt *JobTemplate) AddToFlexLayout(fl *flexlayout.FlexLayout, options Option
 	containerSection := fl.AddSection()
 
 	for _, container := range jt.jobTemplateSpec.Spec.Template.Spec.Containers {
-		containerConfig := NewContainerConfiguration(jt.context, jt.parent, &container, portForwarder, false, options)
+		containerConfig := NewContainerConfiguration(jt.context, jt.parent, &container, portForwarder, IsInit(false), WithPrintOptions(options))
 		summary, err := containerConfig.Create()
 		if err != nil {
 			return err
diff --git a/internal/printer/pod.go b/internal/printer/pod.go
index 2326f54f..14dd7cd7 100644
--- a/internal/printer/pod.go
+++ b/internal/printer/pod.go
@@ -749,7 +749,7 @@ func (p *podHandler) Containers(ctx context.Context, options Options) error {
 
 func defaultPodContainers(ctx context.Context, pod *corev1.Pod, container *corev1.Container, isInit bool, options Options) (*component.Summary, error) {
 	portForwarder := options.DashConfig.PortForwarder()
-	creator := NewContainerConfiguration(ctx, pod, container, portForwarder, isInit, options)
+	creator := NewContainerConfiguration(ctx, pod, container, portForwarder, IsInit(isInit), WithPrintOptions(options))
 	return creator.Create()
 }
 
diff --git a/internal/printer/pod_template.go b/internal/printer/pod_template.go
index b4edbd50..d7d1505c 100644
--- a/internal/printer/pod_template.go
+++ b/internal/printer/pod_template.go
@@ -109,7 +109,13 @@ func podTemplateContainers(ctx context.Context, fl *flexlayout.FlexLayout, optio
 	width := component.WidthHalf
 
 	for index, container := range options.containers {
-		containerConfig := NewContainerConfiguration(ctx, options.parent, &container, portForwarder, options.isInit, options.printOptions)
+		containerConfig := NewContainerConfiguration(
+			ctx, options.parent, &container, portForwarder,
+			IsInit(options.isInit),
+			WithPrintOptions(options.printOptions),
+			WithActions(editContainerAction),
+		)
+
 		summary, err := containerConfig.Create()
 		if err != nil {
 			return err
diff --git a/site/_includes/anchor_headings.html b/site/_includes/anchor_headings.html
index 4a8b32ff..21e2d047 100644
--- a/site/_includes/anchor_headings.html
+++ b/site/_includes/anchor_headings.html
@@ -1,6 +1,6 @@
 {% capture headingsWorkspace %}
   {% comment %}
-    Version 1.0.6
+    Version 1.0.4
       https://github.com/allejo/jekyll-anchor-headings
 
     "Be the pull request you wish to see in the world." ~Ben Balter
@@ -13,8 +13,7 @@
 
     Optional Parameters:
       * beforeHeading (bool)   : false  - Set to true if the anchor should be placed _before_ the heading's content
-      * anchorAttrs   (string) :  ''    - Any custom HTML attributes that will be added to the `<a>` tag; you may NOT use `href`, `class` or `title`;
-                                          the `%heading%` placeholder is available
+      * anchorAttrs   (string) :  ''    - Any custom HTML attributes that will be added to the `<a>` tag; you may NOT use `href`, `class` or `title`
       * anchorBody    (string) :  ''    - The content that will be placed inside the anchor; the `%heading%` placeholder is available
       * anchorClass   (string) :  ''    - The class(es) that will be used for each anchor. Separate multiple classes with a space
       * anchorTitle   (string) :  ''    - The `title` attribute that will be used for anchors
@@ -44,15 +43,11 @@
     {% assign nextChar = node | replace: '"', '' | strip | slice: 0, 1 %}
     {% assign headerLevel = nextChar | times: 1 %}
 
-    <!-- If the level is cast to 0, it means it's not a h1-h6 tag, so let's see if we need to fix it -->
+    <!-- If the level is cast to 0, it means it's not a h1-h6 tag, so let's try to fix it -->
     {% if headerLevel == 0 %}
-      <!-- Split up the node based on closing angle brackets and get the first one. -->
-      {% assign firstChunk = node | split: '>' | first %}
-
-      <!-- If the first chunk does NOT contain a '<', that means we've broken another HTML tag that starts with 'h' -->
-      {% unless firstChunk contains '<' %}
+      {% if nextChar != '<' and nextChar != '' %}
         {% capture node %}<h{{ node }}{% endcapture %}
-      {% endunless %}
+      {% endif %}
 
       {% capture edited_headings %}{{ edited_headings }}{{ node }}{% endcapture %}
       {% continue %}
@@ -81,7 +76,7 @@
       {% endif %}
 
       {% if include.anchorAttrs %}
-        {% capture anchor %}{{ anchor }} {{ include.anchorAttrs | replace: '%heading%', header }}{% endcapture %}
+        {% capture anchor %}{{ anchor }} {{ include.anchorAttrs }}{% endcapture %}
       {% endif %}
 
       {% capture anchor %}<a {{ anchor }}>{{ include.anchorBody | replace: '%heading%', header | default: '' }}</a>{% endcapture %}
@@ -107,4 +102,4 @@
     {% endcapture %}
     {% capture edited_headings %}{{ edited_headings }}{{ new_heading }}{% endcapture %}
   {% endfor %}
-{% endcapture %}{% assign headingsWorkspace = '' %}{{ edited_headings | strip }}
+{% endcapture %}{% assign headingsWorkspace = '' %}{{ edited_headings | strip }}
\ No newline at end of file
diff --git a/site/_layouts/docs.html b/site/_layouts/docs.html
index e8461aed..f22f5401 100644
--- a/site/_layouts/docs.html
+++ b/site/_layouts/docs.html
@@ -31,12 +31,7 @@
               </div>
               <div class="col-md-8">
               <div class="documentation-container">
-                {% include anchor_headings.html
-                     html=content
-                     anchorClass="anchor-link"
-                     anchorBody='<i class="fas fa-link"></i><noscript>#</noscript>'
-                     anchorAttrs='aria-label="Link to &quot;%heading%&quot;"'
-                %}
+                {% include anchor_headings.html html=content %}
               </div>
               </div>
             </div>
diff --git a/site/_scss/site/common/_type.scss b/site/_scss/site/common/_type.scss
index 55533666..f5dfaacf 100644
--- a/site/_scss/site/common/_type.scss
+++ b/site/_scss/site/common/_type.scss
@@ -6,11 +6,6 @@ h4,
 h5,
 h6 {
   font-weight: $font-weight-medium;
-
-  a.anchor-link {
-    font-size: 0.55em;
-    vertical-align: middle;
-  }
 }
 
 h1,
diff --git a/web/src/app/modules/shared/components/presentation/overflow-labels/overflow-labels.component.html b/web/src/app/modules/shared/components/presentation/overflow-labels/overflow-labels.component.html
index 0aa20307..eb137b12 100644
--- a/web/src/app/modules/shared/components/presentation/overflow-labels/overflow-labels.component.html
+++ b/web/src/app/modules/shared/components/presentation/overflow-labels/overflow-labels.component.html
@@ -1,16 +1,13 @@
 <div class="labels-wrapper" *ngIf="labels">
-  <div class="labels-container">
-    <ng-container *ngFor="let label of showLabels; trackBy: trackByIdentity">
-        <span
-          *ngFor="let item of label | keyvalue; trackBy: trackByIdentity"
-          class="label label-light-blue clickable"
-          [title]="item.key+':'+item.value"
-          (click)="filterLabel(item.key, item.value)"
-        >
-          {{ item.key }}:{{ item.value }}
-        </span>
-    </ng-container>
-  </div>
+  <ng-container *ngFor="let label of showLabels; trackBy: trackByIdentity">
+    <span
+      *ngFor="let item of label | keyvalue; trackBy: trackByIdentity"
+      class="label label-light-blue clickable"
+      (click)="filterLabel(item.key, item.value)"
+    >
+      {{ item.key }}:{{ item.value }}
+    </span>
+  </ng-container>
 
   <clr-signpost *ngIf="overflowLabels && overflowLabels.length > 0">
     <span class="badge badge-light-blue" clrSignpostTrigger>
diff --git a/web/src/app/modules/shared/components/presentation/overflow-labels/overflow-labels.component.scss b/web/src/app/modules/shared/components/presentation/overflow-labels/overflow-labels.component.scss
index fc9d44c1..cfc266a3 100644
--- a/web/src/app/modules/shared/components/presentation/overflow-labels/overflow-labels.component.scss
+++ b/web/src/app/modules/shared/components/presentation/overflow-labels/overflow-labels.component.scss
@@ -5,22 +5,10 @@
 .labels-wrapper {
   display: flex;
   align-items: center;
+  flex-wrap: nowrap;
 
-  .labels-container {
-    display: grid;
-    grid-auto-flow: column;
-
-    .label {
-      display: inline-block;
-      line-height: 0.9rem;
-      white-space: nowrap;
-      overflow: hidden;
-      text-overflow: ellipsis;
-
-      &.clickable {
-        cursor: pointer;
-      }
-    }
+  .label.clickable {
+    cursor: pointer;
   }
 }
 
diff --git a/web/src/styles.scss b/web/src/styles.scss
index 0a6c44f2..3d91225a 100644
--- a/web/src/styles.scss
+++ b/web/src/styles.scss
@@ -10,6 +10,17 @@
 @import '~highlight.js/styles/github.css';
 @import '~@ng-select/ng-select/themes/default.theme.css';
 
+/* Clarity's Datagrid overflow is hidden making impossible to display something 
+ * like a Signpost inside of it. This makes it possible.
+ */ 
+clr-datagrid .datagrid-outer-wrapper {
+  &, .datagrid-inner-wrapper {
+    &, .datagrid {
+      overflow: visible;
+    }
+  }
+}
+
 // Avoid Datagrid's header sticky position.
 .datagrid-table .datagrid-header {
   position: initial;


Test patch:
diff --git a/internal/printer/container_test.go b/internal/printer/container_test.go
index d7927d91..c60c72c5 100644
--- a/internal/printer/container_test.go
+++ b/internal/printer/container_test.go
@@ -364,7 +364,7 @@ func Test_ContainerConfiguration(t *testing.T) {
 				tpo.objectStore.EXPECT().Get(ctx, gomock.Eq(key)).Return(testutil.ToUnstructured(t, configMap), nil).AnyTimes()
 			}
 
-			cc := NewContainerConfiguration(ctx, parentPod, tc.container, pf, tc.isInit, printOptions)
+			cc := NewContainerConfiguration(ctx, parentPod, tc.container, pf, IsInit(tc.isInit), WithPrintOptions(printOptions))
 			summary, err := cc.Create()
 			if tc.isErr {
 				require.Error(t, err)