[acl] Add regression test for config acl CLI command

[daall]

Add a new test case to catch the bug where calling "config acl add table" multiple
times causes the "ports" field to be deleted from config DB.

Signed-off-by: Danny Allen [email protected]

What I did/why I did it
I added a test case to guard against regressions like the one described in sonic-net/sonic-utilities#1519.

How I verified it
Ran the test case w/ and w/o the change from 1519.

[lguohan]

since 1519 is not merge, isn't this new test supposed to be fail?

[daall]

since 1519 is not merge, isn't this new test supposed to be fail?

I am trying to figure out if the underlying issue has been resolved in the master branch or if there is some weird discrepancy between the physical device and the virtual switch. Will update shortly.

[daall]

since 1519 is not merge, isn't this new test supposed to be fail?

I am trying to figure out if the underlying issue has been resolved in the master branch or if there is some weird discrepancy between the physical device and the virtual switch. Will update shortly.

The underlying issue seems to have been fixed in the master branch, so let me track down the change and see if this is a bugfix we can also grab for 201911.

[daall]

since 1519 is not merge, isn't this new test supposed to be fail?

I am trying to figure out if the underlying issue has been resolved in the master branch or if there is some weird discrepancy between the physical device and the virtual switch. Will update shortly.

The underlying issue seems to have been fixed in the master branch, so let me track down the change and see if this is a bugfix we can also grab for 201911.

This is the reason for the difference: sonic-net/sonic-utilities#1392 It seems like the bug exists in swsssdk but not swss-common. I think we should still merge this test case just to prevent any similar regressions in the future. @lguohan do you agree?

[qiluo-msft]

This is interesting. How confident are you that sonic-net/sonic-utilities#1392 fixed the bug? It is expected as refactoring. If you know the detail, could you explain your API call sequence into swss-common or swsssdk? I can help check the sequence to make sure it is fixed.

[daall]

This is interesting. How confident are you that Azure/sonic-utilities#1392 fixed the bug? It is expected as refactoring. If you know the detail, could you explain your API call sequence into swss-common or swsssdk? I can help check the sequence to make sure it is fixed.

They're both calling set_entry from ConfigDBConnector. When ConfigDBConnector is imported from swsscommon, everything works as expected and we can call set_entry with ports@ without issue. When ConfigDBConnector is imported from swsssdk, then the first call to set_entry works as expected, but the second call deletes the ports@ field from config DB.

---

In reply to: 820805641

[lguohan]

test looks good to me, please proceed to merge.

[qiluo-msft]

I analyze this flow, and everything you observed is expected. In old swsssdk, set_entry expected array field in data must have no trailing @, and array value must be an array. If you gave the other way, it unfortunately remove the field at 2nd call.
However, swsscommon could handle both styles, so it's more tolerable.

---

In reply to: 820805641