fix(backup): manually synchronizing backup volumes.

[mantissahz]

Which issue(s) this PR fixes:

Issue # longhorn/longhorn#7982

What this PR does / why we need it:

When users make a requst to get backup volumes information by Backup Volume Get/List APIs, it will also trigger the synchronization for these backup volumes once.

Special notes for your reviewer:

Additional documentation or context

[derekbit]

When users make a BackupVolumeGet or BackupVolumeList HTTP call, they can trigger a backup volume refresh.

Is it possible to synchronize backup volumes by simply editing customer resources? Or do we need to a different method that doesn't involve HTTP calls?

[mantissahz]

Is it possible to synchronize backup volumes by simply editing customer resources? Or do we need to a different method that doesn't involve HTTP calls?

Users can modify the field spec.syncRequestedAt of the BackupVolume CR for synchronizing the backup volume. And the time of the filed spec.syncRequestedAt should be after the time of the filed status.lastSyncedAt

[shuo-wu]

Can we just simply update backuptarget.spec.syncRequestedAt instead? Updating the spec for all backup volumes is probably not enough since there may be backup volume CR creation/deletion.

[mantissahz]

Can we just simply update backuptarget.spec.syncRequestedAt instead? Updating the spec for all backup volumes is probably not enough since there may be backup volume CR creation/deletion.

Yes, we can and modified. Thanks.

[shuo-wu]

That's a good idea. And you don't need to worry about the schema validation. I think we can create a separate PR for this improvement.

[mantissahz]

backupTarget.Spec.SyncRequestedAt = "now" // metav1.Time{Time: time.Now().UTC()}

I think it isn't easy to have a constant value representing to trigger the sync , and the mutator will update it with a qualified time value instead because of https://github.com/longhorn/longhorn-manager/blob/master/webhook/admission/request.go#L43.

[derekbit]

Hello @mantissahz Is the PR ready?

[mantissahz]

Hi @derekbit, Yes, it's ready. Thanks

[innobead]

backupTarget.Spec.SyncRequestedAt = "now" // metav1.Time{Time: time.Now().UTC()}

I think it isn't easy to have a constant value representing to trigger the sync , and the mutator will update it with a qualified time value instead because of master/webhook/admission/request.go#L43.

I meant, you can rely on the mutator to update now to a qualified time.

[innobead]

In general, LGTM. however, still have some feedback required to resolve.

P.S. In the current implementation, it's specific for REST API. How do users trigger it by updating the CR? need to exactly update the right time in the syncat field of backup target or backup volume on their own? I still recommend a way that can benefit both interfaces (UI and CLI kubectl)

After a quick discussion with @mantissahz , we added type validation for the sync_at field, so it's infeasible to use a non-time value to trigger the sync. so for non-UI users, we can create a CLI command for it. James will create a ticket for the following improvement.

/chart/templates/crds.yaml#L961-L965

              syncRequestedAt:
                description: The time to request run sync the remote backup target.
                format: date-time
                nullable: true
                type: string

[derekbit]

@mantissahz Ready to review?

[mantissahz]

@mantissahz Ready to review?

@derekbit, Yes, it is ready.

[mantissahz]

@Mergifyio backport v1.6.x

[mergify[bot]]

backport v1.6.x

✅ Backports have been created

• #2788 fix(backup): manually synchronizing backup volumes. (backport #2660) has been created for branch v1.6.x