Merge ~ajorgens/cloud-init:simpletable into cloud-init:master

[Andrew Jorgensen]

Remove prettytable dependency, introduce simpletable

The first revision of this rendered tables with less decoration but there was a desire upstream to avoid possibly breaking some parsing someone might be doing, so it has been revised to render the same as prettytable for the cases cloud-init actually uses.

Remove prettytable dependency, introduce simpletable

simpletable has no external dependencies and very little complexity.

[Andrew Jorgensen (ajorgens)]

Noticed a mention of PrettyTable in the podcast Scott mentioned on the mailing list, so I thought I'd better post what Amazon Linux uses in place of PrettyTable.

No unit tests written yet, but hopefully I'll be able to take some time to do that soon.

This is an Amazon contribution, not an individual contribution, but I got permission from the original author anyway.

[Andrew Jorgensen (ajorgens)]

Here's an example of netinfo output from a console log:

```
ci-info: +++++++++++++++++++++++Net device info+++++++++++++++++++++++
ci-info: Device Up Address Mask Hw-Address
ci-info: lo True 127.0.0.1 255.0.0.0 .
ci-info: eth0 True 172.31.9.66 255.255.240.0 0a:bc:31:2b:2b:7f
ci-info: ++++++++++++++++++++++++++++++Route info+++++++++++++++++++++++++++++++
ci-info: Route Destination Gateway Genmask Interface Flags
ci-info: 0 0.0.0.0 172.31.0.1 0.0.0.0 eth0 UG
ci-info: 1 169.254.169.254 0.0.0.0 255.255.255.255 eth0 UH
ci-info: 2 172.31.0.0 0.0.0.0 255.255.240.0 eth0 U
```

[Andrew Jorgensen (ajorgens)]

Launchpad is collapsing spaces in the above comment, so it's useless in showing what it actually looks like. Sorry.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:69951f18fe402f983653b0c990460d04f7be32a2
https://jenkins.ubuntu.com/server/job/cloud-init-ci/284/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/284/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

Andrew, wrt useless of launchpad collapsing spaces, you can put it into a
pastebin http://paste.ubuntu.com/ and then point to that.

You dont remove the dep from requirements.txt here, so it probably ends
up still being a dep at least in ubuntu

I'd like to drop prettytable or at very least make it a soft dependency,
falling back to your simpletable if it wasnt there.
There is an issue.. though. We were using pretty table to render human
friendly output to the /dev/console. The expectation was that that was
human friendly, but since we were not rendering any *machine* friendly
representation of said data, its possible or likely that machines
were parsing that.

I'd like to have an actual solution for writing machine-friendly
data somewhere that we could depend on. And then i woudlnt care so much
about the user-friendly text changing.

This cleary isn't your fault, but I'm just kind of adverse to changing
something that kind of worked and possibly breaking someone without
putting somethign else in place that could be relied upon.

[Andrew Jorgensen (ajorgens)]

We've been using this in Amazon Linux since early 2014 and not once have we heard a complaint, or even a mention of it. I think the worry that someone is parsing routing data out of console output is probably baseless, since that data is better obtained through queries against your cloud APIs if you are outside the instance, or through OS utilities or syscalls if you are inside the instance.

FWIW, the format produced by simpletable is *more* machine readable than the output of prettytable (and prettier, if you ask me and you don't remove any spaces).

Up to you. Amazon will likely continue to carry this patch.

I'll update with a removal from requirements.txt sometime. We had removed it from our RPM .spec file and I forgot it would need to be removed elsewhere.

[Andrew Jorgensen (ajorgens)]

Okay, I went ahead and made its output look like PrettyTable (even if I'm unhappy about that), stripped out every reference to PrettyTable from the code (including requirements.txt, etc.), and wrote some unit tests that assert that the output is the same as PrettyTable for cases actually used in cloud-init.

review: Needs Resubmitting

[Andrew Jorgensen (ajorgens)]

I think I set the state wrong, setting it back to what I actually think of this.

review: Approve

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:0a3b64acdd0007b5437c37e5553f963af0942a9a
https://jenkins.ubuntu.com/server/job/cloud-init-ci/326/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/326/rebuild

review: Needs Fixing (continuous-integration)

[Andrew Jorgensen (ajorgens)]

Ugh, how embarrassing. I tried to suppress some flake8 errors and did it so wrong.

[Andrew Jorgensen (ajorgens)]

Okay, latest push should do it. 🤞

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:37383e5d99f6885934db1f127658c431b1f65ece
https://jenkins.ubuntu.com/server/job/cloud-init-ci/327/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/327/rebuild

review: Approve (continuous-integration)

[Andrew Jorgensen (ajorgens)]

> PASSED: Continuous integration, rev:37383e5d99f6885934db1f127658c431b1f65ece

Yay!

[Chad Smith (chad.smith)]

Approving pending discussion on inline comments. We probably won't land this until post 17.1 release because the change in dependencies might need a bit more testing on other platforms.

review: Approve

46732a3... by Chad Smith on 2017-09-21

[SimpleTable] Cleaner comments / Don't use StringIO

Changes from Chad Smith suggested on
https://code.launchpad.net/~ajorgens/cloud-init/+git/cloud-init/+merge/330525

Signed-off-by: Andrew Jorgensen <email address hidden>

[Andrew Jorgensen (ajorgens)]

Pulled in Chad's suggestions.

[Andrew Jorgensen (ajorgens)]

(diff comment replies)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:46732a3706e83685ce494e74bac93cae2259e0c2
https://jenkins.ubuntu.com/server/job/cloud-init-ci/338/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/338/rebuild

review: Approve (continuous-integration)

diff --git a/cloudinit/config/cc_ssh_authkey_fingerprints.py b/cloudinit/config/cc_ssh_authkey_fingerprints.py
index 0066e97..35d8c57 100755
--- a/cloudinit/config/cc_ssh_authkey_fingerprints.py
+++ b/cloudinit/config/cc_ssh_authkey_fingerprints.py
@@ -28,7 +28,7 @@ the keys can be specified, but defaults to ``md5``.
 import base64
 import hashlib
 
-from prettytable import PrettyTable
+from cloudinit.simpletable import SimpleTable
 
 from cloudinit.distros import ug_util
 from cloudinit import ssh_util
@@ -74,7 +74,7 @@ def _pprint_key_entries(user, key_fn, key_entries, hash_meth='md5',
         return
     tbl_fields = ['Keytype', 'Fingerprint (%s)' % (hash_meth), 'Options',
                   'Comment']
-    tbl = PrettyTable(tbl_fields)
+    tbl = SimpleTable(tbl_fields)
     for entry in key_entries:
         if _is_printable_key(entry):
             row = []
diff --git a/cloudinit/netinfo.py b/cloudinit/netinfo.py
index 39c79de..8f99d99 100644
--- a/cloudinit/netinfo.py
+++ b/cloudinit/netinfo.py
@@ -13,7 +13,7 @@ import re
 from cloudinit import log as logging
 from cloudinit import util
 
-from prettytable import PrettyTable
+from cloudinit.simpletable import SimpleTable
 
 LOG = logging.getLogger()
 
@@ -170,7 +170,7 @@ def netdev_pformat():
         lines.append(util.center("Net device info failed", '!', 80))
     else:
         fields = ['Device', 'Up', 'Address', 'Mask', 'Scope', 'Hw-Address']
-        tbl = PrettyTable(fields)
+        tbl = SimpleTable(fields)
         for (dev, d) in netdev.items():
             tbl.add_row([dev, d["up"], d["addr"], d["mask"], ".", d["hwaddr"]])
             if d.get('addr6'):
@@ -194,7 +194,7 @@ def route_pformat():
         if routes.get('ipv4'):
             fields_v4 = ['Route', 'Destination', 'Gateway',
                          'Genmask', 'Interface', 'Flags']
-            tbl_v4 = PrettyTable(fields_v4)
+            tbl_v4 = SimpleTable(fields_v4)
             for (n, r) in enumerate(routes.get('ipv4')):
                 route_id = str(n)
                 tbl_v4.add_row([route_id, r['destination'],
@@ -207,7 +207,7 @@ def route_pformat():
         if routes.get('ipv6'):
             fields_v6 = ['Route', 'Proto', 'Recv-Q', 'Send-Q',
                          'Local Address', 'Foreign Address', 'State']
-            tbl_v6 = PrettyTable(fields_v6)
+            tbl_v6 = SimpleTable(fields_v6)
             for (n, r) in enumerate(routes.get('ipv6')):
                 route_id = str(n)
                 tbl_v6.add_row([route_id, r['proto'],
diff --git a/cloudinit/simpletable.py b/cloudinit/simpletable.py
new file mode 100644
index 0000000..9060322
--- /dev/null
+++ b/cloudinit/simpletable.py
@@ -0,0 +1,62 @@
+# Copyright (C) 2017 Amazon.com, Inc. or its affiliates
+#
+# Author: Ethan Faust <[e-mail]>
+# Author: Andrew Jorgensen <[e-mail]>
+#
+# This file is part of cloud-init. See LICENSE file for license information.
+
+
+class SimpleTable(object):
+    """A minimal implementation of PrettyTable
+    for distribution with cloud-init.
+    """
+
+    def __init__(self, fields):
+        self.fields = fields
+        self.rows = []
+
+        # initialize list of 0s the same length
+        # as the number of fields
+        self.column_widths = [0] * len(self.fields)
+        self.update_column_widths(fields)
+
+    def update_column_widths(self, values):
+        for i, value in enumerate(values):
+            self.column_widths[i] = max(
+                len(value),
+                self.column_widths[i])
+
+    def add_row(self, values):
+        if len(values) > len(self.fields):
+            raise TypeError('too many values')
+        values = [str(value) for value in values]
+        self.rows.append(values)
+        self.update_column_widths(values)
+
+    def _hdiv(self):
+        """Returns a horizontal divider for the table."""
+        return '+' + '+'.join(
+            ['-' * (w + 2) for w in self.column_widths]) + '+'
+
+    def _row(self, row):
+        """Returns a formatted row."""
+        return '|' + '|'.join(
+            [col.center(self.column_widths[i] + 2)
+                for i, col in enumerate(row)]) + '|'
+
+    def __str__(self):
+        """Returns a string representation of the table with lines around.
+
+        +-----+-----+
+        | one | two |
+        +-----+-----+
+        |  1  |  2  |
+        |  01 |  10 |
+        +-----+-----+
+        """
+        lines = [self._hdiv(), self._row(self.fields), self._hdiv()]
+        lines += [self._row(r) for r in self.rows] + [self._hdiv()]
+        return '\n'.join(lines)
+
+    def get_string(self):
+        return repr(self)
diff --git a/cloudinit/tests/test_simpletable.py b/cloudinit/tests/test_simpletable.py
new file mode 100644
index 0000000..96bc24c
--- /dev/null
+++ b/cloudinit/tests/test_simpletable.py
@@ -0,0 +1,100 @@
+# Copyright (C) 2017 Amazon.com, Inc. or its affiliates
+#
+# Author: Andrew Jorgensen <[e-mail]>
+#
+# This file is part of cloud-init. See LICENSE file for license information.
+"""Tests that SimpleTable works just like PrettyTable for cloud-init.
+
+Not all possible PrettyTable cases are tested because we're not trying to
+reimplement the entire library, only the minimal parts we actually use.
+"""
+
+from cloudinit.simpletable import SimpleTable
+from cloudinit.tests.helpers import CiTestCase
+
+# Examples rendered by cloud-init using PrettyTable
+NET_DEVICE_FIELDS = (
+    'Device', 'Up', 'Address', 'Mask', 'Scope', 'Hw-Address')
+NET_DEVICE_ROWS = (
+    ('ens3', True, '172.31.4.203', '255.255.240.0', '.', '0a:1f:07:15:98:70'),
+    ('ens3', True, 'fe80::81f:7ff:fe15:9870/64', '.', 'link',
+        '0a:1f:07:15:98:70'),
+    ('lo', True, '127.0.0.1', '255.0.0.0', '.', '.'),
+    ('lo', True, '::1/128', '.', 'host', '.'),
+)
+NET_DEVICE_TABLE = """\
++--------+------+----------------------------+---------------+-------+-------------------+
+| Device |  Up  |          Address           |      Mask     | Scope |     Hw-Address    |
++--------+------+----------------------------+---------------+-------+-------------------+
+|  ens3  | True |        172.31.4.203        | 255.255.240.0 |   .   | 0a:1f:07:15:98:70 |
+|  ens3  | True | fe80::81f:7ff:fe15:9870/64 |       .       |  link | 0a:1f:07:15:98:70 |
+|   lo   | True |         127.0.0.1          |   255.0.0.0   |   .   |         .         |
+|   lo   | True |          ::1/128           |       .       |  host |         .         |
++--------+------+----------------------------+---------------+-------+-------------------+"""  # noqa: E501
+ROUTE_IPV4_FIELDS = (
+    'Route', 'Destination', 'Gateway', 'Genmask', 'Interface', 'Flags')
+ROUTE_IPV4_ROWS = (
+    ('0', '0.0.0.0', '172.31.0.1', '0.0.0.0', 'ens3', 'UG'),
+    ('1', '169.254.0.0', '0.0.0.0', '255.255.0.0', 'ens3', 'U'),
+    ('2', '172.31.0.0', '0.0.0.0', '255.255.240.0', 'ens3', 'U'),
+)
+ROUTE_IPV4_TABLE = """\
++-------+-------------+------------+---------------+-----------+-------+
+| Route | Destination |  Gateway   |    Genmask    | Interface | Flags |
++-------+-------------+------------+---------------+-----------+-------+
+|   0   |   0.0.0.0   | 172.31.0.1 |    0.0.0.0    |    ens3   |   UG  |
+|   1   | 169.254.0.0 |  0.0.0.0   |  255.255.0.0  |    ens3   |   U   |
+|   2   |  172.31.0.0 |  0.0.0.0   | 255.255.240.0 |    ens3   |   U   |
++-------+-------------+------------+---------------+-----------+-------+"""
+
+AUTHORIZED_KEYS_FIELDS = (
+    'Keytype', 'Fingerprint (md5)', 'Options', 'Comment')
+AUTHORIZED_KEYS_ROWS = (
+    ('ssh-rsa', '24:c7:41:49:47:12:31:a0:de:6f:62:79:9b:13:06:36', '-',
+        'ajorgens'),
+)
+AUTHORIZED_KEYS_TABLE = """\
++---------+-------------------------------------------------+---------+----------+
+| Keytype |                Fingerprint (md5)                | Options | Comment  |
++---------+-------------------------------------------------+---------+----------+
+| ssh-rsa | 24:c7:41:49:47:12:31:a0:de:6f:62:79:9b:13:06:36 |    -    | ajorgens |
++---------+-------------------------------------------------+---------+----------+"""  # noqa: E501
+
+# from prettytable import PrettyTable
+# pt = PrettyTable(('HEADER',))
+# print(pt)
+NO_ROWS_FIELDS = ('HEADER',)
+NO_ROWS_TABLE = """\
++--------+
+| HEADER |
++--------+
++--------+"""
+
+
+class TestSimpleTable(CiTestCase):
+
+    def test_no_rows(self):
+        """An empty table is rendered as PrettyTable would have done it."""
+        table = SimpleTable(NO_ROWS_FIELDS)
+        self.assertEqual(str(table), NO_ROWS_TABLE)
+
+    def test_net_dev(self):
+        """Net device info is rendered as it was with PrettyTable."""
+        table = SimpleTable(NET_DEVICE_FIELDS)
+        for row in NET_DEVICE_ROWS:
+            table.add_row(row)
+        self.assertEqual(str(table), NET_DEVICE_TABLE)
+
+    def test_route_ipv4(self):
+        """Route IPv4 info is rendered as it was with PrettyTable."""
+        table = SimpleTable(ROUTE_IPV4_FIELDS)
+        for row in ROUTE_IPV4_ROWS:
+            table.add_row(row)
+        self.assertEqual(str(table), ROUTE_IPV4_TABLE)
+
+    def test_authorized_keys(self):
+        """SSH authorized keys are rendered as they were with PrettyTable."""
+        table = SimpleTable(AUTHORIZED_KEYS_FIELDS)
+        for row in AUTHORIZED_KEYS_ROWS:
+            table.add_row(row)
+        self.assertEqual(str(table), AUTHORIZED_KEYS_TABLE)
diff --git a/packages/pkg-deps.json b/packages/pkg-deps.json
index 822d29d..72409dd 100644
--- a/packages/pkg-deps.json
+++ b/packages/pkg-deps.json
@@ -34,9 +34,6 @@
          "jsonschema" : {
             "3" : "python34-jsonschema"
          },
-         "prettytable" : {
-            "3" : "python34-prettytable"
-         },
          "pyflakes" : {
             "2" : "pyflakes",
             "3" : "python34-pyflakes"
diff --git a/requirements.txt b/requirements.txt
index 61d1e90..dd10d85 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -3,9 +3,6 @@
 # Used for untemplating any files or strings with parameters.
 jinja2
 
-# This is used for any pretty printing of tabular data.
-PrettyTable
-
 # This one is currently only used by the MAAS datasource. If that
 # datasource is removed, this is no longer needed
 oauthlib
diff --git a/tools/build-on-freebsd b/tools/build-on-freebsd
index ff9153a..d23fde2 100755
--- a/tools/build-on-freebsd
+++ b/tools/build-on-freebsd
@@ -18,7 +18,6 @@ pkgs="
    py27-jsonpatch
    py27-jsonpointer
    py27-oauthlib
-   py27-prettytable
    py27-requests
    py27-serial
    py27-six
diff --git a/tox.ini b/tox.ini
index 776f425..aef1f84 100644
--- a/tox.ini
+++ b/tox.ini
@@ -64,7 +64,6 @@ deps =
     # requirements
     jinja2==2.8
     pyyaml==3.11
-    PrettyTable==0.7.2
     oauthlib==1.0.3
     pyserial==3.0.1
     configobj==5.0.6
@@ -89,7 +88,6 @@ deps =
     argparse==1.2.1
     jinja2==2.2.1
     pyyaml==3.10
-    PrettyTable==0.7.2
     oauthlib==0.6.0
     configobj==4.6.0
     requests==2.6.0
@@ -105,7 +103,6 @@ deps =
     argparse==1.3.0
     jinja2==2.8
     PyYAML==3.11
-    PrettyTable==0.7.2
     oauthlib==0.7.2
     configobj==5.0.6
     requests==2.11.1