-
Notifications
You must be signed in to change notification settings - Fork 483
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
HOSTEDCP-1416: Hosted Control Planes ETCD Backup API #1553
Conversation
@jparrill: This pull request references HOSTEDCP-1416 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
#1555 is changing the enhancement template in a way that will cause the header check in the linter job to fail for existing PRs. If this PR is merged within the development period for 4.16 you may override the linter if the only failures are caused by issues with the headers (please make sure the markdown formatting is correct). If this PR is not merged before 4.16 development closes, please update the enhancement to conform to the new template. |
Hey @dhellmann thanks for the heads up, I will take that in mind before the final submission :). |
b6a1715
to
0eac566
Compare
Mentioning the Hypershift team in order to review and add their thoughts (sorry in advance for the spam): /cc @csrwng @sjenning @muraee @enxebre @derekwaynecarr @imain @bryan-cox @Patryk-Stefanski @celebdor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few typos and nitpicks. Otherwise lgtm
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
2d625b7
to
2f661e2
Compare
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
3024126
to
2c442cc
Compare
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
enhancements/hypershift/disaster-recovery/hcp-etcd-backup-api.md
Outdated
Show resolved
Hide resolved
2c442cc
to
abf5b43
Compare
Yep, me too but I prefer to have an MVP with smaller footprint and then add new storage providers. |
Signed-off-by: Juan Manuel Parrilla Madrid <[email protected]>
abf5b43
to
9bd4663
Compare
@jparrill: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall the direction here seems reasonable, but I would like to see some more detail on the responsibilities of the controllers and how and where we re-use OCP core functionality versus where we're adding on top or re-implementing.
Agree, in this case we are adapting the controllers of the ETCD Operator in our code because we don't want to get the whole ETCD Operator as part of the ControlPlane (for scalability reasons) (apart that in their case, this is using a separated CRD). So the responsibility relies on us and we can follow the direction they followed. |
After a team call discussion we decided to go through a couple of spikes regarding OADP and vol-synk from ACM/MCE. I will update the design afterwards. |
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
Stale enhancement proposals rot after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle rotten |
The decision over this enhancement is basically cover the functionality with OADP. We will develop the internals in a document instead of en enhancement, which IMHO makes more sense. |
(automated message) This pull request is closed with lifecycle/rotten. The associated Jira ticket, HOSTEDCP-1370, has status "Closed, Obsolete". Should the PR be reopened, updated, and merged? If not, removing the lifecycle/rotten label will tell this bot to ignore it in the future. |
1 similar comment
(automated message) This pull request is closed with lifecycle/rotten. The associated Jira ticket, HOSTEDCP-1370, has status "Closed, Obsolete". Should the PR be reopened, updated, and merged? If not, removing the lifecycle/rotten label will tell this bot to ignore it in the future. |
This PR contains the definition of a new API under the
hostedcluster.spec.etcd
group which it's calledbackup
and it will help to perform automated etcd backups using the Hypershift API.We are reusing the ETCD backup API, which will help on the homogenisation in the backups for Openshift and Hypershift.