s3ext: update gpcheckcloud behaviors (#54)

1. Add url checking messages for gpcheckcloud,

2. List all bucket content with '-c' option.

3. Add clear config checking messages for gpcheckcloud.
Signed-off-by: NHaozhou Wang <hawang@pivotal.io>
Signed-off-by: Adam Lee ali@pivotal.io

gpAux/extensions/gps3ext/bin/gpcheckcloud/gpcheckcloud.cpp

+#include <map>
+#include <sstream>
+#include <string>
+
 #include "gpcheckcloud.h"
 
-volatile bool QueryCancelPending = false;
+using std::map;
+using std::string;
+using std::stringstream;
 
-int main(int argc, char *argv[]) {
-    int opt = 0;
-    bool ret = true;
+volatile bool QueryCancelPending = false;
 
-    s3ext_loglevel = EXT_ERROR;
-    s3ext_logtype = STDERR_LOG;
+void printUsage(FILE *stream) {
+    fprintf(stream,
+            "Usage: gpcheckcloud -c \"s3://endpoint/bucket/prefix "
+            "config=path_to_config_file\", to check the configuration.\n"
+            "       gpcheckcloud -d \"s3://endpoint/bucket/prefix "
+            "config=path_to_config_file\", to download and output to stdout.\n"
+            "       gpcheckcloud -t, to show the config template.\n"
+            "       gpcheckcloud -h, to show this help.\n");
+}
 
-    if (argc == 1) {
-        print_usage(stderr);
-        exit(EXIT_FAILURE);
-    }
+// parse the arguments into char-string value pairs
+map<char, string> parseCommandLineArgs(int argc, char *argv[]) {
+    int opt = 0;
+    map<char, string> optionPairs;
 
     while ((opt = getopt(argc, argv, "c:d:ht")) != -1) {
         switch (opt) {
             case 'c':
-                ret = check_config(optarg);
-                break;
             case 'd':
-                ret = s3_download(optarg);
-                break;
             case 'h':
-                print_usage(stdout);
-                break;
             case 't':
-                print_template();
+                if (optarg == NULL) {
+                    optionPairs[opt] = "";
+                } else if (optarg[0] == '-') {
+                    fprintf(stderr, "Failed. Invalid argument for -%c: '%s'\n\n", opt, optarg);
+                    printUsage(stderr);
+                    exit(EXIT_FAILURE);
+                } else {
+                    optionPairs[opt] = optarg;
+                }
+
                 break;
-            default:
-                print_usage(stderr);
+            default:  // '?'
+                printUsage(stderr);
                 exit(EXIT_FAILURE);
         }
-
-        break;
     }
 
-    if (ret) {
-        exit(EXIT_SUCCESS);
-    } else {
-        fprintf(stderr, "Failed. Please check the arguments and configuration file.\n\n");
-        print_usage(stderr);
+    return optionPairs;
+}
+
+// check if command line arguments are valid
+void validateCommadLineArgs(map<char, string> &optionPairs) {
+    // only one option is allowed(for now)
+    if (optionPairs.size() > 1) {
+        stringstream ss;
+
+        ss << "Failed. Can't set options ";
+
+        // concatenate all option names
+        // e.g. if we have -c and -d, insert "-c, -d" into the stream.
+        for (map<char, string>::iterator i = optionPairs.begin(); i != optionPairs.end(); i++) {
+            ss << "'-" << i->first << "' ";
+        }
+
+        ss << "at the same time.";
+
+        // example message: "Failed. Can't set options '-c' '-d' at the same time."
+        fprintf(stderr, "%s\n\n", ss.str().c_str());
+        printUsage(stderr);
         exit(EXIT_FAILURE);
     }
 }
 
-void print_template() {
+void printTemplate() {
     printf(
         "[default]\n"
         "secret = \"aws secret\"\n"
@@ -57,27 +86,12 @@ void print_template() {
         "encryption = true\n");
 }
 
-void print_usage(FILE *stream) {
-    fprintf(stream,
-            "Usage: gpcheckcloud -c \"s3://endpoint/bucket/prefix "
-            "config=path_to_config_file\", to check the configuration.\n"
-            "       gpcheckcloud -d \"s3://endpoint/bucket/prefix "
-            "config=path_to_config_file\", to download and output to stdout.\n"
-            "       gpcheckcloud -t, to show the config template.\n"
-            "       gpcheckcloud -h, to show this help.\n");
-}
-
-uint64_t print_contents(ListBucketResult *r) {
+uint64_t printBucketContents(ListBucketResult *result) {
     char urlbuf[256];
     uint64_t count = 0;
     vector<BucketContent *>::iterator i;
 
-    for (i = r->contents.begin(); i != r->contents.end(); i++) {
-        if ((s3ext_loglevel <= EXT_WARNING) && count > 8) {
-            printf("... ...\n");
-            break;
-        }
-
+    for (i = result->contents.begin(); i != result->contents.end(); i++) {
         BucketContent *p = *i;
         snprintf(urlbuf, 256, "%s", p->getName().c_str());
         printf("File: %s, Size: %" PRIu64 "\n", urlbuf, p->getSize());
@@ -88,24 +102,24 @@ uint64_t print_contents(ListBucketResult *r) {
     return count;
 }
 
-bool check_config(const char *url_with_options) {
-    if (!url_with_options) {
+bool checkConfig(const char *urlWithOptions) {
+    if (!urlWithOptions) {
         return false;
     }
 
-    GPReader *reader = reader_init(url_with_options);
+    GPReader *reader = reader_init(urlWithOptions);
     if (!reader) {
         return false;
     }
 
     ListBucketResult *result = reader->getKeyList();
     if (result != NULL) {
-        if (print_contents(result)) {
-            printf("\nYea! Your configuration works well.\n");
+        if (printBucketContents(result)) {
+            fprintf(stderr, "\nYour configuration works well.\n");
         } else {
-            printf(
-                "\nYour configuration works well, however there is no file "
-                "matching your prefix.\n");
+            fprintf(stderr,
+                    "\nYour configuration works well, however there is no file matching your "
+                    "prefix.\n");
         }
     }
 
@@ -114,8 +128,8 @@ bool check_config(const char *url_with_options) {
     return true;
 }
 
-bool s3_download(const char *url_with_options) {
-    if (!url_with_options) {
+bool downloadS3(const char *urlWithOptions) {
+    if (!urlWithOptions) {
         return false;
     }
 
@@ -125,7 +139,7 @@ bool s3_download(const char *url_with_options) {
 
     thread_setup();
 
-    GPReader *reader = reader_init(url_with_options);
+    GPReader *reader = reader_init(urlWithOptions);
     if (!reader) {
         return false;
     }
@@ -134,7 +148,7 @@ bool s3_download(const char *url_with_options) {
         data_len = BUF_SIZE;
 
         if (!reader_transfer_data(reader, data_buf, data_len)) {
-            fprintf(stderr, "Failed to read data\n");
+            fprintf(stderr, "Failed to read data from Amazon S3\n");
             ret = false;
             break;
         }
@@ -148,3 +162,49 @@ bool s3_download(const char *url_with_options) {
 
     return ret;
 }
+
+int main(int argc, char *argv[]) {
+    bool ret = true;
+
+    s3ext_loglevel = EXT_ERROR;
+    s3ext_logtype = STDERR_LOG;
+
+    if (argc == 1) {
+        printUsage(stderr);
+        exit(EXIT_FAILURE);
+    }
+
+    map<char, string> optionPairs = parseCommandLineArgs(argc, argv);
+
+    validateCommadLineArgs(optionPairs);
+
+    if (!optionPairs.empty()) {
+        const char *arg = optionPairs.begin()->second.c_str();
+
+        switch (optionPairs.begin()->first) {
+            case 'c':
+                ret = checkConfig(arg);
+                break;
+            case 'd':
+                ret = downloadS3(arg);
+                break;
+            case 'h':
+                printUsage(stdout);
+                break;
+            case 't':
+                printTemplate();
+                break;
+            default:
+                printUsage(stderr);
+                exit(EXIT_FAILURE);
+        }
+    }
+
+    if (ret) {
+        exit(EXIT_SUCCESS);
+    } else {
+        fprintf(stderr, "Failed. Please check the arguments and configuration file.\n\n");
+        printUsage(stderr);
+        exit(EXIT_FAILURE);
+    }
+}

gpAux/extensions/gps3ext/include/gpcheckcloud.h

@@ -17,14 +17,4 @@
 
 extern volatile bool QueryCancelPending;
 
-void print_template();
-
-void print_usage(FILE *stream);
-
-uint64_t print_contents(ListBucketResult *r);
-
-bool check_config(const char *url_with_options);
-
-bool s3_download(const char *url_with_options);
-
 #endif

gpAux/extensions/gps3ext/src/gpreader.cpp

@@ -100,6 +100,20 @@ void GPReader::close() {
     this->bucketReader.close();
 }
 
+void CheckEssentialConfig() {
+    if (s3ext_accessid.empty()) {
+        CHECK_OR_DIE_MSG(false, "%s", "\"FATAL: access id not set\"");
+    }
+
+    if (s3ext_secret.empty()) {
+        CHECK_OR_DIE_MSG(false, "%s", "\"FATAL: secret id not set\"");
+    }
+
+    if ((s3ext_segnum == -1) || (s3ext_segid == -1)) {
+        CHECK_OR_DIE_MSG(false, "%s", "\"FATAL: segment id is invalid\"");
+    }
+}
+
 // invoked by s3_import(), need to be exception safe
 GPReader* reader_init(const char* url_with_options) {
     GPReader* reader = NULL;
@@ -118,6 +132,7 @@ GPReader* reader_init(const char* url_with_options) {
 
         string config_path = get_opt_s3(urlWithOptions, "config");
         if (config_path.empty()) {
+            S3ERROR("The 'config' parameter is not provided, use default value 's3/s3.conf'.");
             config_path = "s3/s3.conf";
         }
 
@@ -125,6 +140,8 @@ GPReader* reader_init(const char* url_with_options) {
             return NULL;
         }
 
+        CheckEssentialConfig();
+
         InitRemoteLog();
 
         reader = new GPReader(url);
@@ -140,7 +157,7 @@ GPReader* reader_init(const char* url_with_options) {
         if (reader != NULL) {
             delete reader;
         }
-        S3ERROR("reader_init caught an exception: %s, aborting", e.what());
+        S3ERROR("reader_init caught an exception: %s", e.what());
         gpReaderErrorMessage = e.what();
         return NULL;
     }
@@ -162,7 +179,7 @@ bool reader_transfer_data(GPReader* reader, char* data_buf, int& data_len) {
         // sure read_len <= data_len here, hence truncation will never happen
         data_len = (int)read_len;
     } catch (std::exception& e) {
-        S3ERROR("reader_transfer_data caught an exception: %s, aborting", e.what());
+        S3ERROR("reader_transfer_data caught an exception: %s", e.what());
         gpReaderErrorMessage = e.what();
         return false;
     }
@@ -182,7 +199,7 @@ bool reader_cleanup(GPReader** reader) {
             result = false;
         }
     } catch (std::exception& e) {
-        S3ERROR("reader_cleanup caught an exception: %s, aborting", e.what());
+        S3ERROR("reader_cleanup caught an exception: %s", e.what());
         gpReaderErrorMessage = e.what();
         result = false;
     }

gpAux/extensions/gps3ext/src/gps3ext.cpp

@@ -33,20 +33,6 @@ Datum s3_export(PG_FUNCTION_ARGS);
 Datum s3_import(PG_FUNCTION_ARGS);
 }
 
-void check_essential_config() {
-    if (s3ext_accessid == "") {
-        ereport(ERROR, (0, errmsg("ERROR: access id is empty")));
-    }
-
-    if (s3ext_secret == "") {
-        ereport(ERROR, (0, errmsg("ERROR: secret is empty")));
-    }
-
-    if ((s3ext_segnum == -1) || (s3ext_segid == -1)) {
-        ereport(ERROR, (0, errmsg("ERROR: segment id is invalid")));
-    }
-}
-
 /*
  * Import data into GPDB.
  * invoked by GPDB, be careful with C++ exceptions.
@@ -87,8 +73,6 @@ Datum s3_import(PG_FUNCTION_ARGS) {
                                       s3ext_segid, s3ext_segnum, gpReaderErrorMessage.c_str())));
         }
 
-        check_essential_config();
-
         EXTPROTOCOL_SET_USER_CTX(fcinfo, gpreader);
     }