[patch] Refactor agent-sidecar: fix S3 reader & add backoff logic #467

rinx · 2020-06-11T00:59:05Z

Description:

use io.Pipe for reading S3 bucket data
add backoff logic for restorer

Related Issue:

nothing.

How Has This Been Tested?:

nothing

Environment:

Go Version: 1.14.3
Docker Version: 19.03.8
Kubernetes Version: 1.18.2
NGT Version: 1.11.5

Types of changes:

Bug fix [type/bug]
New feature [type/feature]
Add tests [type/test]
Security related changes [type/security]
Add documents [type/documentation]
Refactoring [type/refactoring]
Update dependencies [type/dependency]
Update benchmarks and performances [type/bench]
Update CI [type/ci]

Changes to Core Features:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your core changes, as applicable?
Have you successfully ran tests with your changes locally?

Checklist:

I have read the CONTRIBUTING document.
I have checked open Pull Requests for the similar feature or fixes?
I have added tests and benchmarks to cover my changes.
I have ensured all new and existing tests passed.
I have commented my code, particularly in hard-to-understand areas
I have updated the documentation accordingly.

pull-assistant · 2020-06-11T00:59:08Z

Best reviewed: commit by commit

Optimal code review plan

♻️ use io.pipe for reading s3 buckets

✅ fix tests

💚 Fix CI trigger for agent-sidecar

✨ Add backoff option for agent sidecar initcontainer mode

✅ fix tests

♻️ refactor the order of declaration.

✨ Add internal/net/http/client

✅ fix tests

🔧 Revise default duration

🔧 add sigkill for errors on initcontainer

🤖 Update license headers / Format go codes and yaml files

Powered by Pull Assistant. Last update ac623f0 ... 5278819. Read the comment docs.

codecov · 2020-06-11T01:06:55Z

Codecov Report

Merging #467 into master will decrease coverage by 0.09%.
The diff coverage is 1.71%.

@@            Coverage Diff            @@
##           master    #467      +/-   ##
=========================================
- Coverage    7.92%   7.83%   -0.10%     
=========================================
  Files         392     396       +4     
  Lines       20039   20323     +284     
=========================================
+ Hits         1589    1593       +4     
- Misses      18217   18493     +276     
- Partials      233     237       +4

Impacted Files	Coverage Δ
internal/config/sidecar.go	`0.00% <0.00%> (ø)`
internal/config/transport.go	`0.00% <0.00%> (ø)`
internal/db/storage/blob/s3/reader/reader.go	`0.00% <0.00%> (ø)`
internal/db/storage/blob/s3/s3.go	`0.00% <0.00%> (ø)`
internal/db/storage/blob/s3/session/option.go	`21.15% <0.00%> (-1.77%)`	⬇️
internal/db/storage/blob/s3/session/session.go	`0.00% <0.00%> (ø)`
internal/errors/blob.go	`0.00% <0.00%> (ø)`
internal/net/http/client/client.go	`0.00% <0.00%> (ø)`
pkg/agent/sidecar/service/restorer/option.go	`4.54% <0.00%> (-1.71%)`	⬇️
pkg/agent/sidecar/service/restorer/restorer.go	`0.00% <0.00%> (ø)`
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a608c7...5278819. Read the comment docs.

vdaas-ci · 2020-06-11T03:05:43Z

[WARNING] Changes in interal/config may require you to change Helm charts. Please check.

vdaas-ci · 2020-06-11T03:36:30Z

[WARNING] Changes in interal/config may require you to change Helm charts. Please check.

internal/db/storage/blob/s3/reader/reader.go

vdaas-ci · 2020-06-11T04:54:36Z

[WARNING] Changes in interal/config may require you to change Helm charts. Please check.

internal/db/storage/blob/s3/reader/option_test.go

internal/db/storage/blob/s3/reader/reader.go

internal/errors/blob.go

internal/errors/blob_test.go

vdaas-ci · 2020-06-12T03:07:01Z

[WARNING] Changes in interal/config may require you to change Helm charts. Please check.

github-actions · 2020-06-12T03:12:14Z

internal/net/http/client/option.go

+		return nil
+	}
+
+}


[golangci] _{reported by reviewdog 🐶}
block should not end with a whitespace (or comment) (wsl)

internal/net/http/client/option_test.go

internal/net/http/client/option.go

internal/errors/blob_test.go

vdaas-ci · 2020-06-12T03:24:14Z

[WARNING] Changes in interal/config may require you to change Helm charts. Please check.

github-actions · 2020-06-12T03:29:13Z

internal/db/storage/blob/s3/session/option_test.go

+	// Change interface type to the type of object you are testing
+	type T = interface{}
+	type args struct {
+		client *http.Client


[golangci] _{reported by reviewdog 🐶}
client is unused (structcheck)

vdaas-ci · 2020-06-12T03:30:41Z

[WARNING] Changes in interal/config may require you to change Helm charts. Please check.

kpango

LGTM

kpango · 2020-06-12T09:51:33Z

/rebase
/format
/changelog
/approve

vdaas-ci · 2020-06-12T09:51:58Z

[CHANGELOG] Please edit the following lines.

Refactor agent-sidecar: fix S3 reader & add backoff logic (#467)
🐛 📝 fix link (#471)
Fix /changelog command format (#470)
fix: failing test (#469)
fix: failing test (#468)
✨ Add options for AWS client (#460)
Fix /format command (#466)
Fix /format command (#465)
Fix internal/log/retry pacakge (#458)
🎨 trim white margin at data flow images (#459)

vdaas-ci · 2020-06-12T09:52:09Z

[REBASE] Rebase triggered by kpango for branch: refactor/agent-sidecar/use-io-pipe-for-reading-s3-backet

Signed-off-by: Rintaro Okamura <[email protected]>

vdaas-ci · 2020-06-12T09:52:40Z

[WARNING] Changes in interal/config may require you to change Helm charts. Please check.

vdaas-ci · 2020-06-12T09:53:55Z

[FORMAT] Updating license headers and formatting go codes triggered by kpango.

Signed-off-by: vdaas-ci <[email protected]>

vdaas-ci · 2020-06-12T09:55:49Z

[WARNING] Changes in interal/config may require you to change Helm charts. Please check.

vdaas-ci

[APPROVED] This PR is approved by kpango.

github-actions · 2020-06-12T09:58:31Z

internal/db/storage/blob/s3/session/option_test.go

+	// Change interface type to the type of object you are testing
+	type T = interface{}
+	type args struct {
+		client *http.Client


[golangci] _{reported by reviewdog 🐶}
field client is unused (unused)

github-actions · 2020-06-12T09:58:31Z

pkg/agent/sidecar/service/restorer/option.go

@@ -59,3 +60,15 @@ func WithBlobStorage(storage storage.Storage) Option {
 		return nil
 	}
 }
+
+func WithBackoffOpts(opts ...backoff.Option) Option {


[golangci] _{reported by reviewdog 🐶}
exported function WithBackoffOpts should have comment or be unexported (golint)

github-actions bot added area/internal team/set SET team size/L type/refactoring priority/low labels Jun 11, 2020

github-actions bot added team/sre SRE team type/ci labels Jun 11, 2020

github-actions bot added area/agent/sidecar area/helm labels Jun 11, 2020

github-actions bot added area/tools/cli/loadtest size/XL and removed size/L labels Jun 11, 2020

github-actions bot reviewed Jun 11, 2020

View reviewed changes

internal/db/storage/blob/s3/reader/reader.go Show resolved Hide resolved

internal/db/storage/blob/s3/reader/reader.go Show resolved Hide resolved

rinx changed the title ~~Refactor agent-sidecar: use io pipe for reading s3 bucket~~ Refactor agent-sidecar: fix S3 reader & add backoff logic Jun 11, 2020

rinx marked this pull request as ready for review June 11, 2020 03:43

kpango reviewed Jun 11, 2020

View reviewed changes

internal/db/storage/blob/s3/reader/reader.go Outdated Show resolved Hide resolved

github-actions bot reviewed Jun 11, 2020

View reviewed changes

github-actions bot added size/XXL and removed size/XL labels Jun 12, 2020

github-actions bot reviewed Jun 12, 2020

View reviewed changes

kpango previously approved these changes Jun 12, 2020

View reviewed changes

kpango changed the title ~~Refactor agent-sidecar: fix S3 reader & add backoff logic~~ [patch] Refactor agent-sidecar: fix S3 reader & add backoff logic Jun 12, 2020

rinx added 10 commits June 12, 2020 09:52

♻️ use io.pipe for reading s3 buckets

ac623f0

Signed-off-by: Rintaro Okamura <[email protected]>

✅ fix tests

9ba5648

Signed-off-by: Rintaro Okamura <[email protected]>

💚 Fix CI trigger for agent-sidecar

8ae3004

Signed-off-by: Rintaro Okamura <[email protected]>

✨ Add backoff option for agent sidecar initcontainer mode

5e48fcb

Signed-off-by: Rintaro Okamura <[email protected]>

✅ fix tests

c88f59a

Signed-off-by: Rintaro Okamura <[email protected]>

♻️ refactor the order of declaration.

d02d1f8

Signed-off-by: Rintaro Okamura <[email protected]>

✨ Add internal/net/http/client

6d31953

Signed-off-by: Rintaro Okamura <[email protected]>

✅ fix tests

27919fa

Signed-off-by: Rintaro Okamura <[email protected]>

🔧 Revise default duration

b3dc6a0

Signed-off-by: Rintaro Okamura <[email protected]>

🔧 add sigkill for errors on initcontainer

35fe989

Signed-off-by: Rintaro Okamura <[email protected]>

vdaas-ci dismissed kpango’s stale review via 35fe989 June 12, 2020 09:52

vdaas-ci force-pushed the refactor/agent-sidecar/use-io-pipe-for-reading-s3-backet branch from 05aa4bd to 35fe989 Compare June 12, 2020 09:52

🤖 Update license headers / Format go codes and yaml files

5278819

Signed-off-by: vdaas-ci <[email protected]>

vdaas-ci approved these changes Jun 12, 2020

View reviewed changes

github-actions bot reviewed Jun 12, 2020

View reviewed changes

kpango merged commit b26ac79 into master Jun 12, 2020

kpango deleted the refactor/agent-sidecar/use-io-pipe-for-reading-s3-backet branch June 12, 2020 10:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[patch] Refactor agent-sidecar: fix S3 reader & add backoff logic #467

[patch] Refactor agent-sidecar: fix S3 reader & add backoff logic #467

rinx commented Jun 11, 2020 •

edited

Loading

pull-assistant bot commented Jun 11, 2020 •

edited

Loading

codecov bot commented Jun 11, 2020 •

edited

Loading

vdaas-ci commented Jun 11, 2020

vdaas-ci commented Jun 11, 2020

vdaas-ci commented Jun 11, 2020

vdaas-ci commented Jun 12, 2020

github-actions bot Jun 12, 2020

vdaas-ci commented Jun 12, 2020

github-actions bot Jun 12, 2020

vdaas-ci commented Jun 12, 2020

kpango left a comment

kpango commented Jun 12, 2020

vdaas-ci commented Jun 12, 2020 •

edited by kpango

Loading

vdaas-ci commented Jun 12, 2020

vdaas-ci commented Jun 12, 2020

vdaas-ci commented Jun 12, 2020

vdaas-ci commented Jun 12, 2020

vdaas-ci left a comment

github-actions bot Jun 12, 2020

github-actions bot Jun 12, 2020

[patch] Refactor agent-sidecar: fix S3 reader & add backoff logic #467

[patch] Refactor agent-sidecar: fix S3 reader & add backoff logic #467

Conversation

rinx commented Jun 11, 2020 • edited Loading

Description:

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

pull-assistant bot commented Jun 11, 2020 • edited Loading

codecov bot commented Jun 11, 2020 • edited Loading

Codecov Report

vdaas-ci commented Jun 11, 2020

vdaas-ci commented Jun 11, 2020

vdaas-ci commented Jun 11, 2020

vdaas-ci commented Jun 12, 2020

github-actions bot Jun 12, 2020

Choose a reason for hiding this comment

vdaas-ci commented Jun 12, 2020

github-actions bot Jun 12, 2020

Choose a reason for hiding this comment

vdaas-ci commented Jun 12, 2020

kpango left a comment

Choose a reason for hiding this comment

kpango commented Jun 12, 2020

vdaas-ci commented Jun 12, 2020 • edited by kpango Loading

vdaas-ci commented Jun 12, 2020

vdaas-ci commented Jun 12, 2020

vdaas-ci commented Jun 12, 2020

vdaas-ci commented Jun 12, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

github-actions bot Jun 12, 2020

Choose a reason for hiding this comment

github-actions bot Jun 12, 2020

Choose a reason for hiding this comment

rinx commented Jun 11, 2020 •

edited

Loading

pull-assistant bot commented Jun 11, 2020 •

edited

Loading

codecov bot commented Jun 11, 2020 •

edited

Loading

vdaas-ci commented Jun 12, 2020 •

edited by kpango

Loading