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.

Sign up for GitHub

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

Jump to bottom

Update the full snapshot lease with renewTime set to the time FullSnapshot is taken #753

Merged
merged 3 commits into from
Sep 9, 2024
Merged

Update the full snapshot lease with renewTime set to the time FullSnapshot is taken #753

merged 3 commits into from
Sep 9, 2024

Conversation

anveshreddy18
Copy link
Contributor

@anveshreddy18 anveshreddy18 commented Jul 23, 2024 •
edited by ishan16696
Loading

What this PR does / why we need it:

During update of full snapshot lease, set renewTime to the time the full snapshot was taken, instead of setting it to the time when the lease was updated as part of retry mechanism.

Which issue(s) this PR fixes:
Fixes #752

Special notes for your reviewer:

Release note:

Improves the `renewTime` of full snapshot lease when the lease is updated as part of retry mechanism

@anveshreddy18 anveshreddy18 requested a review from a team as a code owner July 23, 2024 06:34
@gardener-robot gardener-robot added the needs/review Needs review label Jul 23, 2024
@anveshreddy18 anveshreddy18 self-assigned this Jul 23, 2024
@gardener-robot gardener-robot added the size/s Size of pull request is small (see gardener-robot robot/bots/size.py) label Jul 23, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jul 23, 2024
@ishan16696
Copy link
Member

/assign

ishan16696
ishan16696 requested changes Sep 5, 2024
View reviewed changes
pkg/snapshot/snapshotter/fullsnapshotleaseupdate.go Outdated Show resolved Hide resolved
pkg/snapshot/snapshotter/snapshotter.go Outdated Show resolved Hide resolved
pkg/snapshot/snapshotter/snapshotter.go Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Sep 5, 2024
@anveshreddy18
update the full snapshot lease with renewTime set to the time full sn…
ed48157 
…apshot was taken instead of when it's renewed as part of retry method
@anveshreddy18 anveshreddy18 force-pushed the full-snap-lease-renewTime branch from a6177a6 to ed48157 Compare September 6, 2024 10:50
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 6, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 6, 2024
ishan16696
ishan16696 requested changes Sep 6, 2024
View reviewed changes
pkg/compactor/compactor.go Outdated Show resolved Hide resolved
pkg/server/backuprestoreserver.go Outdated Show resolved Hide resolved
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 6, 2024
@ishan16696 ishan16696 added the merge/squash Should be merged via 'Squash and merge' label Sep 6, 2024
ishan16696
ishan16696 reviewed Sep 6, 2024
View reviewed changes
Copy link
Member

@ishan16696 ishan16696 left a comment

Choose a reason for hiding this comment

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

Overall looks good, 1 nit:

pkg/health/heartbeat/heartbeat.go Outdated Show resolved Hide resolved
pkg/health/heartbeat/heartbeat.go Outdated Show resolved Hide resolved
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 7, 2024
ishan16696
ishan16696 approved these changes Sep 9, 2024
View reviewed changes
Copy link
Member

@ishan16696 ishan16696 left a comment

Choose a reason for hiding this comment

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

LGTM!!

@ishan16696
Copy link
Member

@anveshreddy18 please re-trigger the pipeline tests.

@zkdev zkdev added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 9, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 9, 2024
@anveshreddy18 anveshreddy18 merged commit 175d902 into gardener:master Sep 9, 2024
9 checks passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Sep 9, 2024
@anveshreddy18 anveshreddy18 deleted the full-snap-lease-renewTime branch September 9, 2024 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ishan16696 ishan16696 ishan16696 approved these changes

Assignees

@ishan16696 ishan16696

@anveshreddy18 anveshreddy18

Labels
merge/squash Should be merged via 'Squash and merge' needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Milestone
No milestone
6 participants
@anveshreddy18 @ishan16696 @gardener-robot-ci-1 @zkdev @gardener-robot-ci-2 @gardener-robot