Table-mode: sort ips numerically (#2007)

Fix #1746 - sort IPs numerically in the table mode

.gitignore

@@ -8,6 +8,7 @@ _obj
 _test
 .vagrant
 releases
+tmp
 
 # Architecture specific extensions/prefixes
 *.[568vq]

client/app/scripts/components/node-details/__tests__/node-details-table-test.js

@@ -0,0 +1,106 @@
+import React from 'react';
+import TestUtils from 'react/lib/ReactTestUtils';
+import { Provider } from 'react-redux';
+import configureStore from '../../../stores/configureStore';
+
+// need ES5 require to keep automocking off
+const NodeDetailsTable = require('../node-details-table.js').default;
+
+describe('NodeDetailsTable', () => {
+  let nodes, columns, component;
+
+  beforeEach(() => {
+    columns = [
+      { id: 'kubernetes_ip', label: 'IP', dataType: 'ip' },
+      { id: 'kubernetes_namespace', label: 'Namespace' },
+    ];
+    nodes = [
+      {
+        id: 'node-1',
+        metadata: [
+          { id: 'kubernetes_ip', label: 'IP', value: '10.244.253.24' },
+          { id: 'kubernetes_namespace', label: 'Namespace', value: '1111' },
+        ]
+      }, {
+        id: 'node-2',
+        metadata: [
+          { id: 'kubernetes_ip', label: 'IP', value: '10.244.253.4' },
+          { id: 'kubernetes_namespace', label: 'Namespace', value: '12' },
+        ]
+      }, {
+        id: 'node-3',
+        metadata: [
+          { id: 'kubernetes_ip', label: 'IP', value: '10.44.253.255' },
+          { id: 'kubernetes_namespace', label: 'Namespace', value: '5' },
+        ]
+      }, {
+        id: 'node-4',
+        metadata: [
+          { id: 'kubernetes_ip', label: 'IP', value: '10.244.253.100' },
+          { id: 'kubernetes_namespace', label: 'Namespace', value: '00000' },
+        ]
+      },
+    ];
+  });
+
+  function matchColumnValues(columnLabel, expectedValues) {
+    // Get the index of the column whose values we want to match.
+    const columnIndex = columns.findIndex(column => column.id === columnLabel);
+    // Get all the values rendered in the table.
+    const values = TestUtils.scryRenderedDOMComponentsWithClass(component, 'node-details-table-node-value').map(d => d.title);
+    // Since we are interested only in the values that appear in the column `columnIndex`, we drop the rest.
+    // As `values` are ordered by appearance in the DOM structure (that is, first by row and then by column),
+    // the indexes we are interested in are of the form columnIndex + n * columns.length, where n >= 0.
+    // Therefore we take only the values at the index which divided by columns.length gives a reminder columnIndex.
+    const filteredValues = values.filter((element, index) => index % columns.length === columnIndex);
+    // Array comparison
+    expect(filteredValues).toEqual(expectedValues);
+  }
+
+  function clickColumn(title) {
+    const node = TestUtils.scryRenderedDOMComponentsWithTag(component, 'td').find(d => d.title === title);
+    TestUtils.Simulate.click(node);
+  }
+
+  describe('kubernetes_ip', () => {
+    it('sorts by column', () => {
+      component = TestUtils.renderIntoDocument(
+        <Provider store={configureStore()}>
+          <NodeDetailsTable
+            columns={columns}
+            sortBy="kubernetes_ip"
+            nodeIdKey="id"
+            nodes={nodes}
+          />
+        </Provider>
+      );
+
+      matchColumnValues('kubernetes_ip', ['10.44.253.255', '10.244.253.4', '10.244.253.24', '10.244.253.100']);
+      clickColumn('IP');
+      matchColumnValues('kubernetes_ip', ['10.244.253.100', '10.244.253.24', '10.244.253.4', '10.44.253.255']);
+      clickColumn('IP');
+      matchColumnValues('kubernetes_ip', ['10.44.253.255', '10.244.253.4', '10.244.253.24', '10.244.253.100']);
+    });
+  });
+
+  describe('kubernetes_namespace', () => {
+    it('sorts by column', () => {
+      component = TestUtils.renderIntoDocument(
+        <Provider store={configureStore()}>
+          <NodeDetailsTable
+            columns={columns}
+            sortBy="kubernetes_namespace"
+            nodeIdKey="id"
+            nodes={nodes}
+          />
+        </Provider>
+      );
+
+      matchColumnValues('kubernetes_namespace', ['00000', '1111', '12', '5']);
+      clickColumn('Namespace');
+      matchColumnValues('kubernetes_namespace', ['5', '12', '1111', '00000']);
+      clickColumn('Namespace');
+      matchColumnValues('kubernetes_namespace', ['00000', '1111', '12', '5']);
+    });
+  });
+});

client/app/scripts/components/node-details/node-details-table.js

@@ -4,12 +4,19 @@ import classNames from 'classnames';
 
 import ShowMore from '../show-more';
 import NodeDetailsTableRow from './node-details-table-row';
+import { ipToPaddedString } from '../../utils/string-utils';
 
 
 function isNumber(data) {
   return data.dataType && data.dataType === 'number';
 }
 
+
+function isIP(data) {
+  return data.dataType && data.dataType === 'ip';
+}
+
+
 const CW = {
   XS: '32px',
   S: '50px',
@@ -80,7 +87,10 @@ function getNodeValue(node, header) {
     let field = _.union(node.metrics, node.metadata).find(f => f.id === fieldId);
 
     if (field) {
-      if (isNumber(header)) {
+      if (isIP(header)) {
+        // Format the IPs so that they are sorted numerically.
+        return ipToPaddedString(field.value);
+      } else if (isNumber(header)) {
         return parseFloat(field.value);
       }
       return field.value;

client/app/scripts/utils/__tests__/string-utils-test.js

@@ -3,21 +3,30 @@ describe('StringUtils', () => {
   const StringUtils = require('../string-utils');
 
   describe('formatMetric', () => {
-    const formatMetric = StringUtils.formatMetric;
+    const f = StringUtils.formatMetric;
 
     it('it should render 0', () => {
-      expect(formatMetric(0)).toBe('0.00');
+      expect(f(0)).toBe('0.00');
     });
   });
 
   describe('longestCommonPrefix', () => {
-    const fun = StringUtils.longestCommonPrefix;
+    const f = StringUtils.longestCommonPrefix;
 
     it('it should return the longest common prefix', () => {
-      expect(fun(['interspecies', 'interstellar'])).toBe('inters');
-      expect(fun(['space', 'space'])).toBe('space');
-      expect(fun([''])).toBe('');
-      expect(fun(['prefix', 'suffix'])).toBe('');
+      expect(f(['interspecies', 'interstellar'])).toBe('inters');
+      expect(f(['space', 'space'])).toBe('space');
+      expect(f([''])).toBe('');
+      expect(f(['prefix', 'suffix'])).toBe('');
+    });
+  });
+
+  describe('ipToPaddedString', () => {
+    const f = StringUtils.ipToPaddedString;
+
+    it('it should return the formatted IP', () => {
+      expect(f('10.244.253.4')).toBe('010.244.253.004');
+      expect(f('0.24.3.4')).toBe('000.024.003.004');
     });
   });
 });

client/app/scripts/utils/string-utils.js

@@ -22,6 +22,11 @@ function renderSvg(text, unit) {
 }
 
 
+function padToThreeDigits(n) {
+  return `000${n}`.slice(-3);
+}
+
+
 function makeFormatters(renderFn) {
   const formatters = {
     filesize(value) {
@@ -75,6 +80,11 @@ export function longestCommonPrefix(strArr) {
   return (new LCP(strArr)).lcp();
 }
 
+// Converts IPs from '10.244.253.4' to '010.244.253.004' format.
+export function ipToPaddedString(value) {
+  return value.match(/\d+/g).map(padToThreeDigits).join('.');
+}
+
 // Formats metadata values. Add a key to the `formatters` obj
 // that matches the `dataType` of the field. You must return an Object
 // with the keys `value` and `title` defined.

probe/kubernetes/reporter.go

@@ -28,7 +28,7 @@ const (
 var (
 	PodMetadataTemplates = report.MetadataTemplates{
 		State:            {ID: State, Label: "State", From: report.FromLatest, Priority: 2},
-		IP:               {ID: IP, Label: "IP", From: report.FromLatest, Priority: 3},
+		IP:               {ID: IP, Label: "IP", From: report.FromLatest, Datatype: "ip", Priority: 3},
 		report.Container: {ID: report.Container, Label: "# Containers", From: report.FromCounters, Datatype: "number", Priority: 4},
 		Namespace:        {ID: Namespace, Label: "Namespace", From: report.FromLatest, Priority: 5},
 		Created:          {ID: Created, Label: "Created", From: report.FromLatest, Datatype: "datetime", Priority: 6},
@@ -39,8 +39,8 @@ var (
 	ServiceMetadataTemplates = report.MetadataTemplates{
 		Namespace:  {ID: Namespace, Label: "Namespace", From: report.FromLatest, Priority: 2},
 		Created:    {ID: Created, Label: "Created", From: report.FromLatest, Datatype: "datetime", Priority: 3},
-		PublicIP:   {ID: PublicIP, Label: "Public IP", From: report.FromLatest, Priority: 4},
-		IP:         {ID: IP, Label: "Internal IP", From: report.FromLatest, Priority: 5},
+		PublicIP:   {ID: PublicIP, Label: "Public IP", From: report.FromLatest, Datatype: "ip", Priority: 4},
+		IP:         {ID: IP, Label: "Internal IP", From: report.FromLatest, Datatype: "ip", Priority: 5},
 		report.Pod: {ID: report.Pod, Label: "# Pods", From: report.FromCounters, Datatype: "number", Priority: 6},
 	}
 

render/detailed/node.go

@@ -148,7 +148,7 @@ var (
 				Label:      "Services",
 				Columns: []Column{
 					{ID: report.Pod, Label: "# Pods", Datatype: "number"},
-					{ID: kubernetes.IP, Label: "IP"},
+					{ID: kubernetes.IP, Label: "IP", Datatype: "ip"},
 				},
 			},
 		},
@@ -172,7 +172,7 @@ var (
 				Columns: []Column{
 					{ID: kubernetes.State, Label: "State"},
 					{ID: report.Container, Label: "# Containers", Datatype: "number"},
-					{ID: kubernetes.IP, Label: "IP"},
+					{ID: kubernetes.IP, Label: "IP", Datatype: "ip"},
 				},
 			},
 		},

render/detailed/node_test.go

@@ -106,7 +106,7 @@ func TestMakeDetailedHostNode(t *testing.T) {
 				Columns: []detailed.Column{
 					{ID: kubernetes.State, Label: "State"},
 					{ID: report.Container, Label: "# Containers", Datatype: "number"},
-					{ID: kubernetes.IP, Label: "IP"},
+					{ID: kubernetes.IP, Label: "IP", Datatype: "ip"},
 				},
 				Nodes: []detailed.NodeSummary{podNodeSummary},
 			},