Add Design for Upload Speed Test with BSL Config for Object Storage #1558
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
added
the
approved
docs/design/speed-test.md
Outdated
| config: | ||
| region: us-east-1 | ||
| bucket: my-backup-bucket | ||
| uploadSpeedTest: |
There was a problem hiding this comment.
If we're not going to support automatic speed checks, I'm confused why the DPA needs to change initially.
There was a problem hiding this comment.
Auto-Check or not depends on the user configuration here. For each BSL configured we have a uploadSpeedTest struct that has enabled flag and uploadSpeedTest config. If the user sets these then UST CR would be automatically created by OADP Operator for that particular BSL, else the user could create the UST CR themselves.
shubham-pampattiwar
force-pushed
the
add-speed-test-design
branch
from
50ba90f to
a24b4d4
Compare
| ## Abstract | ||
|
|
||
| This document presents the design of a Custom Resource Definition (CRD) and its controller to test the upload speed from an OpenShift cluster to cloud object storage. | ||
| The design leverages the BackupStorageLocation configuration from the OADP operator’s DPA CRD to specify object storage locations for testing. The controller will use |
There was a problem hiding this comment.
we intend to only measure BSLs created by DPA and not all BSLs in OADP namespace?
There was a problem hiding this comment.
So these are 2 separate things:
- You can independently create UST CR for any BSL config (so this would work for OADP BSL and Non-OADP BSLs)
- We have a trigger for OADP BSLs (in this case DPA controller will create a UST CR)
So all the scenarios are covered I believe.
| cloudProviderSecretRef: | ||
| name: cloud-credentials | ||
| namespace: openshift-adp | ||
| ``` |
There was a problem hiding this comment.
Could we change here to only have 2 fields in spec (uploadSpeedTestConfig and backupLocationName)?
cloudProviderSecretRef can be fetched from BSL, right? So does not seem needed
Having BSL spec copied here makes it harder for user to manually create one (even thinking about DPA integration, if BSL spec changes, UST must change. Having just name reference would not simplify workflow?)
There was a problem hiding this comment.
yeah I was thinking about the exact same thing. Making cloudProviderSecretRef optional maybe ? or just remove it.
There was a problem hiding this comment.
Done, removed cloudProviderSecretRef from UST CRD spec, tested AWS POC as well.
| ## High-Level Design | ||
| Components involved and their responsibilities: | ||
|
|
||
| - UploadSpeedTest (UST) CRD: |
There was a problem hiding this comment.
Reading whole design, would make sense to change CRD name to something more generic (NetworkSpeedTest?). If more features, like latency and download speed, are added, the CRD name would not be telling users all things it can do?
There was a problem hiding this comment.
Yeah I had thought about that, Any Network speed would consist of upload as well as download speed, the risk here with NetworkSpeedTest is that it might give a false impression that we are catering/calculating download speed as well, but thats not our goal here. Hence UploadSpeedTest.
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
| metadata: | ||
| name: my-upload-speed-test | ||
| spec: | ||
| backupLocation: |
There was a problem hiding this comment.
| backupLocation: | |
| backupStorageLocationName: <name of bsl to test> | |
| backupLocation: |
There was a problem hiding this comment.
I think we want UST to be able to be used independently (in addition to via DPA) by providing the BSLConfig. Thinking of the the use-case:
- User has a BSL Config,
- they test the BSLConfig via UST
- now use the BSLConfig to create BSL
There was a problem hiding this comment.
So we do not want to reference by name to an already existing BSL?
There was a problem hiding this comment.
Yeah because we already have the functionality to test a particular BSL via DPA, proposed here . Why keep it bi-directional ! I believe this keeps the CRD simpler.
There was a problem hiding this comment.
So an upstream velero user would not be able to use this without copying the whole bsl.spec?
There was a problem hiding this comment.
Perhaps that was not the plan.
There was a problem hiding this comment.
not an upstream feature :)
There was a problem hiding this comment.
Sounds like an easy add. If it does not complicate our designs, I would propose we do things in an upstream usable way.
It'd be much cooler to say "you can try our team's tool to debug your bsl speed"
vs
"If you were using OADP, you'd have ability to debug your bsl speed"
There was a problem hiding this comment.
Can you clarify which path it would be writing to?
How will it handle "bucket full" related errors?
Let say I wanted to test upload of 2TB, we hit bucket full at 1TB. Do we throw away current results and error? or will it preserve info gathered so far ie. how long it took to upload 1TB even tho user specified 2TB?
There was a problem hiding this comment.
Do we want to have continuous test as an option?
ie. don't stop uploading until user change spec from running to false.
Will user see live progress of the current speed (ie. speed over short time interval like 1s)? or only after full test will they get results?
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
|
@shubham-pampattiwar: 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. |
Why the changes were made
Design for Upload Speed Test CRD and Controller
How to test the changes made
Review the Design