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

Fix status.progress not getting updated for backup #6276

Merged
merged 1 commit into from
May 22, 2023
Merged

Fix status.progress not getting updated for backup #6276

merged 1 commit into from
May 22, 2023

Conversation

kkothule
Copy link
Contributor

@kkothule kkothule commented May 16, 2023

Thank you for contributing to Velero!

Please add a summary of your change

Fixed a bug where status.progress is not getting updated for backups.

Does your change fix a particular issue?

Fixes #6275

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

Copy link
Contributor

@blackpiglet blackpiglet left a comment

Choose a reason for hiding this comment

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

Please make the modification according to the comments.

@kkothule kkothule requested a review from blackpiglet May 18, 2023 08:40
@kkothule
Copy link
Contributor Author

kkothule commented May 18, 2023

Thank you @blackpiglet, I have update PR by addressing review comments
Please review

Signed-off-by: kkothule <kkothule@catalogicsoftware.com>
@kkothule kkothule requested a review from blackpiglet May 18, 2023 10:02
blackpiglet
blackpiglet previously approved these changes May 18, 2023
@kkothule
Copy link
Contributor Author

@Lyndon-Li , @shubham-pampattiwar please review

@anshulahuja98
Copy link
Collaborator

@kkothule pl fix the linting
Running golangci-lint run -c hack/../golangci.yaml
pkg/backup/backup_pv_action.go:79: unnecessary trailing newline (whitespace)

@kkothule
Copy link
Contributor Author

kkothule commented May 22, 2023

@kkothule pl fix the linting Running golangci-lint run -c hack/../golangci.yaml pkg/backup/backup_pv_action.go:79: unnecessary trailing newline (whitespace)

@anshulahuja98 The file pkg/backup/backup_pv_action.go is not part of my PR, Should i still go ahead and fix the issue?
The lint work well for previous push

@codecov-commenter
Copy link

Codecov Report

Merging #6276 (317083d) into main (fe5182d) will decrease coverage by 0.22%.
The diff coverage is 26.59%.

❗ Current head 317083d differs from pull request most recent head 6a569ca. Consider uploading reports for the commit 6a569ca to get more accurate results

@@            Coverage Diff             @@
##             main    #6276      +/-   ##
==========================================
- Coverage   41.20%   40.99%   -0.22%     
==========================================
  Files         252      255       +3     
  Lines       23503    23818     +315     
==========================================
+ Hits         9684     9763      +79     
- Misses      13061    13293     +232     
- Partials      758      762       +4     
Impacted Files Coverage Δ
pkg/client/factory.go 20.22% <0.00%> (-0.71%) ⬇️
pkg/cmd/cli/nodeagent/server.go 11.76% <0.00%> (-0.16%) ⬇️
pkg/cmd/server/server.go 5.08% <0.00%> (-0.03%) ⬇️
pkg/cmd/util/output/backup_describer.go 0.00% <0.00%> (ø)
pkg/controller/pod_volume_restore_controller.go 20.16% <0.00%> (-4.84%) ⬇️
pkg/install/resources.go 48.55% <0.00%> (-1.03%) ⬇️
pkg/repository/config/aws.go 22.72% <0.00%> (-1.67%) ⬇️
pkg/repository/udmrepo/kopialib/backend/s3.go 28.57% <0.00%> (-23.29%) ⬇️
pkg/uploader/provider/provider.go 0.00% <0.00%> (ø)
pkg/uploader/provider/restic.go 26.04% <ø> (ø)
... and 11 more

... and 1 file with indirect coverage changes

@kkothule kkothule requested a review from blackpiglet May 22, 2023 06:44
@shubham-pampattiwar shubham-pampattiwar merged commit 7fa9106 into vmware-tanzu:main May 22, 2023
42 of 43 checks passed
@draghuram
Copy link
Contributor

In today's community call, we discussed about back-porting this PR to 1.11 as it fixes a high impact problem. @sseago mentioned that there is no clearly defined process for selecting fixes for back-porting, as there is for major releases. One solution is to require two or more maintainers to agree to back-port.

As that process is discussed, it would be nice if we can make a decision about this particular issue being part of 1.11.1. CC @blackpiglet @shubham-pampattiwar @weshayutin.

@sseago
Copy link
Collaborator

sseago commented May 30, 2023

@draghuram Actually, we just merged that today: #6324
I didn't realize during the call that the cherry-pick PR to 1.11 was already submitted, so we were able to review and merge it today.

@blackpiglet
Copy link
Contributor

@draghuram
We are using the labels to trace PR back-porting.
#6275 is labeled the tag target/1.11.1.

@draghuram
Copy link
Contributor

Great. Thanks.

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.

Progress field is missing in status of backup CR
7 participants