From 5999b0bccc014e743a67fdbebbbdf2482d79efe1 Mon Sep 17 00:00:00 2001 From: Trevor Bonas <45324987+trevorbonas@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:27:26 -0700 Subject: [PATCH] Clarify host formatting for Influx migration script (#11) Changes: - The help messages for `--src-host` and `--dest-host` have been updated to specify the expected url formatting. - The error log when a health check fails has been changed to indicate that it is an API call failure. - A confusing comment in a code block in the README has been fixed. - A typo in the README has been fixed. --- tools/python/influx-migration/README.md | 8 ++--- .../influx-migration/influx_migration.py | 31 +++++++++++++------ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/tools/python/influx-migration/README.md b/tools/python/influx-migration/README.md index 0350ea62..8e411fdb 100644 --- a/tools/python/influx-migration/README.md +++ b/tools/python/influx-migration/README.md @@ -114,7 +114,7 @@ The following is a formatted version of the output: **`--dest-bucket DEST_BUCKET`**: Optional. The name of the InfluxDB bucket in the destination server, must not be an already existing bucket. Defaults to value of `--src-bucket` or `None` if `--src-bucket` not provided. -**`--dest-host DEST_HOST`**: The host for the destination server. Example: http://localhost:8086. +**`--dest-host DEST_HOST`**: The host for the destination server. Must have a scheme, domain or IP address, and port, e.g., `http://127.0.0.1:8086` or `https://:`. **`--dest-org DEST_ORG`**: Optional. The name of the organization to restore buckets to in the destination server. If this is omitted, then all migrated buckets from the source server will retain their original organization and migrated buckets may not be visible in the destination server without creating and switching organizations. This value will be used in all forms of restoration whether a single bucket, a full migration, or any migration using csv files for backup and restoration. @@ -134,7 +134,7 @@ The following is a formatted version of the output: **`--src-bucket SRC_BUCKET`**: Optional. The name of the InfluxDB bucket in the source server. If not provided, then `--full` must be provided. -**`--src-host SRC_HOST`**: Optional. The host for the source server. Defaults to http://localhost:8086. +**`--src-host SRC_HOST`**: Optional. The host for the source server. Must have a scheme, domain or IP address, and port, e.g., `http://127.0.0.1:8086` or `https://:`. Defaults to http://localhost:8086 if no value is specified. > As mentioned previously, `mountpoint-s3` and `rclone` are needed if `--s3-bucket` is to be used, but can be ignored if the user doesn't provide a value for `--s3-bucket`, in which case backup files will be stored in a unique directory locally. @@ -152,7 +152,7 @@ After meeting the prerequisites: b. Listing buckets with `influx bucket list -t --host --skip-verify`. - c. Using `influx v1 shell -t --host --skip-verify` and running `SELECT * FROM .. LIMIT 100` to view contents of a bucket or `SELECT COUNT(*) FROM ..` to verify the correct number of records have been migrated. + c. Using `influx v1 shell -t --host --skip-verify` and running `SELECT * FROM .. LIMIT 100` to view contents of a bucket or `SELECT COUNT(*) FROM ..` to verify the correct number of records have been migrated. d. By running a query using `influx query -t --host --skip-verify 'from(bucket: "") |> range(start: , stop: )'`. Adding `|> count()` to the query is also a way to verify the correct number of records have been migrated. @@ -178,7 +178,7 @@ After meeting the prerequisites: - (optional) S3 bucket name and credentials, AWS CLI credentials should be set in the OS environment variables. ``` - # AWS credentials (for timestream testing) + # AWS credentials export AWS_ACCESS_KEY_ID="xxx" export AWS_SECRET_ACCESS_KEY="xxx" ``` diff --git a/tools/python/influx-migration/influx_migration.py b/tools/python/influx-migration/influx_migration.py index 675ccf31..0bdd2c48 100644 --- a/tools/python/influx-migration/influx_migration.py +++ b/tools/python/influx-migration/influx_migration.py @@ -265,7 +265,7 @@ def create_backup_directory(backup_directory, mount_point=None, bucket_name=None raise RuntimeError("Backup directory could not be created within S3 bucket") return backup_directory -def health_check(host, token, skip_verify): +def health_check(host, token, skip_verify, debug=False): """ Pings the InfluxDB instance to determine health. @@ -278,9 +278,17 @@ def health_check(host, token, skip_verify): # Ensure hosts can be connected to try: client = InfluxDBClient(url=host, - token=token, timeout=MILLISECOND_TIMEOUT, verify_ssl=not skip_verify) + token=token, timeout=MILLISECOND_TIMEOUT, verify_ssl=not skip_verify, debug=debug) if not client.ping(): - raise InfluxDBError(message=f"{host} ping failed") + url_parse = urllib.parse.urlparse(host) + if url_parse.scheme == "": + logging.error("Host is missing scheme") + if url_parse.port is None: + logging.error("Host is missing port") + if url_parse.scheme == "" or url_parse.port is None: + logging.error("The expected format for host urls is ::. " + "For example, http://127.0.0.1:8086") + raise InfluxDBError(message=f"InfluxDB API call to {host}/ping failed") except InfluxDBError as error: logging.error(str(error)) return False @@ -373,14 +381,16 @@ def parse_args(args): "source server. If not provided, then --full must be provided.", required=False) parser.add_argument("--dest-bucket", help="Optional. The name of the InfluxDB bucket in the " - "destination server, must not be an already existing bucket. Defaults to value of" + "destination server, must not be an already existing bucket. Defaults to value of " "--src-bucket or None if --src-bucket not provided.", required=False) - parser.add_argument("--src-host", help="Optional. The host for the source server. Defaults to " - "http://localhost:8086.", + parser.add_argument("--src-host", help="Optional. The host for the source server. " + "Must have a scheme, domain or IP address, and port, e.g., http://127.0.0.1:8086 or " + "https://:. Defaults to http://localhost:8086 if no value is specified.", default="http://localhost:8086", required=False) - parser.add_argument("--dest-host", help="The host for the destination server. Example: " - "http://localhost:8086.", + parser.add_argument("--dest-host", help="The host for the destination server. " + "Must have a scheme, domain or IP address, and port, e.g., http://127.0.0.1:8086 or " + "https://.", required=True) parser.add_argument("--full", help="Optional. Whether to perform a full restore, replacing all " "data on destination server with all data from source server from all organizations, " @@ -765,9 +775,10 @@ def verify_instances(args, src_token, dest_token): :returns: None :raises InfluxDBError: If any check fails. """ + debug = args.log_level.lower() == "debug" # Source checks if src_token is not None: - if not health_check(args.src_host, src_token, args.skip_verify): + if not health_check(args.src_host, src_token, args.skip_verify, debug): raise InfluxDBError(message="Health check for source host failed") if not verify_token(args.src_host, src_token, args.skip_verify): raise InfluxDBError(message="Could not verify source token") @@ -780,7 +791,7 @@ def verify_instances(args, src_token, dest_token): raise InfluxDBError(message="TLS certificate could not be verified for source host") # Destination checks - if not health_check(args.dest_host, dest_token, args.skip_verify): + if not health_check(args.dest_host, dest_token, args.skip_verify, debug): raise InfluxDBError(message="Health check for destination host failed") if not args.skip_verify and not verify_tls(args.dest_host): raise InfluxDBError(message="TLS certificate could not be verified for destination host")