You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2018/11/06 22:22:48 UTC
[GitHub] dangogh closed pull request #2967: Fixed compare tool generating
diffs based on timestamp in configuration file headers
dangogh closed pull request #2967: Fixed compare tool generating diffs based on timestamp in configuration file headers
URL: https://github.com/apache/trafficcontrol/pull/2967
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/docs/source/tools/compare.rst b/docs/source/tools/compare.rst
index 165d7c692..37996d72d 100644
--- a/docs/source/tools/compare.rst
+++ b/docs/source/tools/compare.rst
@@ -72,34 +72,32 @@ These can be overridden by command line switches as described above. If a userna
genConfigRoutes.py
------------------
-usage: genConfigRoutes.py [-h] [-k] [-v] InstanceA InstanceB LoginA [LoginB]
--h, --help show this help message and exit
--k, --insecure Do not verify SSL certificate signatures against *either* Traffic Ops instance (default: False)
--l LOG_LEVEL, --log_level LOG_LEVEL Sets the Python log level, one of 'DEBUG', 'INFO', 'WARN', 'ERROR', or 'CRITICAL'
--q, --quiet Suppresses all logging output - even for critical errors
--v, --version Print version information and exit
+``usage: genConfigRoutes.py [-h] [--refURL REFURL] [--testURL TESTURL]``
+ ``[--refUser REFUSER] [--refPasswd REFPASSWD]``
+ ``[--testUser TESTUSER] [--testPasswd TESTPASSWD] [-k]``
+ ``[-v] [-l LOG_LEVEL] [-q]``
-.. note:: If you're using a CDN-in-a-Box environment for testing, it's likely that you'll need the ``-k``/``--insecure`` option if you're outside the Docker network
+A simple script to generate API routes to server configuration files for a
+given pair of Traffic Ops instances. This, for the purpose of using the
+'compare' tool
-.. _compare-genConfigRoutes-positional_parameters:
-.. table:: Positional Parameters (In Order)
+-h, --help show this help message and exit
+--refURL REFURL The full URL of the reference Traffic Ops instance (default: None)
+--testURL TESTURL The full URL of the testing Traffic Ops instance (default: None)
+--refUser REFUSER A username for logging into the reference Traffic Ops instance. (default: None)
+--refPasswd REFPASSWD A password for logging into the reference Traffic Ops instance (default: None)
+--testUser TESTUSER A username for logging into the testing Traffic Ops instance. If not given, the value for the reference instance will be used. (default: None)
+--testPasswd TESTPASSWD A password for logging into the testing Traffic Ops instance. If not given, the value for the reference instance will be used. (default: None)
+-k, --insecure Do not verify SSL certificate signatures against *either* Traffic Ops instance (default: False)
+-v, --version Print version information and exit
+-l LOG_LEVEL, --log_level LOG_LEVEL Sets the Python log level, one of 'DEBUG', 'INFO', 'WARN', 'ERROR', or 'CRITICAL' (default: INFO)
+-q, --quiet Suppresses all logging output - even for critical errors (default: False)
- +-----------+---------------------------------------------------------------------------------------------------------------------------------------+
- | Name | Description |
- +===========+=======================================================================================================================================+
- | InstanceA | The full URL of the first Traffic Ops instance |
- +-----------+---------------------------------------------------------------------------------------------------------------------------------------+
- | InstanceB | The full URL of the second Traffic Ops instance |
- +-----------+---------------------------------------------------------------------------------------------------------------------------------------+
- | LoginA | A login string containing credentials for logging into the first Traffic Ops instance (InstanceA) in the format 'username:password'. |
- +-----------+---------------------------------------------------------------------------------------------------------------------------------------+
- | LoginB | A login string containing credentials for logging into the second Traffic Ops instance (InstanceB) in the format 'username:password'. |
- | | If not given, LoginA will be re-used for the second connection (default: None) |
- +-----------+---------------------------------------------------------------------------------------------------------------------------------------+
+.. note:: If you're using a CDN-in-a-Box environment for testing, it's likely that you'll need the ``-k``/``--insecure`` option if you're outside the Docker network
-.. note:: The full behaviour of the ``LoginB`` parameter described in :ref:`compare-genConfigRoutes-positional_parameters` is such that supports not only a fully missing authentication credentials pair, but also a blank string for each of the pair members. This means that you can, for instance, give it the testing pair 'admin:' to use the testing username 'admin', but default to the password used by the reference user. Or, another example is that passing ``LoginB`` as simple ``:`` will result in the same behaviour as not passing it all.
+.. note:: This script will use the same environment variables as `compare`, which can be overridden by the above command line parameters
The genConfigRoutes.py script will output list of unique API routes (relative to the desired Traffic Ops URL) that point to generated configuration files for a sample set of servers common to both Traffic Ops instances. The results are printed to stdout, making the output perfect for piping directly into ``compare`` like so:
@@ -128,6 +126,7 @@ TEST_PASSWORD
.. code-block:: shell
:caption: Sample Script to Build and Run
+
sudo docker build . -f traffic_ops/testing/compare/Dockerfile -t compare:latest
sudo docker run -v $PWD/artifacts:/artifacts -e TO_URL="$TO_URL" -e TEST_URL="$TEST_URL" -e TO_USER="admin" -e TO_PASSWORD="twelve" -e TEST_USER="admin" -e TEST_PASSWORD="twelve" compare:latest
diff --git a/traffic_ops/testing/compare/Dockerfile b/traffic_ops/testing/compare/Dockerfile
index e11041734..c7bab63dc 100644
--- a/traffic_ops/testing/compare/Dockerfile
+++ b/traffic_ops/testing/compare/Dockerfile
@@ -44,5 +44,5 @@ RUN go get -v ./...
RUN go build .
RUN cp testroutes.txt /artifacts/
-CMD ./genConfigRoutes.py -k $TO_URL $TEST_URL $TO_USER:$TO_PASSWORD $TEST_USER:$TEST_PASSWORD -l INFO 2>&1 >>/artifacts/testroutes.txt | tee /artifacts/genRoutesConfig.log &&\
- ./compare --ref_url=$TO_URL --test_url=$TEST_URL --ref_user=$TO_USER --ref_passwd=$TO_PASSWORD --test_user=$TEST_USER --test_passwd=$TEST_PASSWORD -r /artifacts </artifacts/testroutes.txt
+CMD ./genConfigRoutes.py -k --refURL=$TO_URL --testURL=$TEST_URL --refUser=$TO_USER --refPasswd=$TO_PASSWORD --testUser=$TEST_USER --testPasswd=$TEST_PASSWORD -l INFO 2>&1 >>/artifacts/testroutes.txt | tee /artifacts/genRoutesConfig.log &&\
+ ./compare -s --ref_url=$TO_URL --test_url=$TEST_URL --ref_user=$TO_USER --ref_passwd=$TO_PASSWORD --test_user=$TEST_USER --test_passwd=$TEST_PASSWORD -r /artifacts </artifacts/testroutes.txt
diff --git a/traffic_ops/testing/compare/README.md b/traffic_ops/testing/compare/README.md
index f7fc302fa..4bc594e06 100644
--- a/traffic_ops/testing/compare/README.md
+++ b/traffic_ops/testing/compare/README.md
@@ -70,17 +70,47 @@ These can be overridden by command line switches as described above. If a userna
### genConfigRoutes.py
-usage: genConfigRoutes.py \[-h\] \[-k\] \[-v\] InstanceA InstanceB LoginA \[LoginB\]
+usage: genConfigRoutes.py [-h] [--refURL REFURL] [--testURL TESTURL]
+ [--refUser REFUSER] [--refPasswd REFPASSWD]
+ [--testUser TESTUSER] [--testPasswd TESTPASSWD] [-k]
+ [-v] [-l LOG_LEVEL] [-q]
+
+A simple script to generate API routes to server configuration files for a
+given pair of Traffic Ops instances. This, for the purpose of using the
+'compare' tool
+
+optional arguments:
+ -h, --help show this help message and exit
+ --refURL REFURL The full URL of the reference Traffic Ops instance
+ (default: None)
+ --testURL TESTURL The full URL of the testing Traffic Ops instance
+ (default: None)
+ --refUser REFUSER A username for logging into the reference Traffic Ops
+ instance. (default: None)
+ --refPasswd REFPASSWD
+ A password for logging into the reference Traffic Ops
+ instance (default: None)
+ --testUser TESTUSER A username for logging into the testing Traffic Ops
+ instance. If not given, the value for the reference
+ instance will be used. (default: None)
+ --testPasswd TESTPASSWD
+ A password for logging into the testing Traffic Ops
+ instance. If not given, the value for the reference
+ instance will be used. (default: None)
+ -k, --insecure Do not verify SSL certificate signatures against
+ *either* Traffic Ops instance (default: False)
+ -v, --version Print version information and exit
+ -l LOG_LEVEL, --log_level LOG_LEVEL
+ Sets the Python log level, one of 'DEBUG', 'INFO',
+ 'WARN', 'ERROR', or 'CRITICAL' (default: INFO)
+ -q, --quiet Suppresses all logging output - even for critical
+ errors (default: False)
--h, --help show this help message and exit
--k, --insecure Do not verify SSL certificate signatures against *either* Traffic Ops instance (default: False)
--l LOG_LEVEL, --log_level LOG_LEVEL Sets the Python log level, one of 'DEBUG', 'INFO', 'WARN', 'ERROR', or 'CRITICAL' (default: CRITICAL)
--q, --quiet Suppresses all logging output - even for critical errors
--v, --version Print version information and exit
+!!! note
+ This script will use the same environment variables as `compare`, which can be overridden by the above command line parameters
-> **note**
->
-> If you're using a CDN-in-a-Box environment for testing, it's likely that you'll need the `-k`/`--insecure` option if you're outside the Docker network
+!!! note
+ If you're using a CDN-in-a-Box environment for testing, it's likely that you'll need the `-k`/`--insecure` option if you're outside the Docker network
The genConfigRoutes.py script will output list of unique API routes (relative to the desired Traffic Ops URL) that point to generated configuration files for a sample set of servers common to both Traffic Ops instances. The results are printed to stdout, making the output perfect for piping directly into `compare` like so:
diff --git a/traffic_ops/testing/compare/compare.go b/traffic_ops/testing/compare/compare.go
index 8598f3a19..1f38871a0 100644
--- a/traffic_ops/testing/compare/compare.go
+++ b/traffic_ops/testing/compare/compare.go
@@ -35,7 +35,9 @@ import (
"golang.org/x/net/publicsuffix"
)
-const __version__ = "1.0.0"
+const __version__ = "1.1.0"
+const SHORT_HEADER = "# DO NOT EDIT"
+const LONG_HEADER = "# TRAFFIC OPS NOTE:"
// Environment variables used:
// TO_URL -- URL for reference Traffic Ops
@@ -138,7 +140,59 @@ func testRoute(tos []*Connect, route string) {
wg.Wait()
close(ch)
- if res[0].Res == res[1].Res {
+ // Check for Traffic Ops headers and remove them before comparison
+ refResult := res[0].Res
+ testResult := res[1].Res
+ if strings.Contains(route, "configfiles") {
+ refLines := strings.Split(refResult, "\n")
+ testLines := strings.Split(testResult, "\n")
+
+ // If the two files have different numbers of lines, they definitely differ
+ if len(refLines) != len(testLines) {
+ log.Print("Diffs from ", route, " written to")
+ p, err := res[0].TO.writeResults(route, refResult)
+ if err != nil {
+ log.Fatal("Error writing results for ", route)
+ }
+ log.Print(" ", p)
+ p, err = res[1].TO.writeResults(route, testResult)
+ if err != nil {
+ log.Fatal("Error writing results for ", route)
+ }
+ log.Print(" and ", p)
+ }
+
+ refResult = ""
+ testResult = ""
+
+ for i, refLine := range refLines {
+ if len(refLine) < len(SHORT_HEADER) {
+ refResult += refLine
+ } else if refLine[:len(SHORT_HEADER)] != SHORT_HEADER {
+ if len(refLine) >= len(LONG_HEADER) {
+ if refLine[:len(LONG_HEADER)] != LONG_HEADER {
+ refResult += refLine
+ }
+ } else {
+ refResult += refLine
+ }
+ }
+
+ if len(testLines[i]) < len(SHORT_HEADER) {
+ testResult += testLines[i]
+ } else if testLines[i][:len(SHORT_HEADER)] != SHORT_HEADER {
+ if len(testLines[i]) >= len(LONG_HEADER) {
+ if testLines[i][:len(LONG_HEADER)] != LONG_HEADER {
+ testResult += testLines[i]
+ }
+ } else {
+ testResult += testLines[i]
+ }
+ }
+ }
+ }
+
+ if refResult == testResult {
log.Printf("Identical results (%d bytes) from %s", len(res[0].Res), route)
} else {
log.Print("Diffs from ", route, " written to")
diff --git a/traffic_ops/testing/compare/genConfigRoutes.py b/traffic_ops/testing/compare/genConfigRoutes.py
index 0725c8892..d80beb28c 100755
--- a/traffic_ops/testing/compare/genConfigRoutes.py
+++ b/traffic_ops/testing/compare/genConfigRoutes.py
@@ -46,7 +46,7 @@
#: A format specifier for logging output. Propagates to all imported modules.
LOG_FMT = "%(levelname)s: %(asctime)s line %(lineno)d in %(module)s.%(funcName)s: %(message)s"
-__version__ = "1.0.0"
+__version__ = "2.0.0"
def getConfigRoutesForServers(servers:typing.List[dict], inst:TOSession) \
-> typing.Generator[str, None, None]:
@@ -80,13 +80,83 @@ def getCRConfigs(A:TOSession, B:TOSession) -> typing.Generator[str, None, None]:
if not cdns:
logging.error("The two instances have NO CDNs in common! This almost certainly means that "\
"you're not doing what you want to do")
- yield from []
- else:
- yield from ["CRConfig-Snapshots/%s/CRConfig.json" % cdn for cdn in cdns]
+ yield from ["CRConfig-Snapshots/%s/CRConfig.json" % cdn for cdn in cdns]
+
+
+def consolidateVariables(kwargs:argparse.Namespace) -> typing.Tuple[str, str,
+ typing.Tuple[str, str], typing.Tuple[str]]:
+ """
+ Consolidates the arguments passed on the command line with the ones in the environment
+
+ :param kwargs: The arguments passed on the command line
+ :returns: In order: the reference Traffic Ops URL, the testing Traffic Ops URL, the login
+ information for the reference instance, and the login information for the testing instance
+ :raises ValueError: if a required variable is not defined
+ """
+ instanceA = kwargs.refURL if kwargs.refURL else os.environ.get("TO_URL", None)
+ if instanceA is None:
+ logging.critical("Must specify the URL of the reference instance!")
+ raise ValueError()
+
+ instanceB = kwargs.testURL if kwargs.testURL else os.environ.get("TEST_URL", None)
+ if instanceB is None:
+ logging.critical("Must specify the URL of the testing instance!")
+ raise ValueError()
+
+ refUser = kwargs.refUser if kwargs.refUser else os.environ.get("TO_USER", None)
+ if refUser is None:
+ logging.critical("Must specify reference instance username!")
+ raise ValueError()
+
+ refPasswd = kwargs.refPasswd if kwargs.refPasswd else os.environ.get("TO_PASSWORD", None)
+ if refPasswd is None:
+ logging.critical("Must specify reference instance password!")
+ raise ValueError()
+
+ testUser = kwargs.testUser if kwargs.testUser else os.environ.get("TEST_USER", refUser)
+ testPasswd = kwargs.testPasswd if kwargs.testPasswd else os.environ.get("TEST_PASSWORD", refPasswd)
+
+ # Peel off all schemas
+ if instanceA.startswith("https://"):
+ instanceA = instanceA[8:]
+ elif instanceA.startswith("http://"):
+ instanceA = instanceA[7:]
+
+ if instanceB.startswith("https://"):
+ instanceB = instanceB[8:]
+ elif instanceB.startswith("http://"):
+ instanceB = instanceB[7:]
+
+ # Parse out port numbers, if specified
+ try:
+ if ':' in instanceA:
+ instanceA = instanceA.split(':')
+ if len(instanceA) != 2:
+ logging.critical("'%s' is not a valid Traffic Ops URL!", kwargs.InstanceA)
+ raise ValueError()
+ instanceA = {"host": instanceA[0], "port": int(instanceA[1])}
+ else:
+ instanceA = {"host": instanceA, "port": 443}
+ except TypeError as e:
+ logging.critical("'%s' is not a valid port number!", instanceA[1])
+ raise ValueError from e
+ try:
+ if ':' in instanceB:
+ instanceB = instanceB.split(':')
+ if len(instanceB) != 2:
+ logging.critical("'%s' is not a valid Traffic Ops URL!", kwargs.InstanceB)
+ raise ValueError()
+ instanceB = {"host": instanceB[0], "port": int(instanceB[1])}
+ else:
+ instanceB = {"host": instanceB, "port": 443}
+ except TypeError as e:
+ logging.critical("'%s' is not a valid port number!", instanceB[1])
+ raise ValueError from e
+ return (instanceA, instanceB, (refUser, refPasswd), (testUser, testPasswd))
-def genRoutes(A:TOSession, B:TOSession) -> typing.List[str]:
+def genRoutes(A:TOSession, B:TOSession) -> typing.Generator[str, None, None]:
"""
Generates routes to check for ATS config files from two valid Traffic Ops sessions
@@ -168,67 +238,14 @@ def main(kwargs:argparse.Namespace) -> int:
print("Unrecognized log level:", kwargs.log_level, file=sys.stderr)
return 1
- instanceA = kwargs.InstanceA
- instanceB = kwargs.InstanceB
-
try:
- loginA = kwargs.LoginA.split(':')
- loginA = (loginA[0], ':'.join(loginA[1:]))
- except (KeyError, IndexError) as e:
- logging.critical("Bad username/password pair: '%s' (hint: try -h/--help)", kwargs.LoginA)
- return 1
-
- loginB = loginA
-
- try:
- if kwargs.LoginB:
- loginB = kwargs.LoginB.split(':')
- loginB = (loginB[0], ':'.join(loginB[1:]))
- loginB = (loginB[0] if loginB[0] else loginA[0], loginB[1] if loginB[1] else loginA[1])
- except (KeyError, IndexError) as e:
- logging.critical("Bad username/password pair: '%s' (hint: try -h/--help)", kwargs.LoginB)
- return 1
-
- verify = not kwargs.insecure
-
- # Peel off all schemas
- if instanceA.startswith("https://"):
- instanceA = instanceA[8:]
- elif instanceA.startswith("http://"):
- instanceA = instanceA[7:]
-
- if instanceB.startswith("https://"):
- instanceB = instanceB[8:]
- elif instanceB.startswith("http://"):
- instanceB = instanceB[7:]
-
- # Parse out port numbers, if specified
- try:
- if ':' in instanceA:
- instanceA = instanceA.split(':')
- if len(instanceA) != 2:
- logging.critical("'%s' is not a valid Traffic Ops URL!", kwargs.InstanceA)
- return 1
- instanceA = {"host": instanceA[0], "port": int(instanceA[1])}
- else:
- instanceA = {"host": instanceA, "port": 443}
- except TypeError as e:
- logging.critical("'%s' is not a valid port number!", instanceA[1])
+ instanceA, instanceB, loginA, loginB = consolidateVariables(kwargs)
+ except ValueError as e:
logging.debug("%s", e, exc_info=True, stack_info=True)
+ logging.critical("(hint: try '-h'/'--help')")
return 1
- try:
- if ':' in instanceB:
- instanceB = instanceB.split(':')
- if len(instanceB) != 2:
- logging.critical("'%s' is not a valid Traffic Ops URL!", kwargs.InstanceB)
- instanceB = {"host": instanceB[0], "port": int(instanceB[1])}
- else:
- instanceB = {"host": instanceB, "port": 443}
- except TypeError as e:
- logging.critical("'%s' is not a valid port number!", instanceB[1])
- logging.debug("%s", e, exc_info=True, stack_info=True)
- return 1
+ verify = not kwargs.insecure
# Instantiate connections and login
with TOSession(host_ip=instanceA["host"], host_port=instanceA["port"], verify_cert=verify) as A,\
@@ -258,22 +275,26 @@ def main(kwargs:argparse.Namespace) -> int:
"instances. This, for the purpose of using the 'compare' tool",
formatter_class=argparse.ArgumentDefaultsHelpFormatter)
- parser.add_argument("InstanceA",
- help="The full URL of the first Traffic Ops instance",
+ parser.add_argument("--refURL",
+ help="The full URL of the reference Traffic Ops instance",
type=str)
- parser.add_argument("InstanceB",
- help="The full URL of the second Traffic Ops instance",
+ parser.add_argument("--testURL",
+ help="The full URL of the testing Traffic Ops instance",
type=str)
- parser.add_argument("LoginA",
- help="A login string containing credentials for logging into the first "\
- "Traffic Ops instance (InstanceA) in the format 'username:password'.",
+ parser.add_argument("--refUser",
+ help="A username for logging into the reference Traffic Ops instance.",
+ type=str)
+ parser.add_argument("--refPasswd",
+ help="A password for logging into the reference Traffic Ops instance",
+ type=str)
+ parser.add_argument("--testUser",
+ help="A username for logging into the testing Traffic Ops instance. If "\
+ "not given, the value for the reference instance will be used.",
+ type=str)
+ parser.add_argument("--testPasswd",
+ help="A password for logging into the testing Traffic Ops instance. If "\
+ "not given, the value for the reference instance will be used.",
type=str)
- parser.add_argument("LoginB",
- help="A login string containing credentials for logging into the second "\
- "Traffic Ops instance (InstanceB) in the format 'username:password'. "\
- "If not given, LoginA will be re-used for the second connection",
- type=str,
- nargs='?')
parser.add_argument("-k", "--insecure",
help="Do not verify SSL certificate signatures against *either* Traffic "\
"Ops instance",
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services