Skip to content
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

feat: use etcd to store the JSON schema of APISIX, then the manager API can read it from etcd #2700

Closed
membphis opened this issue Nov 11, 2020 · 20 comments
Assignees
Labels
discuss enhancement New feature or request
Milestone

Comments

@membphis
Copy link
Member

membphis commented Nov 11, 2020

here is the discuss mail:

https://lists.apache.org/thread.html/rb1c19fe9b6e51ede3b5aead122e2bcd4de6249f42b971eabfb8a79ac%40%3Cdev.apisix.apache.org%3E

@membphis membphis added the enhancement New feature or request label Nov 11, 2020
@membphis
Copy link
Member Author

@nic-chen do you want to handle this job?

@nic-chen
Copy link
Member

OK, let me try.

@nic-chen
Copy link
Member

I think the main logic should be:

When APISIX starts, read the key /apisix/jsonschema of ETCD.

If the key does not exist or is empty, then read the JSON schema from APISIX and save to the ETCD key /apisix/jsonschema.

If the key already exists and is not empty, nothing will be done.

APISIX provides a command line tool that can be used to manually update the JSON schema, like: apisix sync-schema,
users can use this tool to update JSON schema when needed.

What do you think ?

cc @membphis @moonming @liuxiran @dabue @tokers @ShiningRush

@tokers
Copy link
Contributor

tokers commented Nov 12, 2020

I think the main logic should be:

When APISIX starts, read the key /apisix/jsonschema of ETCD.

If the key does not exist or is empty, then read the JSON schema from APISIX and save to the ETCD key /apisix/jsonschema.

If the key already exists and is not empty, nothing will be done.

APISIX provides a command line tool that can be used to manually update the JSON schema, like: apisix sync-schema,
users can use this tool to update JSON schema when needed.

What do you think ?

cc @membphis @moonming @liuxiran @dabue @tokers @ShiningRush

So all schemas are put inside a single JSON string here?

BTW, have you consider the grayscale of APISIX itself? If the new version of APISIX has some incompatible changes for some schemas, once the schema is synced, all instances which shares the same ETCD cluster will be effected. So maybe versioning or other way like naming isolation is required.

@liuxiran
Copy link
Contributor

liuxiran commented Nov 12, 2020

Is it necessary to automatically trigger schema synchronization when adding a new schema(e.g: add a new plugin) or modifying a schema, so that the consumer of the schema (e.g manager-api) can use the feature in a non-perceptible way?

@nic-chen
Copy link
Member

I think the main logic should be:
When APISIX starts, read the key /apisix/jsonschema of ETCD.
If the key does not exist or is empty, then read the JSON schema from APISIX and save to the ETCD key /apisix/jsonschema.
If the key already exists and is not empty, nothing will be done.
APISIX provides a command line tool that can be used to manually update the JSON schema, like: apisix sync-schema,
users can use this tool to update JSON schema when needed.
What do you think ?
cc @membphis @moonming @liuxiran @dabue @tokers @ShiningRush

So all schemas are put inside a single JSON string here?

BTW, have you consider the grayscale of APISIX itself? If the new version of APISIX has some incompatible changes for some schemas, once the schema is synced, all instances which shares the same ETCD cluster will be effected. So maybe versioning or other way like naming isolation is required.

Thanks for suggestion, that's really helpful.

So we could use key like /apisix/jsonschema/$schemaversion for isolation.

And I think we need to manually maintain the schema version in both APISIX and manager api.

what do you think ?

@tokers
Copy link
Contributor

tokers commented Nov 12, 2020

I think the main logic should be:
When APISIX starts, read the key /apisix/jsonschema of ETCD.
If the key does not exist or is empty, then read the JSON schema from APISIX and save to the ETCD key /apisix/jsonschema.
If the key already exists and is not empty, nothing will be done.
APISIX provides a command line tool that can be used to manually update the JSON schema, like: apisix sync-schema,
users can use this tool to update JSON schema when needed.
What do you think ?
cc @membphis @moonming @liuxiran @dabue @tokers @ShiningRush

So all schemas are put inside a single JSON string here?
BTW, have you consider the grayscale of APISIX itself? If the new version of APISIX has some incompatible changes for some schemas, once the schema is synced, all instances which shares the same ETCD cluster will be effected. So maybe versioning or other way like naming isolation is required.

Thanks for suggestion, that's really helpful.

So we could use key like /apisix/jsonschema/$schemaversion for isolation.

And I think we need to manually maintain the schema version in both APISIX and manager api.

what do you think ?

What about using a separate key for each schema? just use /apisix/jsonschema/ as the prefix, like other resources?

/apisix/jsonschema/route
/apisix/jsonschema/upstream
/apisix/jsonschema/plugin/echo

I am not sure whether the versioning is a good way to solve this, because APISIX should remember each schema's version that it needs.

@nic-chen
Copy link
Member

Is it necessary to automatically trigger schema synchronization when adding a new schema(e.g: add a new plugin) or modifying a schema, so that the consumer of the schema (e.g manager-api) can use the feature in a non-perceptible way?

Thanks for suggestion. I have considered this before, but this will cause version problems, so I think it needs to be automatically generated if it does not exist, and the update should be triggered manually by the user. After solved the version issue, it could be automatically updated.

@nic-chen
Copy link
Member

I think the main logic should be:
When APISIX starts, read the key /apisix/jsonschema of ETCD.
If the key does not exist or is empty, then read the JSON schema from APISIX and save to the ETCD key /apisix/jsonschema.
If the key already exists and is not empty, nothing will be done.
APISIX provides a command line tool that can be used to manually update the JSON schema, like: apisix sync-schema,
users can use this tool to update JSON schema when needed.
What do you think ?
cc @membphis @moonming @liuxiran @dabue @tokers @ShiningRush

So all schemas are put inside a single JSON string here?
BTW, have you consider the grayscale of APISIX itself? If the new version of APISIX has some incompatible changes for some schemas, once the schema is synced, all instances which shares the same ETCD cluster will be effected. So maybe versioning or other way like naming isolation is required.

Thanks for suggestion, that's really helpful.
So we could use key like /apisix/jsonschema/$schemaversion for isolation.
And I think we need to manually maintain the schema version in both APISIX and manager api.
what do you think ?

What about using a separate key for each schema? just use /apisix/jsonschema/ as the prefix, like other resources?

/apisix/jsonschema/route
/apisix/jsonschema/upstream
/apisix/jsonschema/plugin/echo

I am not sure whether the versioning is a good way to solve this, because APISIX should remember each schema's version that it needs.

I think separating them will increase the complexity, especially the version issue.

@membphis
Copy link
Member Author

JSON Schema version is a useful field, we need to store and compare it.

Needs to show a warning message if the JSON schema version is different from APISIX version.

@ShiningRush
Copy link
Contributor

Here is no need create version for schema, because the schema is build from APISIX, so schema version should same as APISIX.
I think we can do like that:

  • APISIX will automatically write schema and APISIX-VERSION to /apisix/jsonschema at start
  • Dashboard maintain a white/black version list in config and watch the schema changes, if the APISIX version is incompatible we can show message to user such as the current dashboard version is incompatible with APISIX(x.x.x), please update dashboard or APISIX

Automation is very important to a product, we should not let users decide when to synchronize and when not to synchronize schema when it's effect scope is controllable( schema mismatch is will just effect dashboard not data plane), it will burden users.

@tokers
Copy link
Contributor

tokers commented Nov 14, 2020

Here is no need create version for schema, because the schema is build from APISIX, so schema version should same as APISIX.
I think we can do like that:

  • APISIX will automatically write schema and APISIX-VERSION to /apisix/jsonschema at start
  • Dashboard maintain a white/black version list in config and watch the schema changes, if the APISIX version is incompatible we can show message to user such as the current dashboard version is incompatible with APISIX(x.x.x), please update dashboard or APISIX

Automation is very important to a product, we should not let users decide when to synchronize and when not to synchronize schema when it's effect scope is controllable( schema mismatch is will just effect dashboard not data plane), it will burden users.

The first step can be done in the apisix init step.

@membphis
Copy link
Member Author

The first step can be done in the apisix init step.

that is not a good idea, it is unsafe

@tokers
Copy link
Contributor

tokers commented Nov 18, 2020

If we want to make sure the data plane doesn't modify etcd, another control plane component should be introduced to take over the uploading of jsonschema from APISIX, status collection or even the configuration delivery.

@membphis
Copy link
Member Author

another control plane component should be introduced to take over the uploading of jsonschema from APISIX,

yes, that is the right way. How about this? apisix-admin sync-schema

@tokers
Copy link
Contributor

tokers commented Nov 19, 2020

another control plane component should be introduced to take over the uploading of jsonschema from APISIX,

yes, that is the right way. How about this? apisix-admin sync-schema

That's good. But the jsonschema source still in APIS
IX, we have to solve another problem that how apisix-admin gets the jsonschema, maybe apisix-admin could be written based on ngx_lua and share the code base of APISIX.

@nic-chen
Copy link
Member

another control plane component should be introduced to take over the uploading of jsonschema from APISIX,

yes, that is the right way. How about this? apisix-admin sync-schema

That's good. But the jsonschema source still in APIS
IX, we have to solve another problem that how apisix-admin gets the jsonschema, maybe apisix-admin could be written based on ngx_lua and share the code base of APISIX.

then we go back to the old issue again..

@ShiningRush
Copy link
Contributor

ShiningRush commented Nov 19, 2020

that is not a good idea, it is unsafe

@membphis Why it is unsafe, I can think of the worst result that is not compatible with manager-api, but this does not affect the normal works of the data plane, does it?
Because the synchronization schema is only a system bypass

@tokers
Copy link
Contributor

tokers commented Nov 19, 2020

another control plane component should be introduced to take over the uploading of jsonschema from APISIX,

yes, that is the right way. How about this? apisix-admin sync-schema

That's good. But the jsonschema source still in APIS

IX, we have to solve another problem that how apisix-admin gets the jsonschema, maybe apisix-admin could be written based on ngx_lua and share the code base of APISIX.

then we go back to the old issue again..

We have to if we wanna the data plane reads etcd only. 😂

@membphis
Copy link
Member Author

I gona to close this issue, we have discussed at maillist: https://lists.apache.org/thread.html/rb1c19fe9b6e51ede3b5aead122e2bcd4de6249f42b971eabfb8a79ac%40%3Cdev.apisix.apache.org%3E

I'll create a new issue with the latest result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants