Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨[patch] Implementation of agent-sidecar storage backup logic #409

Merged
merged 39 commits into from
Jun 3, 2020

Conversation

rinx
Copy link
Contributor

@rinx rinx commented May 27, 2020

Description:

In this PR, a blob storage backup logic of agent-sidecar is implemented.
Currently, it only supports S3.

Bugfix:

  • internal/core/ngt/ngt.go ... remove useless C.frees

Related Issue:

Nothing.

How Has This Been Tested?:

Nothing.

Environment:

  • Golang 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
Copy link

pull-assistant bot commented May 27, 2020

Score: 0.95

Best reviewed: commit by commit


Optimal code review plan (10 warnings)

✨ Implement S3 multipart upload

...l/db/storage/blob/s3/option.go 55% changes removed in 🚚 rename s3cli...

.../db/storage/blob/s3/s3_test.go 43% changes removed in ✅ f...

✨ Added single upload mode for s3 writer

...orage/blob/s3/writer/option.go 57% changes removed in ✨ Add max_p...

...orage/blob/s3/writer/writer.go 51% changes removed in ✨ Add max_p...

     🔥 Remove C.frees in NGT.Close func

     ✨ Add Reader Writer funcs to compress structs

✨ Add sidecar backup implementations

...ernal/db/storage/blob/s3/s3.go 67% changes removed in ✨ Add max_p...

...l/db/storage/blob/s3/option.go 56% changes removed in 🚚 rename s3cli...

...ar/service/storage_observer.go 43% changes removed in 🐛 backup trigger...

     🐳 Fix Dockerfile for agent-sidecar

     ✅ Add tests

♻️ use single dir for watch target

...ar/service/storage_observer.go 70% changes removed in ♻️ revise obs...

⚡ Using io.Pipe

...ar/service/storage_observer.go 47% changes removed in ♻️ revise obs...

✅ gen tests

...ce/blob_storage_option_test.go 61% changes removed in ✨ Add initc...

     ✨ Add initcontainer mode for agent-sidecar

✅ gen tests

.../db/storage/blob/s3/s3_test.go 50% changes removed in ✅ f...

internal/config/blob_test.go 50% changes removed in ✅ f...

     ☸️ Add sidecar to chart

     🔥 Remove C.frees

✅ Fix tests

.../db/storage/blob/s3/s3_test.go 55% changes removed in ✅ f...

     ☸️ remove agent.sidecar.volumeMounts

     ♻️ fix behavior of agent-sidecar initContainer mode

     🐛 backup triggered from a single goroutine

     🔥 Remove auto_backup_duration_limit

     ☸️ Add agent-sidecar service

     ☸️ Fix templates for agent initContainers

     ♻️ Fix order of defer

     ✨ Add wg

     ♻️ Add another wg

     ✅ Fix tests

     ♻️ Fix order of closing io.Writer, io.Reader

     ✏️ Fix typo

🐛 Fix

...r/service/observer/observer.go 86% changes removed in ♻️ revise obs...

     ✨ Add poststop

     🔧 revise max-part-size

     🔧 Fix maxPartSize

✨ Add max_part_size_mb parameter

...ernal/db/storage/blob/s3/s3.go 50% changes removed in 🚚 rename s3cli...

     ✅ fix broken tests

     🚨 fix warnings of deepsource go

     ☸️ update terminationGracePeriodSeconds of agent

     ♻️ revise observer backup function

     ✅ fix tests and fix permissions

     🚚 rename s3client to client / add validation for s3 partsize

     ✅ fix tests

Powered by Pull Assistant. Last update cdd9817 ... 5a23ba3. Read the comment docs.

@vdaas-ci
Copy link
Collaborator

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

internal/core/ngt/ngt.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #409 into master will decrease coverage by 0.36%.
The diff coverage is 0.91%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #409      +/-   ##
=========================================
- Coverage    7.71%   7.35%   -0.37%     
=========================================
  Files         372     383      +11     
  Lines       18845   19664     +819     
=========================================
- Hits         1454    1446       -8     
- Misses      17178   17999     +821     
- Partials      213     219       +6     
Impacted Files Coverage Δ
internal/compress/gob.go 27.50% <0.00%> (-33.62%) ⬇️
internal/compress/gzip.go 34.00% <0.00%> (-17.52%) ⬇️
internal/compress/lz4.go 41.66% <0.00%> (-17.16%) ⬇️
internal/compress/zstd.go 35.29% <0.00%> (-17.65%) ⬇️
internal/config/blob.go 0.00% <0.00%> (ø)
internal/config/compress.go 0.00% <0.00%> (ø)
internal/config/sidecar.go 0.00% <0.00%> (ø)
internal/db/storage/blob/s3/reader/option.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%> (ø)
... and 33 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 7bf7dbf...5a23ba3. Read the comment docs.

@rinx rinx force-pushed the feature/agent-sidecar/implement-storage-backup-logic branch from 32e50d0 to a548f15 Compare May 27, 2020 11:49
@vdaas-ci
Copy link
Collaborator

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

1 similar comment
@vdaas-ci
Copy link
Collaborator

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

pkg/agent/sidecar/service/storage_observer.go Outdated Show resolved Hide resolved
pkg/agent/sidecar/service/storage_observer.go Outdated Show resolved Hide resolved
internal/config/sidecar.go Outdated Show resolved Hide resolved
@vdaas-ci
Copy link
Collaborator

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

2 similar comments
@vdaas-ci
Copy link
Collaborator

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

@vdaas-ci
Copy link
Collaborator

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

@rinx rinx force-pushed the feature/agent-sidecar/implement-storage-backup-logic branch from ffdef45 to 27c3bad Compare May 28, 2020 07:21
@vdaas-ci
Copy link
Collaborator

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

@rinx rinx force-pushed the feature/agent-sidecar/implement-storage-backup-logic branch from 27c3bad to db07fab Compare May 28, 2020 07:40
@vdaas-ci
Copy link
Collaborator

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

@rinx rinx force-pushed the feature/agent-sidecar/implement-storage-backup-logic branch from db07fab to c18cf63 Compare May 28, 2020 09:12
@vdaas-ci
Copy link
Collaborator

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

@rinx rinx marked this pull request as ready for review May 28, 2020 09:13
@rinx rinx force-pushed the feature/agent-sidecar/implement-storage-backup-logic branch from c18cf63 to 6f61a72 Compare May 28, 2020 09:37
rinx added 25 commits June 3, 2020 09:01
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@vdaas-ci vdaas-ci force-pushed the feature/agent-sidecar/implement-storage-backup-logic branch from d36d7ee to 5a23ba3 Compare June 3, 2020 09:01
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 3, 2020

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

@rinx rinx merged commit 58ed429 into master Jun 3, 2020
@rinx rinx deleted the feature/agent-sidecar/implement-storage-backup-logic branch June 3, 2020 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants