Merge pull request ceph#65456 from yuvalif/wip-yuval-72940

rgw/logging: rollover objects when conf changes

Author: yuvalif

doc/radosgw/s3/bucketops.rst

@@ -651,7 +651,7 @@ Parameters
 Response Entities
 ~~~~~~~~~~~~~~~~~
 
-Response is XML encoded in the body of the request, in the following format:
+The response is XML encoded in the body of the request, in the following format:
 
 ::
 
@@ -790,7 +790,7 @@ Parameters are XML encoded in the body of the request, in the following format:
 |                               |           | between different source buckets writing log records to the same log bucket.         |          |
 +-------------------------------+-----------+--------------------------------------------------------------------------------------+----------+
 | ``LoggingType``               | String    | The type of logging. Valid values are:                                               | No       |
-|                               |           | ``Standard`` (default) all bucket operations are logged after being perfomed.        |          |
+|                               |           | ``Standard`` (default) all bucket operations are logged after being performed.        |          |
 |                               |           | The log record will contain all fields.                                              |          |
 |                               |           | ``Journal`` only operations that modify and object are logged.                       |          |
 |                               |           | Will record the minimum subset of fields in the log record that is needed            |          |
@@ -800,6 +800,18 @@ Parameters are XML encoded in the body of the request, in the following format:
 |                               |           | object added to the log bucket. Default is 3600 seconds (1 hour).                    |          |
 +-------------------------------+-----------+--------------------------------------------------------------------------------------+----------+
 
+Response Entities
+~~~~~~~~~~~~~~~~~
+
+The response is XML encoded in the body of the request, only if a configuration change triggers flushing of the current logging object.
+In this case it will return the name of the flushed logging object in following format:
+
+::
+
+  <PostBucketLoggingOutput xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
+    <FlushedLoggingObject>string</FlushedLoggingObject>
+  </PostBucketLoggingOutput>
+
 
 HTTP Response
 ~~~~~~~~~~~~~
@@ -864,7 +876,7 @@ Syntax
 Response Entities
 ~~~~~~~~~~~~~~~~~
 
-Response header contains ``Last-Modified`` date/time of the logging configuration.
+The response header contains ``Last-Modified`` date/time of the logging configuration.
 Logging configuration is XML encoded in the body of the response, in the following format:
 
 ::
@@ -931,7 +943,7 @@ Syntax
 Response Entities
 ~~~~~~~~~~~~~~~~~
 
-Response is XML encoded in the body of the request, in the following format:
+The response is XML encoded in the body of the request, in the following format:
 
 ::
 

examples/rgw/boto3/service-2.sdk-extras.json

@@ -24,6 +24,17 @@
             "output": {"shape": "PostBucketLoggingOutput"},
             "documentationUrl":"https://docs.ceph.com/docs/master/radosgw/s3/bucketops/#post-bucket-logging",
             "documentation":"<p>Flushes the logging objects of the buckets.</p>"
+        },
+   			"PutBucketLogging":{
+      		"name":"PutBucketLogging",
+      		"http":{
+        		"method":"PUT",
+        		"requestUri":"/{Bucket}?logging"
+      		},
+      		"input":{"shape":"PutBucketLoggingRequest"},
+          "output": {"shape": "PutBucketLoggingOutput"},
+          "documentationUrl":"https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketLogging.html",
+          "documentation":"<p>Put bucket logging configuration on source bucket.</p>"
         },
         "GetUsageStats":{
             "name":"GetUsageStats",
@@ -402,6 +413,15 @@
               }
             }
         },
+        "PutBucketLoggingOutput": {
+            "type":"structure",
+            "members":{
+              "FlushedLoggingObject": {
+                "shape":"FlushedLoggingObject",
+                "documentation":"<p>Name of the pending logging object that was flushed.</p>"
+              }
+            }
+        },
         "FlushedLoggingObject":{
             "type":"string"
         },

src/rgw/rgw_bucket_logging.cc

@@ -857,14 +857,15 @@ int bucket_deletion_cleanup(const DoutPrefixProvider* dpp,
     }
   }
 
-  return source_bucket_cleanup(dpp, driver, bucket, false, y);
+  return source_bucket_cleanup(dpp, driver, bucket, false, y, nullptr);
 }
 
 int source_bucket_cleanup(const DoutPrefixProvider* dpp,
                                    sal::Driver* driver,
                                    sal::Bucket* bucket,
                                    bool remove_attr,
-                                   optional_yield y) {
+                                   optional_yield y,
+                                   std::string* last_committed) {
   std::optional<configuration> conf;
   if (const int ret = retry_raced_bucket_write(dpp, bucket, [dpp, bucket, &conf, remove_attr, y] {
     auto& attrs = bucket->get_attrs();
@@ -899,7 +900,7 @@ int source_bucket_cleanup(const DoutPrefixProvider* dpp,
     return 0;
   }
   const auto& info = bucket->get_info();
-  if (const int ret = commit_logging_object(*conf, dpp, driver, info.bucket.tenant, y, nullptr); ret < 0) {
+  if (const int ret = commit_logging_object(*conf, dpp, driver, info.bucket.tenant, y, last_committed); ret < 0) {
     ldpp_dout(dpp, 5) << "WARNING: could not commit pending logging object of bucket '" <<
       bucket->get_key() << "' during cleanup. ret = " << ret << dendl;
   } else {

src/rgw/rgw_bucket_logging.h

@@ -253,11 +253,13 @@ int bucket_deletion_cleanup(const DoutPrefixProvider* dpp,
 // in addition:
 // any pending log objects should be comitted to the log bucket
 // and the log bucket should be updated to remove the bucket as a source
+// if "last_committed" is not null, it will be set to the name of the last committed object
 int source_bucket_cleanup(const DoutPrefixProvider* dpp,
                                    sal::Driver* driver,
                                    sal::Bucket* bucket,
                                    bool remove_attr,
-                                   optional_yield y);
+                                   optional_yield y,
+                                   std::string* last_committed);
 
 // verify that the target bucket has the correct policy to allow the source bucket to log to it
 // note that this function adds entries to the request state environment

src/rgw/rgw_rest_bucket_logging.cc

@@ -111,6 +111,7 @@ class RGWGetBucketLoggingOp : public RGWOp {
   }
 
   void send_response() override {
+    set_req_state_err(s, op_ret);
     dump_errno(s);
     if (mtime) {
       dump_last_modified(s, *mtime);
@@ -123,6 +124,7 @@ class RGWGetBucketLoggingOp : public RGWOp {
     s->formatter->close_section();
     rgw_flush_formatter_and_reset(s, s->formatter);
   }
+
   const char* name() const override { return "get_bucket_logging"; }
   std::string canonical_name() const override { return fmt::format("REST.{}.LOGGING", s->info.method); }
   RGWOpType get_type() override { return RGW_OP_GET_BUCKET_LOGGING; }
@@ -136,6 +138,7 @@ class RGWPutBucketLoggingOp : public RGWDefaultResponseOp {
   // and usd in execute()
   rgw::bucketlogging::configuration configuration;
   std::unique_ptr<rgw::sal::Bucket> target_bucket;
+  std::string old_obj; // used when conf change triggers a rollover
 
   int init_processing(optional_yield y) override {
     if (const auto ret = verify_bucket_logging_params(this, s); ret < 0) {
@@ -234,7 +237,7 @@ class RGWPutBucketLoggingOp : public RGWDefaultResponseOp {
     }
 
     if (!configuration.enabled) {
-      op_ret = rgw::bucketlogging::source_bucket_cleanup(this, driver, src_bucket.get(), true, y);
+      op_ret = rgw::bucketlogging::source_bucket_cleanup(this, driver, src_bucket.get(), true, y, &old_obj);
       return;
     }
 
@@ -306,12 +309,27 @@ class RGWPutBucketLoggingOp : public RGWDefaultResponseOp {
       }
     } else if (*old_conf != configuration) {
       // conf changed - do cleanup
-      if (const auto ret = commit_logging_object(*old_conf, target_bucket, this, y, nullptr); ret < 0) {
-        ldpp_dout(this, 1) << "WARNING: could not commit pending logging object when updating logging configuration of bucket '" <<
-          src_bucket->get_key() << "', ret = " << ret << dendl;
+      RGWObjVersionTracker objv_tracker;
+      std::string obj_name;
+      const auto region = driver->get_zone()->get_zonegroup().get_api_name();
+      if (const auto ret = rollover_logging_object(*old_conf,
+            target_bucket,
+            obj_name,
+            this,
+            region,
+            src_bucket,
+            y,
+            false, // rollover should happen even if commit failed
+            &objv_tracker,
+            &old_obj); ret < 0) {
+        ldpp_dout(this, 1) << "WARNING: failed to flush pending logging object '" << obj_name << "'"
+            << " to target bucket '" << target_bucket_id << "'. "
+            << " last committed object is '" << old_obj <<
+            "' when updating logging configuration of bucket '" << src_bucket->get_key() << ". error: " << ret << dendl;
       } else {
-        ldpp_dout(this, 20) << "INFO: committed pending logging object when updating logging configuration of bucket '" <<
-          src_bucket->get_key() << "'" << dendl;
+        ldpp_dout(this, 20) << "INFO: flushed pending logging object '" << old_obj
+          << "' to target bucket '" << target_bucket_id << "' when updating logging configuration of bucket '"
+          << src_bucket->get_key() << "'" << dendl;
       }
       if (old_conf->target_bucket != configuration.target_bucket) {
         rgw_bucket old_target_bucket_id;
@@ -334,6 +352,19 @@ class RGWPutBucketLoggingOp : public RGWDefaultResponseOp {
       ldpp_dout(this, 20) << "INFO: logging configuration of bucket '" << src_bucket_id << "' did not change" << dendl;
     }
   }
+
+  void send_response() override {
+    set_req_state_err(s, op_ret);
+    dump_errno(s);
+    end_header(s, this, to_mime_type(s->format));
+    if (!old_obj.empty()) {
+      dump_start(s);
+      s->formatter->open_object_section_in_ns("PutBucketLoggingOutput", XMLNS_AWS_S3);
+      s->formatter->dump_string("FlushedLoggingObject", old_obj);
+      s->formatter->close_section();
+      rgw_flush_formatter_and_reset(s, s->formatter);
+    }
+  }
 };
 
 // Post /<bucket name>/?logging
@@ -407,6 +438,7 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp {
   }
 
   void send_response() override {
+    set_req_state_err(s, op_ret);
     dump_errno(s);
     end_header(s, this, to_mime_type(s->format));
     dump_start(s);