OADP-363 plugin-registry design, registry deployment removal proposal #737
Conversation
kaovilai
changed the title
kaovilai
changed the title
openshift-ci
bot
requested review from
savitharaghunathan and
shubham-pampattiwar
docs/design/plugin-registry.md
Outdated
|
|
||
| ### Implementation Details/Notes/Constraints [optional] | ||
|
|
||
| The logs relating to registry will be mixed into velero pod logs |
There was a problem hiding this comment.
How noisy are these logs? Is this something we can control with the debug flag?
There was a problem hiding this comment.
You can get logs per container:
oc logs <pod-name> -c <conatiner_name>
There was a problem hiding this comment.
@sseago these logs will be mixed into velero container. Logs only occur when registry is accessed. When no backup is happening, it will be silent.
There was a problem hiding this comment.
The registry sidecar container idea was thrown out.
There was a problem hiding this comment.
Yes I believe you can control with debug flag.. but currently left as default. Could be reading from velero/dpa loglevel flag or other annotations.
There was a problem hiding this comment.
The default is set to debug currently.
https://github.com/kaovilai/udistribution/blob/d7f491d7c354caa1df6893d20c735b9c08c20108/pkg/client/default/default.go#L5
There was a problem hiding this comment.
Please include information about the image data being stored in the backup storage.
Today, we stand up a registry deployment that backs data up to a directory in an s3 store. It's not currently scoped under any BSL description. What is our plan with this workflow as we consume it via a library?
kaovilai
requested review from
sseago,
dymurray,
shubham-pampattiwar and
savitharaghunathan
|
In-line image will work when viewed at https://github.com/kaovilai/oadp-operator/blob/velero-plugin-registry-designdoc/docs/design/plugin-registry.md |
There was a problem hiding this comment.
+1 Great approach Thank you @kaovilai. Added some comments. Also, it would be really great if you could help me understand more and address the following:
- A high-level component diagram, +1 on the current lucid chart, but could just add a component diagram without the implementation details, or just explain the involved components/functions like Whats is registry App, Transport, serveHTTP method etc. I believe this will be really helpful in understanding the approach flow. (A brief description about each would also do without a diagram)
- Are the requests made via udistribution secure ?
- How does the image get transported ? I understand there is a working PoC, but lets say we have an app image
foothat we want to transport to s3 , how would the new approach go about it?
|
|
||
| ## Open Questions [optional] | ||
|
|
||
| Do we want registry logs to be included in velero backup and/or restore logs? |
There was a problem hiding this comment.
I would love to have the registry logs, +1 from my side.
There was a problem hiding this comment.
Additionally, lets not call them registry logs, these are technically plugin logs, right ?
There was a problem hiding this comment.
No I think there are 2 things we are talking about here.
- the plugin logs that records the process of backing up and restoring images.
- The registry library doing the actual push/pull of the images.
I think 1. should be in the backup/restore for sure. 2. feels overly verbose
There was a problem hiding this comment.
Yes. Plugin logs. However the logs generated from registry will not be in backup logs in object storage since the velero backup logger isn't being passed to the ServeHTTP method.
There was a problem hiding this comment.
We do want them but I believe that per current implementation it is not possible.
There was a problem hiding this comment.
On the positive side, this means that we can leave log level as debug for the registry component without additional configuration flags.
There was a problem hiding this comment.
Yeah let's leave it as a potential future enhancement. Kinda opens the door for some other mechanism within velero plugins to pass logs back
|
|
||
| ## Summary | ||
|
|
||
| removes a listening HTTP server and connect [openshift-velero-plugin](https://github.com/openshift/openshift-velero-plugin/) directly with the [distribution/distribution](https://github.com/distribution/distribution) code to serve requests from [container/image/copy](https://github.com/containers/image/tree/main/copy) method as needed. A "library" to help achieve this is [kaovilai/udistribution](https://github.com/kaovilai/udistribution) which may be moved to konveyor org. |
There was a problem hiding this comment.
So basically. Current workflow is,
Velero gRPC req -> Plugin server -> Plugin HTTP req to registry server -> then storage driver makes the using object storage client.
And now,
Velero gRPC req -> Plugin server -> Plugin uses udistribution lib to make direct object storage calls.
Did I understand this correctly ?
docs/design/plugin-registry.md
Outdated
|
|
||
| ### Non-Goals | ||
|
|
||
| Creating another custom resource and a controller to replace usage of [distribution/distribution](https://github.com/distribution/distribution). |
There was a problem hiding this comment.
I think this falls in the category of Alternatives considered rather than a non-goal.
| Creating another custom resource and a controller to replace usage of [distribution/distribution](https://github.com/distribution/distribution). | ||
|
|
||
| ## Proposal | ||
|
|
There was a problem hiding this comment.
More from a high level this proposal is to
Redesign the image backup/restore workflow to instead consume a library directly from within the plugin rather than standing up a transient registry deployment.
|
|
||
| ### Implementation Details/Notes/Constraints [optional] | ||
|
|
||
| First, a udistribution client are to be initialized using [`udistribution.NewTransportFromNewConfig()`](https://github.com/kaovilai/udistribution/blob/2b5e16ac1f8efa0bbcdc513ae7103d4f56f3befa/pkg/image/udistribution/docker_transport.go#L50) function. |
There was a problem hiding this comment.
Just a suggestion, a short paragraph that summarizes what udistribution is would be helpful.
There was a problem hiding this comment.
I see its more explained in line 88, but in this section stay away from in depth code references just give a high level description of the library
There was a problem hiding this comment.
Had to reread section description in https://github.com/konveyor/enhancements/blob/master/guidelines/enhancement_template.md
Will move this to design detail and summary here.
OADP-363
Implementation
openshift/openshift-velero-plugin#145
#743
linked lucidchart
Rich markdown with working in-line image can be viewed at https://github.com/kaovilai/oadp-operator/blob/velero-plugin-registry-designdoc/docs/design/plugin-registry.md
Simplified diagram of changes.
