Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -4630,12 +4630,14 @@ def validate_vlan_exists(db,text):
# 'add' subcommand
#

@ip.command()

@ip.command('add')
@click.argument('interface_name', metavar='<interface_name>', required=True)
@click.argument("ip_addr", metavar="<ip_addr>", required=True)
@click.argument('gw', metavar='<default gateway IP address>', required=False)
@click.option('--secondary', "-s", is_flag=True, default=False)
@click.pass_context
def add(ctx, interface_name, ip_addr, gw):
def add_interface_ip(ctx, interface_name, ip_addr, gw, secondary):
"""Add an IP address towards the interface"""
# Get the config_db connector
config_db = ValidatedConfigDBConnector(ctx.obj['config_db'])
Expand Down Expand Up @@ -4720,7 +4722,25 @@ def add(ctx, interface_name, ip_addr, gw):
config_db.set_entry(table_name, interface_name, {"admin_status": "up"})
else:
config_db.set_entry(table_name, interface_name, {"NULL": "NULL"})
config_db.set_entry(table_name, (interface_name, str(ip_address)), {"NULL": "NULL"})

if secondary:
# We update the secondary flag only in case of VLAN Interface.
if table_name == "VLAN_INTERFACE":
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For other interface types, do we need to throw an error?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need a validation to make sure not all IPs are marked as secondary?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need a validation to make sure no more than one primary IP for a given VLAN interface?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need a validation to make sure not all IPs are marked as secondary?

Ack, this is a good check.

Do we need a validation to make sure no more than one primary IP for a given VLAN interface?

This we cannot do for two reasons: a) impact to potential existing devices with more than one interface b) interchanging primary with secondary would need more than one primary intermittently.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This has been addressed. Only vlans with atleast one primary can be added with secondaries.

vlan_interface_table = config_db.get_table(table_name)
contains_primary = False
for key, value in vlan_interface_table.items():
if not isinstance(key, tuple):
continue
name, prefix = key
if name == interface_name and "secondary" not in value:
contains_primary = True
if contains_primary:
config_db.set_entry(table_name, (interface_name, str(ip_address)), {"secondary": "true"})
else:
ctx.fail("Primary for the interface {} is not set, so skipping adding the interface"
.format(interface_name))
else:
config_db.set_entry(table_name, (interface_name, str(ip_address)), {"NULL": "NULL"})

#
# 'del' subcommand
Expand Down
29 changes: 29 additions & 0 deletions tests/ip_config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,35 @@ def test_add_del_interface_valid_ipv4(self):
assert mock_run_command.call_count == 1
assert ('Eth36.10', '32.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

# config int ip add vlan1000 10.21.20.1/24 as secondary
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"],
["Vlan1000", "10.11.20.1/24", "--secondary"], obj=obj)
assert result.exit_code == 0
assert ('Vlan1000', '10.11.20.1/24') in db.cfgdb.get_table('VLAN_INTERFACE')
assert db.cfgdb.get_table('VLAN_INTERFACE')[('Vlan1000', '10.11.20.1/24')]['secondary'] == "true"

# config int ip add vlan2000 10.21.20.1/24 as secondary
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"],
["Vlan2000", "10.21.20.1/24", "-s"], obj=obj)
assert result.exit_code == 0
assert ('Vlan2000', '10.21.20.1/24') in db.cfgdb.get_table('VLAN_INTERFACE')
assert db.cfgdb.get_table('VLAN_INTERFACE')[('Vlan2000', '10.21.20.1/24')]['secondary'] == "true"

# config int ip add vlan4000 10.16.20.1/24 as primary and make sure secondary is not present in table
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"],
["Vlan4000", "10.16.20.1/24"], obj=obj)
assert result.exit_code == 0
assert ('Vlan4000', '10.16.20.1/24') in db.cfgdb.get_table('VLAN_INTERFACE')
assert 'secondary' not in db.cfgdb.get_table('VLAN_INTERFACE')[('Vlan4000', '10.16.20.1/24')]

# create vlan 500
result = runner.invoke(config.config.commands["vlan"].commands["add"], ["500"], obj=db)
# config int ip add vlan500 10.21.20.1/24 as secondary - should fail as vlan500 is not added in table
ERR_MSG = "Error: Primary for the interface Vlan500 is not set, so skipping adding the interface"
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"],
["Vlan500", "10.21.20.1/24", "--secondary"], obj=obj)
assert result.exit_code != 0
assert ERR_MSG in result.output

def test_add_interface_invalid_ipv4(self):
db = Db()
Expand Down