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

Bump up kopia v0.13 #6248

Merged
merged 1 commit into from
May 15, 2023
Merged

Conversation

Lyndon-Li
Copy link
Contributor

@Lyndon-Li Lyndon-Li commented May 10, 2023

Bump up kopia to v0.13

Fix issue #5999

@github-actions github-actions bot added Dependencies Pull requests that update a dependency file has-unit-tests labels May 10, 2023
@Lyndon-Li Lyndon-Li force-pushed the bump-up-kopia-v0.13 branch 2 times, most recently from a4930c5 to b497863 Compare May 10, 2023 01:42
@Lyndon-Li
Copy link
Contributor Author

We face this problem of controller-gen after bumping up and changing go.mod, as a result, the make update always crashes. So we need to bump up controller-gen to v0.12.0.

@Lyndon-Li Lyndon-Li force-pushed the bump-up-kopia-v0.13 branch from b497863 to 76e2a8f Compare May 10, 2023 02:08
@codecov-commenter
Copy link

codecov-commenter commented May 10, 2023

Codecov Report

Merging #6248 (307b82a) into main (4db1a78) will decrease coverage by 0.01%.
The diff coverage is 32.02%.

@@            Coverage Diff             @@
##             main    #6248      +/-   ##
==========================================
- Coverage   41.19%   41.18%   -0.01%     
==========================================
  Files         252      252              
  Lines       23418    23490      +72     
==========================================
+ Hits         9647     9675      +28     
- Misses      13015    13057      +42     
- Partials      756      758       +2     
Impacted Files Coverage Δ
pkg/repository/udmrepo/kopialib/backend/azure.go 91.30% <0.00%> (ø)
pkg/repository/udmrepo/kopialib/backend/common.go 22.22% <0.00%> (ø)
pkg/repository/udmrepo/kopialib/backend/gcs.go 55.55% <0.00%> (ø)
pkg/repository/udmrepo/kopialib/backend/s3.go 53.84% <0.00%> (ø)
pkg/repository/udmrepo/kopialib/lib_repo.go 30.14% <0.00%> (-0.18%) ⬇️
pkg/uploader/kopia/progress.go 0.00% <0.00%> (ø)
pkg/uploader/kopia/shim.go 0.00% <0.00%> (ø)
pkg/util/logging/kopia_log.go 37.23% <36.36%> (+12.90%) ⬆️
pkg/uploader/kopia/snapshot.go 34.09% <72.22%> (-3.75%) ⬇️
pkg/uploader/provider/kopia.go 53.22% <100.00%> (-4.04%) ⬇️

... and 2 files with indirect coverage changes

@Lyndon-Li Lyndon-Li force-pushed the bump-up-kopia-v0.13 branch from 76e2a8f to 9fea274 Compare May 10, 2023 02:26
@Lyndon-Li Lyndon-Li marked this pull request as ready for review May 10, 2023 09:57
@github-actions github-actions bot requested review from blackpiglet and sseago May 10, 2023 09:57
@Lyndon-Li Lyndon-Li requested a review from qiuming-best May 10, 2023 09:57
@@ -68,6 +68,7 @@ type ObjectWriteOptions struct {
Prefix ID // A prefix of the name used to save the object
AccessMode int // OBJECT_DATA_ACCESS_*
BackupMode int // OBJECT_DATA_BACKUP_*
AsyncWrites int // Num of async writes for the object, 0 means no async write
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there is no place to assign value to the AsyncWrites parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in (sr *shimRepository) NewObjectWriter, it is an optional value, so right now, we don't need to set a value, just convey the value from shim to unified repo.

return object.ID{}, errors.New("ConcatenateObjects is not supported")
}

func (sr *shimRepository) OnSuccessfulFlush(callback repo.RepositoryWriterCallback) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious about why not OnSuccessfulFlush function return value handling is different from other newly added functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prototype of these functions are defined by Kopia, specifically, they implements Kopia' Repository interface. In the interface, OnSuccessfulFlush has no return value.
On the other hand, this function is a callback under the Flush operation, while this callback is optional, if it is not to be handled, just leave it as empty.

@blackpiglet
Copy link
Contributor

blackpiglet commented May 13, 2023

Could you give more information on why the uber zap log library is used instead in pkg/util/logging/kopia_log.go?

Signed-off-by: Lyndon-Li <[email protected]>
@Lyndon-Li Lyndon-Li force-pushed the bump-up-kopia-v0.13 branch from 9fea274 to 307b82a Compare May 14, 2023 23:23
@Lyndon-Li
Copy link
Contributor Author

Could you give more information on why the uber zap log library is used instead in pkg/util/logging/kopia_log.go?

pkg/util/logging/kopia_log.go is still used. zap log is the logger used by Kopia all the time.
In the previous release, Kopia has a wrapper of the zap log, so Velero doesn't need to hook zap log in order to redirect Kopia log to Velero.
In the current release, zap log is used directly by Kopia, so in Velero code we have to hook zap log for the same purpose.

@Lyndon-Li Lyndon-Li merged commit ea4e49f into vmware-tanzu:main May 15, 2023
@Lyndon-Li Lyndon-Li deleted the bump-up-kopia-v0.13 branch May 15, 2023 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file has-changelog has-unit-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants