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

fix: fixed delete by resync failure of init file data by using agentctl #1782

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

fgschwan
Copy link
Collaborator

When using init file for importing configuration data by VPP-Agent start and then you try to delete them by resync with empty configuration (using agentctl) then failure happens (this is not the case for using agentctl config delete). The root cause of this is different data source of init file data and data that can agentctl work with. The data source behaves for resyncs similar to linux namespaces, you can't clean data from different data source/namespace (agentctl works with 'grpc' data source data and init file data are 'datasync'-data-source data).

The fix provided by this PR enables the user to configure the init file data to have different data source (i.e.'grpc' to be compatible with agentctl resync). However, due to aggregating of data from multiple data sources (aggregator functionality), the data source will be changes also for data from other sources (etcd, redis,...). That means that this fix is only for certain scenarios when it is used only init file and no etcd, redis,... .

Proper handling of all possible combinations (init file/init file + etcd/init file + redis/...) to achieve functionality of agentctl's resync ('agentctl update --replace') is out of scope of this fix. It would probably require to do multiple resync at VPP-Agent start (multiple data inputs can have different data source labels -> for each one must be done resync) and handle all corner cases.

@milanlenco
Copy link
Collaborator

milanlenco commented Jan 28, 2021

Would it be possible to disassociate init-file from datasync (kvdb), i.e. to be able to have different data source for init file than to have for e.g. etcd?
It seems that it is possible to put data-source into the context of change/resync event:
https://github.com/ligato/vpp-agent/blob/master/plugins/orchestrator/orchestrator.go#L255-L262
https://github.com/ligato/vpp-agent/blob/master/plugins/orchestrator/orchestrator.go#L209-L216

If it would be possible to set context from initfileregistry.go then the option to customize data source of init-file could go to initfileregistry.conf.

@fgschwan
Copy link
Collaborator Author

Would it be possible to disassociate init-file from datasync (kvdb), i.e. to be able to have different data source for init file than to have for e.g. etcd?
It seems that it is possible to put data-source into the context of change/resync event:
https://github.com/ligato/vpp-agent/blob/master/plugins/orchestrator/orchestrator.go#L255-L262
https://github.com/ligato/vpp-agent/blob/master/plugins/orchestrator/orchestrator.go#L209-L216

If it would be possible to set context from initfileregistry.go then the option to customize data source of init-file could go to initfileregistry.conf.

I use local client to set the init file data into local registry. If its API had the ability to set the transaction commit context(it has NOT right now), then i could decorate it with datasource and when it comes to aggregator (i think it should pass the context as is, but didn't checked in runtime, just some code checking) then i could do some different actions for data from initfile. However, this can be done also by some hardwiring in aggregator (local client data are already hardwired with different logic). This of course doesn't solve the out of scope issue to do multiple resyncs due to multiple data sources.
If the purpose is just to move the configuration to initfile config, then this would be misleading, because the context labeling with data source happens for all data inputs of the agregator at level where are the changes merged. So it doesn't effect only the initfile data, but all data. Hence, the new config.

@milanlenco milanlenco merged commit 4088145 into ligato:master Feb 3, 2021
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.

2 participants