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: added REST API for getting and setting NB VPP-Agent configuration #1787

Merged
merged 6 commits into from
Mar 10, 2021

Conversation

fgschwan
Copy link
Collaborator

@fgschwan fgschwan commented Feb 19, 2021

Added ability to change NB VPP-Agent configuration by REST API.

  • Use GET REST action on <VPP-Agent IP address>:9191/configuration to retrieve NB VPP-Agent configuration in YAML format.
  • Use PUT REST action on <VPP-Agent IP address>:9191/configuration with configuration to update in YAML format in http body to update NB VPP-Agent configuration.
  • Also resync is possible by using PUT REST action on <VPP-Agent IP address>:9191/configuration?replace=true (<VPP-Agent IP address>:9191/configuration?replace will do the same).

The structure of YAML in all cases is the same as for agentctl config ... commands. Basically, it can be viewed as REST API equivalent to agentctl config get, agentctl config update, agentctl config update --replace. However, the REST API output is always YAML and can't be formatted to alternative formats as in case of agentctl tool.

There are some code dependencies to not yet merged PR #1786, therefore this PR is based on the commits of that PR. That means that commits of both PRs are visible here. (rebased on master)

@fgschwan fgschwan changed the title WIP feat: added REST API for getting and setting NB VPP-Agent configuration feat: added REST API for getting and setting NB VPP-Agent configuration Feb 22, 2021
@fgschwan fgschwan force-pushed the get-set-yaml-rest branch from b67b19c to fcb0d2f Compare March 1, 2021 16:34
@chrismetz09
Copy link

Confirming that Ligato REST docs will need update. I'll open PR once this is committed.

fgschwan added 5 commits March 3, 2021 14:16
…related utils (bad type assumption that luckily worked for dynamic message but not for statically-genereated messages)

Signed-off-by: Filip Gschwandtner <[email protected]>
…ge into conversion function (dynamic to staticly-generated message) that needs it (this simplifies function usage and related nameFunc API)

Signed-off-by: Filip Gschwandtner <[email protected]>
@fgschwan fgschwan force-pushed the get-set-yaml-rest branch from fcb0d2f to 29aa4e6 Compare March 3, 2021 13:19
@chrismetz09
Copy link

Confirming this PR requires REST docs/OpenAPI updates?

@fgschwan
Copy link
Collaborator Author

fgschwan commented Mar 8, 2021

Confirming this PR requires REST docs/OpenAPI updates?

@chrismetz09 Hi, this PR adds new capabilities to REST API. I'm not aware that i missed to update some documentation somewhere. If this is the case, let me know where so i can fix it. Unless of course you want to do the documentation update (as stated in your other comment). However I would like to know which documentation i missed, just for the future to not repeat this mistake again.

@chrismetz09
Copy link

Hey @fgschwan , Filip,I have been maintaining Ligato Docs at docs.ligato.io. https://docs.ligato.io/en/latest/api/api-vpp-agent/ describes VPP agent REST APIs. To date, REST is GET configs only. Now PR appears to support PUT configs? So this section, elsewhere in plugins section, and separate OpenAPI definitions need updating. Docs format must be consistent. I can hack the PR, then you can review?

@fgschwan
Copy link
Collaborator Author

fgschwan commented Mar 9, 2021

Hey @fgschwan , Filip,I have been maintaining Ligato Docs at docs.ligato.io. docs.ligato.io/en/latest/api/api-vpp-agent describes VPP agent REST APIs. To date, REST is GET configs only. Now PR appears to support PUT configs? So this section, elsewhere in plugins section, and separate OpenAPI definitions need updating. Docs format must be consistent. I can hack the PR, then you can review?

@chrismetz09
Just to clarify, the GET REST APIs with "dump" in URI that you describe in docs get actual VPP configuration by calling binary API/CLI API directly on VPP (in time of REST API call). This is southbound (SB)(if you are familiar with this kind of terminology). I made REST API for getting and setting the northbound(NB) configuration. That is the configuration that user of VPP-Agent wants to have configured inside VPP. When every config synchronization went ok (transaction are successful and there are no unresolved dependencies,i.e. missing interface with given name), the NB and SB are the same. However if something went wrong and all configuration can't be applied for whatever reason, the NB and SB configuration are different. That means the my new GET in REST API (NB config get) can get different configuration than existing "dump" GET REST API (SB config get).

Also i think that my last previous PR is missing in REST API docs also.

Feel free to hack this PR with docs update. I will gladly review it.

PS: If you need some examples for NB configuration GET/PUT i can provide some (It is the yaml used for agentctl config ... commands)

@chrismetz09
Copy link

Thanks @fgschwan, okay understand exactly what are saying. Send me one example each of GET and PUT and I will start PR. Assuming I can run new APIs using quickstart config? Will also cover your comments in simple diagram that would be helpful.

Let's move discussion to: ligato/docs#97

for _, configMessage := range configMessages {
// convert config message from dynamic to statically-generated proto message
// (this is needed for later processing of message - generated KVDescriptor adapters cast
// to statically-generated proto message and fail with dynamicpb.Message proto messages)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that these REST APIs will not work for remotely known models?

Note: In StoneWork items of remotely known models are handled by "proxy" descriptors that do not cast proto messages to statically-generated types, instead just forward them via gRPC to the destination CNF (which knows those models locally). It means that only marshalling/unmarshalling is done with remotely known items, no type casting. So in this case it could be allowed to pass dynamic message to KvScheduler transaction.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ff22b3e

… in PUT NB configuration in REST API

Signed-off-by: Filip Gschwandtner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants