-
Notifications
You must be signed in to change notification settings - Fork 84
OADP-6765: docs: add CA Certificate Bundle documentation for ImageStream backups #1974
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
OADP-6765: docs: add CA Certificate Bundle documentation for ImageStream backups #1974
Conversation
Signed-off-by: Tiger Kaovilai <[email protected]>
|
Skipping CI for Draft Pull Request. |
| - ❌ Toggling `backupImages` setting | ||
| - ❌ Adding/removing DPA `spec.backupLocations` unnecessarily | ||
|
|
||
| **Future improvement**: OADP may implement a watch on all BSLs in the namespace (not just owned ones) to automatically detect Non-DPA BSL changes, eliminating the need for manual triggering. Currently, `Owns(&velerov1.BackupStorageLocation{})` only watches OADP-created BSLs. |
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.
This is an enhancement that we can do now as immediate follow up if necessary.. or later follow up.
…ro-plugin-for-aws Signed-off-by: Tiger Kaovilai <[email protected]>
…m backups Signed-off-by: Tiger Kaovilai <[email protected]>
|
@kaovilai: This pull request references OADP-6765 which is a valid jira issue. 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. |
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.
/approve /LGTM
…tation for ImageStream backups Signed-off-by: Tiger Kaovilai <[email protected]>
…ckups Signed-off-by: Tiger Kaovilai <[email protected]>
…arameters for CA certificate handling Signed-off-by: Tiger Kaovilai <[email protected]>
…eters for CA certificate handling Signed-off-by: Tiger Kaovilai <[email protected]>
|
/lgtm |
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.
We need to see some doc referencing the need for
backupstoragelocations.velero.io minio app.kubernetes.io/component=bsl
|
Will update with current label behavior leaving room for enhancements |
…and CA certificate collection process Signed-off-by: Tiger Kaovilai <[email protected]>
|
|
||
| From `tests/e2e/backup_restore_suite_test.go`: | ||
|
|
||
| **When `backupImages=true`** (line 638-649): |
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.
Is this reference outdated? The tests/e2e/backup_restore_suite_test.go has 469 lines of code...
Also avoid using lines here when referencing file that is not permanent link.
| }) |
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.
will follow up
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.
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.
/lgtm
One nit in the code pointing to a file from different branch? but it's ok as is - very clear and informative.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai, mpryc, shubham-pampattiwar, weshayutin 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 |
|
/override ci/prow/4.19-images ci/prow/4.20-ci-index ci/prow/4.20-images ci/prow/images ci/prow/unit-test |
|
@kaovilai: Overrode contexts on behalf of kaovilai: ci/prow/4.19-images, ci/prow/4.20-ci-index, ci/prow/4.20-images, ci/prow/images, ci/prow/unit-test 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 kubernetes-sigs/prow repository. |
|
@kaovilai: 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-sigs/prow repository. I understand the commands that are listed here. |
Signed-off-by: Tiger Kaovilai [email protected]
Why the changes were made
How to test the changes made