Merge pull request #8264 from cbodley/wip-rgw-admin-errors

radosgw-admin: 'period commit' supplies user-readable error messages

Reviewed-by: Yehuda Sadeh <yehuda@redhat.com>

src/rgw/rgw_admin.cc

@@ -1319,15 +1319,13 @@ static int send_to_remote_gateway(const string& remote, req_info& info,
   }
   rgw_user user;
   int ret = conn->forward(user, info, NULL, MAX_REST_RESPONSE, &in_data, &response);
-  if (ret < 0) {
-    return ret;
-  }
-  ret = parser.parse(response.c_str(), response.length());
-  if (ret < 0) {
+
+  int parse_ret = parser.parse(response.c_str(), response.length());
+  if (parse_ret < 0) {
     cerr << "failed to parse response" << std::endl;
-    return ret;
+    return parse_ret;
   }
-  return 0;
+  return ret;
 }
 
 static int send_to_url(const string& url, RGWAccessKey& key, req_info& info,
@@ -1338,15 +1336,13 @@ static int send_to_url(const string& url, RGWAccessKey& key, req_info& info,
 
   bufferlist response;
   int ret = req.forward_request(key, info, MAX_REST_RESPONSE, &in_data, &response);
-  if (ret < 0) {
-    return ret;
-  }
-  ret = parser.parse(response.c_str(), response.length());
-  if (ret < 0) {
+
+  int parse_ret = parser.parse(response.c_str(), response.length());
+  if (parse_ret < 0) {
     cout << "failed to parse response" << std::endl;
-    return ret;
+    return parse_ret;
   }
-  return 0;
+  return ret;
 }
 
 static int send_to_remote_or_url(const string& remote, const string& url,
@@ -1388,7 +1384,7 @@ static int commit_period(RGWRealm& realm, RGWPeriod& period,
       return ret;
     }
     // the master zone can commit locally
-    ret = period.commit(realm, current_period);
+    ret = period.commit(realm, current_period, cerr);
     if (ret < 0) {
       cerr << "failed to commit period: " << cpp_strerror(-ret) << std::endl;
     }
@@ -1413,6 +1409,12 @@ static int commit_period(RGWRealm& realm, RGWPeriod& period,
   int ret = send_to_remote_or_url(remote, url, access, secret, info, bl, p);
   if (ret < 0) {
     cerr << "request failed: " << cpp_strerror(-ret) << std::endl;
+
+    // did we parse an error message?
+    auto message = p.find_obj("Message");
+    if (message) {
+      cerr << "Reason: " << message->get_data() << std::endl;
+    }
     return ret;
   }
 
@@ -2747,6 +2749,11 @@ int main(int argc, char **argv)
                                         info, bl, p);
         if (ret < 0) {
           cerr << "request failed: " << cpp_strerror(-ret) << std::endl;
+          if (ret == -EACCES) {
+            cerr << "If the realm has been changed on the master zone, the "
+                "master zone's gateway may need to be restarted to recognize "
+                "this user." << std::endl;
+          }
           return ret;
         }
         RGWRealm realm;

src/rgw/rgw_rados.cc

@@ -1273,27 +1273,32 @@ int RGWPeriod::update_sync_status()
   return 0;
 }
 
-int RGWPeriod::commit(RGWRealm& realm, const RGWPeriod& current_period)
+int RGWPeriod::commit(RGWRealm& realm, const RGWPeriod& current_period,
+                      std::ostream& error_stream)
 {
   ldout(cct, 20) << __func__ << " realm " << realm.get_id() << " period " << current_period.get_id() << dendl;
   // gateway must be in the master zone to commit
   if (master_zone != store->get_zone_params().get_id()) {
-    ldout(cct, 0) << "period commit on zone " << store->get_zone_params().get_id()
-        << ", not period's master zone " << master_zone << dendl;
+    error_stream << "Cannot commit period on zone "
+        << store->get_zone_params().get_id() << ", it must be sent to "
+        "the period's master zone " << master_zone << '.' << std::endl;
     return -EINVAL;
   }
   // period predecessor must match current period
   if (predecessor_uuid != current_period.get_id()) {
-    ldout(cct, 0) << "period predecessor " << predecessor_uuid
+    error_stream << "Period predecessor " << predecessor_uuid
         << " does not match current period " << current_period.get_id()
-        << dendl;
+        << ". Use 'period pull' to get the latest period from the master, "
+        "reapply your changes, and try again." << std::endl;
     return -EINVAL;
   }
   // realm epoch must be 1 greater than current period
   if (realm_epoch != current_period.get_realm_epoch() + 1) {
-    ldout(cct, 0) << "period's realm epoch " << realm_epoch
+    error_stream << "Period's realm epoch " << realm_epoch
         << " does not come directly after current realm epoch "
-        << current_period.get_realm_epoch() << dendl;
+        << current_period.get_realm_epoch() << ". Use 'realm pull' to get the "
+        "latest realm and period from the master zone, reapply your changes, "
+        "and try again." << std::endl;
     return -EINVAL;
   }
   // did the master zone change?
@@ -1325,8 +1330,10 @@ int RGWPeriod::commit(RGWRealm& realm, const RGWPeriod& current_period)
   }
   // period must be based on current epoch
   if (epoch != current_period.get_epoch()) {
-    ldout(cct, 0) << "period epoch " << epoch << " does not match "
-        "predecessor epoch " << current_period.get_epoch() << dendl;
+    error_stream << "Period epoch " << epoch << " does not match "
+        "predecessor epoch " << current_period.get_epoch()
+        << ". Use 'period pull' to get the latest epoch from the master zone, "
+        "reapply your changes, and try again." << std::endl;
     return -EINVAL;
   }
   // set period as next epoch

src/rgw/rgw_rados.h

@@ -1469,7 +1469,8 @@ public:
   int update();
 
   // commit a staging period; only for use on master zone
-  int commit(RGWRealm& realm, const RGWPeriod &current_period);
+  int commit(RGWRealm& realm, const RGWPeriod &current_period,
+             std::ostream& error_stream);
 
   void encode(bufferlist& bl) const {
     ENCODE_START(1, 1, bl);

src/rgw/rgw_rest_realm.cc

@@ -17,6 +17,7 @@ static const uint32_t PERIOD_HISTORY_FETCH_MAX = 64;
 class RGWOp_Period_Base : public RGWRESTOp {
  protected:
   RGWPeriod period;
+  std::ostringstream error_stream;
  public:
   int verify_permission() override { return 0; }
   void send_response() override;
@@ -25,12 +26,19 @@ class RGWOp_Period_Base : public RGWRESTOp {
 // reply with the period object on success
 void RGWOp_Period_Base::send_response()
 {
+  s->err.message = error_stream.str();
+
   set_req_state_err(s, http_ret);
   dump_errno(s);
   end_header(s);
 
-  if (http_ret < 0)
+  if (http_ret < 0) {
+    if (!s->err.message.empty()) {
+      ldout(s->cct, 4) << "Request failed with " << http_ret
+          << ": " << s->err.message << dendl;
+    }
     return;
+  }
 
   encode_json("period", period, s->formatter);
   flusher.flush();
@@ -85,8 +93,8 @@ void RGWOp_Period_Post::execute()
 
   // require period.realm_id to match our realm
   if (period.get_realm() != store->realm.get_id()) {
-    lderr(cct) << "period with realm id " << period.get_realm()
-        << " doesn't match current realm " << store->realm.get_id() << dendl;
+    error_stream << "period with realm id " << period.get_realm()
+        << " doesn't match current realm " << store->realm.get_id() << std::endl;
     http_ret = -EINVAL;
     return;
   }
@@ -112,7 +120,7 @@ void RGWOp_Period_Post::execute()
 
   // if period id is empty, handle as 'period commit'
   if (period.get_id().empty()) {
-    http_ret = period.commit(realm, current_period);
+    http_ret = period.commit(realm, current_period, error_stream);
     if (http_ret < 0) {
       lderr(cct) << "master zone failed to commit period" << dendl;
     }