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

Backup: init backup, restore and migrate api #377

Merged
merged 6 commits into from
Sep 8, 2023

Conversation

Xieql
Copy link
Contributor

@Xieql Xieql commented Aug 25, 2023

What type of PR is this?

/kind api-change
/kind design
/kind documentation

What this PR does / why we need it:
add backup sync api
see #374

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

add backup sync api 

@netlify
Copy link

netlify bot commented Aug 25, 2023

Deploy Preview for kurator-dev ready!

Name Link
🔨 Latest commit 9298ea2
🔍 Latest deploy log https://app.netlify.com/sites/kurator-dev/deploys/64f9c12a288be800085569e3
😎 Deploy Preview https://deploy-preview-377--kurator-dev.netlify.app/references/backups_v1alpha1_types
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kurator-bot
Copy link
Collaborator

@Xieql: The label(s) kind/design cannot be applied, because the repository doesn't have them.

In response to this:

/kind api-change
/kind design
/kind documentation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Xieql Xieql changed the title Backup: add backup sync api Backup: init backup, restore, migrate api Aug 26, 2023
@Xieql Xieql changed the title Backup: init backup, restore, migrate api Backup: init backup, restore and migrate api Aug 26, 2023
@Xieql Xieql marked this pull request as draft August 28, 2023 02:25
@Xieql Xieql force-pushed the backup-api branch 3 times, most recently from 9a1088b to bf1435f Compare August 29, 2023 12:26
@Xieql Xieql marked this pull request as ready for review August 29, 2023 12:29
@Xieql Xieql marked this pull request as draft August 30, 2023 09:27
@Xieql Xieql marked this pull request as ready for review August 30, 2023 09:53
@Xieql
Copy link
Contributor Author

Xieql commented Aug 31, 2023

/label tide/merge-method-squash

@Xieql Xieql marked this pull request as draft September 4, 2023 07:10
// ```
// +optional
// +nullable
OrderedResources map[string]string `json:"orderedResources,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

why do we need order for backup

Copy link
Contributor Author

@Xieql Xieql Sep 5, 2023

Choose a reason for hiding this comment

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

The resource backup order feature provided by Velero is primarily designed to address scenarios like primary-secondary database configurations.
In such setups, there's an inherent delay in data synchronization or state propagation.
To ensure data consistency and coherence after backup, it's imperative to first back up the 'source' resource upon which others depend, such as the master database, followed by the 'dependent' resources, like the slave database.

see the proposal about orderedResouces in velero: https://github.com/vmware-tanzu/velero/blob/c9e1ade1f7c3b52ac3a38ae853e79a507b33bf95/design/Implemented/backup-resources-order.md

// If empty, it applies to all resources.
// +optional
// +nullable
IncludedResources []string `json:"includedResources,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

add examples

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

// ExcludedResources specifies the resources to which will not restore the status.
// +optional
// +nullable
ExcludedResources []string `json:"excludedResources,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

add examples

// Clusters determine a set of clusters as destination clusters.
// The field will only take effect when Fleet is not set.
// +optional
Clusters []*corev1.ObjectReference `json:"clusters,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

so this means clusters will have lower priority

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes.

Alternatively, we could allow both to coexist and then take the union.

Copy link
Contributor Author

@Xieql Xieql Sep 5, 2023

Choose a reason for hiding this comment

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

We can simplify the design by removing the ClusterSelector. If both Fleet and Clusters are specified, we will take their union as the final set of clusters.
ClusterSelector is confusing currently, we can consider reintroducing ClusterSelector in the future if the "selector" need arises.

Copy link
Member

@hzxuzhonghu hzxuzhonghu left a comment

Choose a reason for hiding this comment

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

Please rebase

@kurator-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Xieql
Copy link
Contributor Author

Xieql commented Sep 6, 2023

Please rebase

done

@hzxuzhonghu
Copy link
Member

/lgtm

Signed-off-by: Xieql <[email protected]>
@kurator-bot kurator-bot removed the lgtm label Sep 7, 2023
@hzxuzhonghu
Copy link
Member

/lgtm

@kurator-bot kurator-bot merged commit 1709fe3 into kurator-dev:main Sep 8, 2023
9 checks passed
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