From 0156c21eff95a8feab7e0af0bf0dcf070d8d368b Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Tue, 24 May 2022 18:42:54 +0800 Subject: [PATCH] [macsec-cli]: Fixing to config MACsec on the port will clear port attributes in config db (#10903) Why I did it There is a bug that the Port attributes in CONFIG_DB will be cleared if using sudo config macsec port add Ethernet0 or sudo config macsec port del Ethernet0 How I did it To fetch the port attributes before set/remove MACsec field in port table. Signed-off-by: Ze Gan --- .../docker-macsec/cli-plugin-tests/conftest.py | 1 + .../cli-plugin-tests/test_config_macsec.py | 4 +++- .../docker-macsec/cli/config/plugins/macsec.py | 16 ++++++++++++++-- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/dockers/docker-macsec/cli-plugin-tests/conftest.py b/dockers/docker-macsec/cli-plugin-tests/conftest.py index 56dcc3f302..e6608ce712 100644 --- a/dockers/docker-macsec/cli-plugin-tests/conftest.py +++ b/dockers/docker-macsec/cli-plugin-tests/conftest.py @@ -9,6 +9,7 @@ def mock_cfgdb(): CONFIG = { 'PORT': { 'Ethernet0': { + "admin_status": "up" } } } diff --git a/dockers/docker-macsec/cli-plugin-tests/test_config_macsec.py b/dockers/docker-macsec/cli-plugin-tests/test_config_macsec.py index 4635092386..425ca2afa4 100644 --- a/dockers/docker-macsec/cli-plugin-tests/test_config_macsec.py +++ b/dockers/docker-macsec/cli-plugin-tests/test_config_macsec.py @@ -116,6 +116,7 @@ class TestConfigMACsec(object): port_table = db.cfgdb.get_entry("PORT", "Ethernet0") assert port_table assert port_table["macsec"] == "test" + assert port_table["admin_status"] == "up" result = runner.invoke(macsec.macsec.commands["profile"].commands["del"], ["test"], obj=db) assert result.exit_code != 0 @@ -123,7 +124,8 @@ class TestConfigMACsec(object): result = runner.invoke(macsec.macsec.commands["port"].commands["del"], ["Ethernet0"], obj=db) assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) port_table = db.cfgdb.get_entry("PORT", "Ethernet0") - assert not port_table["macsec"] + assert "macsec" not in port_table or not port_table["macsec"] + assert port_table["admin_status"] == "up" def test_macsec_invalid_operation(self, mock_cfgdb): diff --git a/dockers/docker-macsec/cli/config/plugins/macsec.py b/dockers/docker-macsec/cli/config/plugins/macsec.py index e5bc628b8c..b76de8c98c 100644 --- a/dockers/docker-macsec/cli/config/plugins/macsec.py +++ b/dockers/docker-macsec/cli/config/plugins/macsec.py @@ -42,7 +42,13 @@ def add_port(db, port, profile): if len(profile_entry) == 0: ctx.fail("profile {} doesn't exist".format(profile)) - db.cfgdb.set_entry("PORT", port, {'macsec': profile}) + port_entry = db.cfgdb.get_entry('PORT', port) + if len(port_entry) == 0: + ctx.fail("port {} doesn't exist".format(port)) + + port_entry['macsec'] = profile + + db.cfgdb.set_entry("PORT", port, port_entry) # @@ -64,7 +70,13 @@ def del_port(db, port): if port is None: ctx.fail("cannot find port name for alias {}".format(alias)) - db.cfgdb.set_entry("PORT", port, {'macsec': ""}) + port_entry = db.cfgdb.get_entry('PORT', port) + if len(port_entry) == 0: + ctx.fail("port {} doesn't exist".format(port)) + + del port_entry['macsec'] + + db.cfgdb.set_entry("PORT", port, port_entry) #