Repository: ceph/go-ceph
Pull request #937 (2023-10-26 18:54:27)

rgw/admin: add creation time to bucket
The API endpoint for bucket stats includes a `creation_time` field which is not included in the `admin.Bucket` struct. This simply adds the field.

## Checklist
- [x] Added tests for features and functional changes
- [x] Public functions and types are documented
- [x] Standard formatting is applied to Go code
- [x] Is this a new API? Added a new file that begins with `//go:build ceph_preview`
- [x] Ran `make api-update` to record new APIs

New or infrequent contributors may want to review the go-ceph [Developer's Guide](https://github.com/ceph/go-ceph/blob/master/docs/development.md) including the section on how we track [API Status](https://github.com/ceph/go-ceph/blob/master/docs/development.md#api-status) and the [API Stability Plan](https://github.com/ceph/go-ceph/blob/master/docs/api-stability.md).

The go-ceph project uses mergify. View the [mergify command guide](https://docs.mergify.com/commands/#commands) for information on how to interact with mergify. Add a comment with `@Mergifyio` `rebase` to rebase your PR when github indicates that the PR is out of date with the base branch.


github-actions[bot]: This Pull Request has been automatically marked as stale because it has not had recent activity. It will be closed in 21 days if no further activity occurs. Remember, a closed PR can always be reopened. Thank you for your contribution.
ansiwen: @mergifyio rebase

mergify[bot]: > rebase

#### ✅ Branch has been successfully rebased



<!---
DO NOT EDIT
-*- Mergify Payload -*-
{"command": "rebase", "conclusion": "success", "action_is_running": true}
-*- Mergify Payload End -*-
-->
peterwillis: I have added a test, but I am not sure it is the right place to add a test for this. It tests that the creation date was within a reasonable duration as I couldn't see a way to make the test server return a known date. It might be better to test the other end. I also made a test with a mock response similar to what is going on in `TestUnmarshal`, but that would add some burden in the future when the response is changed. Happy to add that instead or aswell?
github-actions[bot]: This Pull Request has been automatically marked as stale because it has not had recent activity. It will be closed in 21 days if no further activity occurs. Remember, a closed PR can always be reopened. Thank you for your contribution.
ansiwen: > I have added a test, but I am not sure it is the right place to add a test for this. It tests that the creation date was within a reasonable duration as I couldn't see a way to make the test server return a known date. It might be better to test the other end. I also made a test with a mock response similar to what is going on in `TestUnmarshal`, but that would add some burden in the future when the response is changed. Happy to add that instead or aswell?

@phlogistonjohn since you asked for the unit test, you want to take a final look. 
phlogistonjohn: Great, I am happy that there is a test. My lack of knowledge around specific of RGW make me hesitant to approve the PR though. It has my tacit approval now :-)
phlogistonjohn: @mergifyio rebase
mergify[bot]: > rebase

#### ✅ Branch has been successfully rebased



<!---
DO NOT EDIT
-*- Mergify Payload -*-
{"command": "rebase", "conclusion": "success", "action_is_running": true}
-*- Mergify Payload End -*-
-->
phlogistonjohn: The nautilus test is now failing in the same location:
```
{"bucket":"test","num_shards":0,"tenant":"","zonegroup":"8fb5e482-7816-4c4a-bf48-c7c0f29bec50","placement_rule":"default-placement","explicit_placement":{"data_pool":"","data_extra_pool":"","index_pool":""},"id":"b872f21e-d86c-4c80-9d87-d6550b207fe0.4151.1","marker":"b872f21e-d86c-4c80-9d87-d6550b207fe0.4151.1","index_type":"Normal","owner":"admin","ver":"0#1","master_ver":"0#0","mtime":"2024-05-20 13:57:38.916311Z","max_marker":"0#","usage":{},"bucket_quota":{"enabled":false,"check_on_raw":false,"max_size":-1,"max_size_kb":0,"max_objects":-1}}
=== NAME  TestRadosGWTestSuite/TestBucket
    bucket_test.go:46: 
        	Error Trace:	/go/src/github.com/ceph/go-ceph/rgw/admin/bucket_test.go:46
        	Error:      	Expected value not to be nil.
        	Test:       	TestRadosGWTestSuite/TestBucket
--- FAIL: TestRadosGWTestSuite (1.56s)
    --- FAIL: TestRadosGWTestSuite/TestBucket (1.56s)
        --- PASS: TestRadosGWTestSuite/TestBucket/list_buckets (0.05s)
        --- PASS: TestRadosGWTestSuite/TestBucket/info_non-existing_bucket (0.04s)
        --- PASS: TestRadosGWTestSuite/TestBucket/info_existing_bucket (0.04s)
        --- FAIL: TestRadosGWTestSuite/TestBucket/existing_bucket_has_valid_creation_date (0.04s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x9fee34]

goroutine 41 [running]:
testing.tRunner.func1.2({0xa87fc0, 0x13f2930})
	/opt/go/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
	/opt/go/src/testing/testing.go:1634 +0x377
panic({0xa87fc0?, 0x13f2930?})
	/opt/go/src/runtime/panic.go:770 +0x132
github.com/ceph/go-ceph/rgw/admin.(*RadosGWTestSuite).TestBucket.func4(0xc000550340?)
	/go/src/github.com/ceph/go-ceph/rgw/admin/bucket_test.go:47 +0x1d4
testing.tRunner(0xc000550340, 0xc00053af30)
	/opt/go/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 21
	/opt/go/src/testing/testing.go:1742 +0x390
FAIL	github.com/ceph/go-ceph/rgw/admin	1.569s
FAIL
```

I suspect this may be a real regression. Someone please look into this. Thanks!
ansiwen: @mergifyio rebase
mergify[bot]: > rebase

#### ✅ Branch has been successfully rebased



<!---
DO NOT EDIT
-*- Mergify Payload -*-
{"command": "rebase", "conclusion": "success", "action_is_running": true}
-*- Mergify Payload End -*-
-->

Patch:
diff --git a/rgw/admin/bucket.go b/rgw/admin/bucket.go
index de1cf99..d54e35e 100644
--- a/rgw/admin/bucket.go
+++ b/rgw/admin/bucket.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"time"
 )
 
 // Bucket describes an object store bucket
@@ -19,15 +20,16 @@ type Bucket struct {
 		DataExtraPool string `json:"data_extra_pool"`
 		IndexPool     string `json:"index_pool"`
 	} `json:"explicit_placement"`
-	ID        string `json:"id"`
-	Marker    string `json:"marker"`
-	IndexType string `json:"index_type"`
-	Owner     string `json:"owner"`
-	Ver       string `json:"ver"`
-	MasterVer string `json:"master_ver"`
-	Mtime     string `json:"mtime"`
-	MaxMarker string `json:"max_marker"`
-	Usage     struct {
+	ID           string     `json:"id"`
+	Marker       string     `json:"marker"`
+	IndexType    string     `json:"index_type"`
+	Owner        string     `json:"owner"`
+	Ver          string     `json:"ver"`
+	MasterVer    string     `json:"master_ver"`
+	Mtime        string     `json:"mtime"`
+	CreationTime *time.Time `json:"creation_time"`
+	MaxMarker    string     `json:"max_marker"`
+	Usage        struct {
 		RgwMain struct {
 			Size           *uint64 `json:"size"`
 			SizeActual     *uint64 `json:"size_actual"`


Test patch:
diff --git a/rgw/admin/bucket_test.go b/rgw/admin/bucket_test.go
index 46ee723..15b39ba 100644
--- a/rgw/admin/bucket_test.go
+++ b/rgw/admin/bucket_test.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"net/http"
 	"testing"
+	"time"
 
 	"github.com/ceph/go-ceph/internal/util"
 	"github.com/stretchr/testify/assert"
@@ -18,6 +19,7 @@ func (suite *RadosGWTestSuite) TestBucket() {
 	s3, err := newS3Agent(suite.accessKey, suite.secretKey, suite.endpoint, true)
 	assert.NoError(suite.T(), err)
 
+	beforeCreate := time.Now()
 	err = s3.createBucket(suite.bucketTestName)
 	assert.NoError(suite.T(), err)
 
@@ -38,6 +40,13 @@ func (suite *RadosGWTestSuite) TestBucket() {
 		assert.NoError(suite.T(), err)
 	})
 
+	suite.T().Run("existing bucket has valid creation date", func(_ *testing.T) {
+		b, err := co.GetBucketInfo(context.Background(), Bucket{Bucket: suite.bucketTestName})
+		assert.NoError(suite.T(), err)
+		assert.NotNil(suite.T(), b.CreationTime)
+		assert.WithinDuration(suite.T(), beforeCreate, *b.CreationTime, time.Minute)
+	})
+
 	suite.T().Run("get policy non-existing bucket", func(_ *testing.T) {
 		_, err := co.GetBucketPolicy(context.Background(), Bucket{Bucket: "foo"})
 		assert.Error(suite.T(), err)