-
Notifications
You must be signed in to change notification settings - Fork 79
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
enhancement: extend the p2p preheat policy #252
Open
chlins
wants to merge
1
commit into
goharbor:main
Choose a base branch
from
chlins:enhancement/p2p-preheat-attributes
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
# Proposal: P2P Preheat Enhancement | ||
|
||
Author: [Chenyu Zhang](https://github.com/chlins) | ||
|
||
## Abstract | ||
|
||
This proposal aims to expand the p2p preheat policy to accommodate a wider range of configurations and scenarios. | ||
To achieve this, two options will be added to the policy: preheat scope and extra attributes. | ||
|
||
## Motivation | ||
|
||
The current Harbor p2p preheat policy offers basic options like filters and triggers. However, these are tailored | ||
from Harbor's perspective, omitting the p2p or preheat viewpoints, particularly in today's AI-driven scenarios. | ||
Users might need to preheat images to p2p clusters with additional p2p related configurations or parameters. | ||
This proposal aims to broaden the p2p preheat policy to accommodate more flexible configurations and scenarios. | ||
|
||
## Solution | ||
|
||
Add the following two options to the p2p preheat policy: | ||
|
||
- scope: The scope of the preheat, which can be set to "single_peer" or "all_peers". The default value is "single_peer". | ||
- extra_attrs: The additional attributes that can be used to configure the preheat for vendor or provider specified arguments. | ||
This is a key-value pair as JSON format, and it's optional. | ||
|
||
## Goal | ||
|
||
1. Some common and abstractable parameters that are not strongly bound to P2P providers can be abstracted as a field in a policy. | ||
2. Also provides an extended parameter for users to specify provider-related or unique configurations. | ||
|
||
## Personas and User Stories | ||
|
||
This section lists the user stories regarding this enhancements for the different personas interacting with the p2p preheat. | ||
|
||
* Personas | ||
|
||
P2P Preheat is a System Administrator and Project Administrator operation in Harbor. | ||
|
||
* User Stories | ||
|
||
1. As a system/project administrator, I can create/update a p2p preheat policy with the scope set to "single_peer" or "all_peers". | ||
2. As a system/project administrator, I can create/update a p2p preheat policy with extra attributes to configure the preheat for vendor or provider specified arguments. | ||
(e.g: I can set the cluster ids for dragonfly provider) | ||
|
||
* Scenario Cases | ||
|
||
1. If the user using the dragonfly as p2p provider, when he sets the scope to "single_peer", the preheat will only preheats the image to one peer in the cluster. | ||
2. If the user using the dragonfly as p2p provider, when he sets the scope to "all_peers", the preheat will preheats the image to all peers in the cluster. | ||
3. If the user using the dragonfly as p2p provider, when he sets the extra_attrs to {"cluster_ids": [1, 2, 3]}, the preheat will only preheats the image to the specified clusters. | ||
|
||
## Scheme Changes | ||
|
||
The schema changes are as follows: | ||
|
||
```sql | ||
ALTER TABLE p2p_preheat_policy ADD COLUMN IF NOT EXISTS scope varchar(255); | ||
ALTER TABLE p2p_preheat_policy ADD COLUMN IF NOT EXISTS extra_attrs json; | ||
``` | ||
|
||
## UI | ||
|
||
The UI changes are as follows: | ||
|
||
1. Add a new field named "Scope" to the p2p preheat policy page, which can be set to "single_peer" or "all_peers". | ||
2. Add a new field named "Extra Attributes" to the p2p preheat policy page, which is a key-value pair as JSON format. | ||
|
||
![p2p preheat policy](../images/p2p/p2p-preheat-policy-extra-attrs.jpg) | ||
|
||
## API | ||
|
||
No any breaking change for preheat policy, and not introduce the new APIs, just add the new fields to payload for existing APIs. | ||
|
||
Payload example: | ||
|
||
```json | ||
{ | ||
"creation_time": "2024-10-30T07:11:36.349Z", | ||
"enabled": true, | ||
"filters": "[{\"type\":\"repository\",\"value\":\"**\"},{\"type\":\"tag\",\"value\":\"**\"}]", | ||
"id": 4, | ||
"name": "test", | ||
"project_id": 1, | ||
"provider_id": 2, | ||
"provider_name": "d7y", | ||
"scope": "single_peer", | ||
"trigger": "{\"type\":\"manual\",\"trigger_setting\":{\"cron\":\"\"}}", | ||
"extra_attrs": "{\"cluster_ids\":[1,2,3]}", | ||
"update_time": "2024-10-30T07:46:14.497Z" | ||
} | ||
``` |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If the extra_attrs are provider specific, how will Harbor send the extra_attrs?
IMO this has to be answered to make sure other p2p providers can consume the new attributes.
Additionally, why not make the "scope" also part of extra_attrs, if at this moment only dragonfly can consume it?
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.
In the harbor codebase, each p2p provider will have a driver implementation capable of parsing or handling
extra_attrs
independently to determine how to transmit them. We emphasize the scope separately fromextra_attrs
because it's a universally applicable, vendor-neutral property in p2p scenarios. Placing it as a standalone attribute is more user-friendly than including it withinextra_attrs
, as the latter is intended for vendor-specific attributes. We consider scope a standard p2p option.Currently, I think we do not have much choice for image p2p solution, harbor integrates dragonfly and kraken, and it seems kraken project is no longer in an active maintenance state, as it's last release version was still published four years ago.
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 it already in the code base to handle extra_attrs? I don't see how the
Driver
interface handles the extra_attrs or even the policy. If there are any changes need to made in the driver side they should also be reflected in this design.I agree "scope" can be a common concept for P2P scenario, but different vendors may parse the param differently, for example, a vendor may support distributing the artifacts to certain regions or nodes with certain labels. Therefore, I wish to suggest we leave it in the "extra_attrs" to provide more flexibilities for providers.
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 don't need to change the
Driver
interface, we only need to add the field in thePreheat
function parameters, then the driver self can get it and process it by their own logic. I can supplement this field into the design.I agree with what you said that different vendors may have different requirements, such as region or label, etc., so these custom items for vendors can all be placed in extra. However, I still believe that scope should be a very general and clear concept that can be shared or recognized by all p2p solutions, so the separate abstraction can bring a better user experience. Because although extra can contain anything, from my personal perspective, the user experience of extra is not very friendly, and its concept is quite broad.