onosproject / onos-config

Configuration subsystem for ONOS (µONOS Architecture)
45 stars 55 forks source link

if creation of a net-change fails it is not rolled back fully #657

Closed SeanCondon closed 4 years ago

SeanCondon commented 5 years ago

Describe the bug When creating a network change e.g. through a gnmi Set() operation, if there is an error then the creation of any subtending configuration changes is not rolled back on failure.

To Reproduce Make sure no configuration named strat2-1.0.0 exists on the system. Use

onos config get configs

Pass an invalid name for the network change (through Extension 100) when creating it - see in the example below the name used is illegal net change name - it is illegal because it has spaces in it.

gnmi_cli -address onos-config:5150 -set \
    -proto "update: <path: <target: 'strat2', elem: <name: 'interfaces'> elem: <name: 'interface' key:<key:'name' value:'eth1' >> elem: <name: 'config'> elem: <name: 'name'>> val: <string_val: 'eth1'>> update: <path: <target: 'strat2', elem: <name: 'interfaces'> elem: <name: 'interface' key:<key:'name' value:'eth1' >> elem: <name: 'config'> elem: <name: 'description'>> val: <string_val: 'This is a second stratum'>> extension: <registered_ext: <id: 100, msg:'illegal net change name'>> extension: <registered_ext: <id: 101, msg:'1.0.0'>> extension: <registered_ext: <id: 102, msg:'Stratum'>>" \
    -timeout 5s -alsologtostderr \
    -client_crt /etc/ssl/certs/client1.crt \
    -client_key /etc/ssl/certs/client1.key \
    -ca_crt /etc/ssl/certs/onfca.crt

During this a configuration strat2-1.0.0 is created. It is left there after the operation fails.

Expected behavior Expect the configuration strat2-1.0.0 to have been deleted in handling the error.

Logs, text or screenshots No logs

Additional context The failure should bring the system back to the state it was in before the error.

SeanCondon commented 4 years ago

This will be obsoleted/eclipsed by the new structure for Network

Andrea-Campanella commented 4 years ago

closing since it refers to pre-atomix code