-
Notifications
You must be signed in to change notification settings - Fork 34
Add schema definition of creating pluggable data store #1750
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
base: v2
Are you sure you want to change the base?
Conversation
| - name: dataStoreVersion | ||
| uiType: Toggle | ||
| description: Data store version | ||
| - name: nodes |
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.
@chilagrow @recharte — could you help me understand this?
I see the customResource field, and its description suggests that this may not represent the entire schema, but rather only the part related to resources. However, in the uiComponents array I see UI components such as name, namespace, and dataStoreVersion. This implies that the schema is not currently split into logical parts and then mapped to steps in the UI, and I am not sure where the correct fix should be applied.
@chilagrow, to display steps in a wizard, we need to split the schema into sections corresponding to those steps—for example, resources, backups, and monitoring. If all components are listed sequentially in a single array, they will all be rendered on one page.
Should we remove provider, name, namespace, and dataStoreVersion from this array and place them elsewhere at a higher level, or should we introduce an additional level of hierarchy under uiComponents?
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.
Should we separate the choosing of dbEngine(provider)? To avoid of problems during creation with default and already filled in parameters? In that case should we have it in schema at all? Or in some separate "global" section as it was done during this POC?
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.
and answering your question, I think you missed small/medium/large options. It's just an addition to the UI that allows to set pre-prepared values for cpu/memory/disk size. But... it means user can add some "function" to schema, that tells how other fields should work with default values... we need to think how we can describe this on the UI from the plugin developer's side, and do we really need 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.
Should we remove provider, name, namespace, and dataStoreVersion from this array and place them elsewhere at a higher level, or should we introduce an additional level of hierarchy under uiComponents?
I'm not sure we need any of these in the UI schema at all because these are fields that will always exist as part of any provider. I don't think that the UI schema needs to have absolutely everything that's in a DataStore CR, there are some fields that are mandatory and can never change, thus their component types can be hardcoded in the FE code.
Should we separate the choosing of dbEngine(provider)? To avoid of problems during creation with default and already filled in parameters? In that case should we have it in schema at all? Or in some separate "global" section as it was done during this POC?
Yeah, I think we should separate it. And yes, I don't think it should be in the schema.
|
@recharte could you take a look please 🙏 |
recharte
left a comment
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.
Sorry @chilagrow I forgot about this PR 🙈
docs/spec/openapi.yml
Outdated
| type: object | ||
| required: | ||
| - provider | ||
| - topology |
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.
I'm not sure topology should be mandatory.
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.
Perhaps not 😓
docs/spec/openapi.yml
Outdated
| components: | ||
| type: object | ||
| properties: | ||
| engine: | ||
| description: engine component configuration | ||
| type: object | ||
| required: | ||
| - type | ||
| properties: | ||
| type: | ||
| description: Type of the engine | ||
| type: string | ||
| default: pxc | ||
| version: | ||
| description: Version of the engine | ||
| type: string | ||
| replicas: | ||
| description: Number of replicas | ||
| type: number | ||
| resources: | ||
| description: Resources configuration | ||
| type: object | ||
| properties: | ||
| cpu: | ||
| description: Number of CPU cores (e.g., 1) | ||
| type: string | ||
| memory: | ||
| description: Size of the memory (e.g., 2G) | ||
| type: string | ||
| storage: | ||
| description: Storage configuration | ||
| type: object | ||
| properties: | ||
| class: | ||
| description: Storage class name | ||
| type: string | ||
| size: | ||
| description: Size of the storage (e.g., 5Gi) | ||
| type: string | ||
| podSchedulingPolicyName: | ||
| description: Pod scheduling policy name | ||
| type: string | ||
| default: "everest-default-mysql" | ||
| proxy: | ||
| description: Proxy component configuration | ||
| type: object | ||
| properties: | ||
| type: | ||
| description: Type of the proxy | ||
| type: string | ||
| replicas: | ||
| description: Number of replicas | ||
| type: number | ||
| resources: | ||
| description: Resources configuration | ||
| type: object | ||
| properties: | ||
| cpu: | ||
| description: Number of CPU cores (e.g., 0.6) | ||
| type: string | ||
| memory: | ||
| description: Size of the memory (e.g., 0.6G) | ||
| type: string | ||
| exposes: | ||
| description: Resources configuration | ||
| type: object | ||
| properties: | ||
| type: | ||
| description: Type of the expose | ||
| type: string | ||
| loadBalancerConfigName: | ||
| description: Load balancer configuration name | ||
| type: string | ||
| monitoring: | ||
| description: Monitoring component configuration | ||
| type: object | ||
| properties: | ||
| type: | ||
| description: Type of the monitoring component | ||
| type: string | ||
| version: | ||
| description: Version of the monitoring component | ||
| type: string |
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 can't have the components defined in this API schema because each provider will have a different set of components. But to be honest I'm not sure how to actually present the schema for this 🤔
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.
Yeah, DataStoreCRSchema is just a blob, it has no schema, all above is under example of what a pxc provider may present. I am trying to show case how it may work with UI. I thought OpenAPI like schema definition might be one way. Let me think if there is other way of presenting what each CR exposes. 🤔
DataStoreCRSchema:
description: DataStoreCRSchema defines the the custom resource of the data store
type: object
example:
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.
Actually I would prefer an approach where CR schema can be generated from each provider.
Also do you think UI schema can be generated or do we see it as a manual task of adding a plugin?
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.
Also do you think UI schema can be generated or do we see it as a manual task of adding a plugin?
I would heavily push for auto-generation because it's a huge pain to do this manually, not to mention error-prone. But we need a mechanism that allows for extensive customization.
Something inspired by kubebuilder tags (e.g. // +kubebuilder:validation:MaxLength=15) might be the way to go.
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.
I agree, perhaps this is something to look into for me. It would be great amount of work to add even a few componets.
| value: "2.0.0" | ||
| - name: nodes | ||
| uiType: Group | ||
| description: Nodes configuration |
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.
The description, name, and label keys have different meanings in the UI, and their styles depend on where they are used.
In the case of node configuration, description means that this text will be rendered in the UI. Based on the screenshot, I don’t see any description with the text “Nodes configuration”, which is why I’m highlighting this. If we follow the spec as-is, an extra description text will appear in the UI.
For field descriptions we use the body2 style, which is different. The same logic applies to label: when the UI encounters a label key, it knows that a label element should be rendered (example of label - below).
So for next several lines we will see the different result from what we have on your picture. There will be plain text (not bold) with "Number of nodes"
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.
Thank for identifying issues 🤗. The screenshot is the current UI on main branch and I would like to describe in UI schema, hence the question. Yeah current example doesn't work. From the spec https://github.com/openeverest/specs/blob/main/specs/001-plugins-architecture.md#ui-form-generator-yaml-schema-examples, what does uiType: 'Group' mean?
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.
The group was meant to identify sections within the wizard. These sections could be collapsible for example.
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.
Thanks, I am trying to identify what part of spec is Material UI and must follow the spec. There must be mapping between UI component and the CR. Any thoughts how best it can be done @solovevayaroslavna? 🤔
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.
@recharte correct me if i'm wrong, but i thought that group means group of anything. Like: group of fields, group of groups, etc. Because section, probably not the single case where we can need to group something. So... we can start with this approach, but during our work we can faced with the need to add some additional field like: group type to specify what it is - a section, a separate page, a dropdown with fields or something else. Maybe i'm wrong but it seems to me that just "group" is not enought to cover all ui situations
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.
@chilagrow To be honest, not yet, I think the best way is to prepare UI examples along with the specification so that the reader of the specification can understand what a section is, what a label is, a group, etc. Perhaps a storybook with our components will help us with this. It would be ideal if we could integrate storybook or links to it into the documentation.
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 to chime in from #1750 (comment). For plugin developers, being able to generate basic UI schema would ease adding new data store. So UI example or spec would also be valuable for identifying what should be generated.
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.
@recharte correct me if i'm wrong, but i thought that group means group of anything. Like: group of fields, group of groups, etc. Because section, probably not the single case where we can need to group something. So... we can start with this approach, but during our work we can faced with the need to add some additional field like: group type to specify what it is - a section, a separate page, a dropdown with fields or something else. Maybe i'm wrong but it seems to me that just "group" is not enought to cover all ui situations
Yeah, I agree that the groups construct can be for more than sections. If we can have that flexibility of choosing the type of group, that's even better.
| uiDescription: Number of nodes | ||
| mapping: spec.engine.replicas | ||
| params: | ||
| badge: node |
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.
Also! One important mention. If we want to see the same view as on the picture, we shouldn't have uiType: Number because number field looks like this one:
I'm not sure is this expected or not, but if we want to see this as a number of buttons, we probably need to have a separate uiType like: "number of nodes" or "numberInoutButtonGroup" or something like that to allow user to use it the same way as on the screenshot
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.
Yeah agree uiType: Number doesn't look like the fit.
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.
@recharte yeah! you are absolutely right! We use badge for these purposes.
This brings up another question. If a plugin developer uses a badge, should we also expose this string in the API? At the moment, the UI shows a “GB” badge, but the API request contains only “G”. But for CPU we have other logic
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.
I see several possible approaches to address this.
-
We could add a configuration option for plugin developers that determines whether the badge value should be included in the API. In this case, the UI and API values would be identical.
-
Alternatively, we could provide predefined components for CPU, memory, and similar metrics to simplify adoption. However, I would still prefer the first option, as it offers greater flexibility in configuration.
| $ref: '#/components/schemas/Secret' | ||
|
|
||
|
|
||
| '/cluster/{cluster}/namespaces/{namespace}/data-store/{dataStore}/schema': |
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.
I don't think this is the correct path.
The schema is unique per provider and its purpose is to expose the information that clients (mostly the FE) need to understand to know how to build a DataStore CR for that given provider. Knowing the schema definition is a prerequisite to creating a DataStore. As such, it doesn't make sense to have this under /data-store/{dataStore}.
The way I envision the flow from a client POV is:
- Which providers are installed in a given cluster? GET
/clusters/{cluster}/providers - How can I create a DataStore CR for a given provider? GET
/clusters/{cluster]/providers/{provider}/schema - Now that I know that
ProviderA is installed and I understand the schema for that provider I can create a DataStore CR for it. POST/clusters/{cluster}/namespaces/{namespace}/data-store`
Does this make sense?
| '/cluster/{cluster}/namespaces/{namespace}/data-store/{dataStore}/schema': | |
| '/clusters/{cluster}/providers/{provider}/schema': |
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 we want to be less generic and since the schema actually refers to the dataStore maybe the following would also work:
| '/cluster/{cluster}/namespaces/{namespace}/data-store/{dataStore}/schema': | |
| '/clusters/{cluster}/providers/{provider}/data-store-schema': |
This is probably better because we might need to add some more schemas in the future and this way we are more future proof.
WDYT?
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.
Thanks, yes that's right about the path!
As I understand, backup and restore will also be specific to provider CR. For example I would suppose it possible for a data store to not implement PITR, could that be possible?
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.
As I understand, backup and restore will also be specific to provider CR.
Well, yes, but a given backup or restores object will always be coupled to a given data store. The we currently have in v1 is that you create backups by POST /namespaces/{namespace}/database-cluster-backups but then you can list the backups of a given database by GET /namespaces/{namespace}/database-clusters/{cluster-name}/backups. So it's kind of a hybrid, we can rethink this API design if needed.
| label: Memory | ||
| type: object | ||
| DataStoreUISchema: | ||
| description: DataStoreUISchema defines a single property in the data store schema |
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.
Don't understand while "single" property.Do you mean one of many?
| $ref: '#/components/schemas/DataStoreUISchema' | ||
| example: | ||
| - name: provider | ||
| uiType: Toggle |
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.
| uiType: Toggle | |
| uiType: Select |
| uiType: Toggle | ||
| description: Data store provider | ||
| mapping: spec.engine.type | ||
| disable-edit: ['all'] |
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.
For me, it’s not obvious that “Edit” only works after submission, so i suppose this one, because it means i can edit during working with this part of form (as example). Maybe i'm wrong
| disable-edit: ['all'] | |
| disable: ["update'', "restore"] |


Refs solanicaio/engineering#15.
This PR adds an endpoint to get definition of providers' CR schema and UI components. Minimal changes were made in code so that generation of openapi doesn't fail.