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.

Sign up for GitHub

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

tools: fix reload interface deletion #16723

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+7 −6
Open

tools: fix reload interface deletion #16723

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 7 additions & 6 deletions tools/frr-reload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1740,12 +1740,13 @@ def compare_context_objects(newconf, running):
delete_bgpd = True
lines_to_del.append((running_ctx_keys, None))

# We cannot do 'no interface' or 'no vrf' in FRR, and so deal with it
elif (
running_ctx_keys[0].startswith("interface")
or running_ctx_keys[0].startswith("vrf")
or running_ctx_keys[0].startswith("router pim")
):
elif running_ctx_keys[0].startswith("interface"):
Copy link
Member

Choose a reason for hiding this comment

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

If nothing is present under interface context, then it's not printed at all. I'm trying to understand what problem are we solving here.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the confusion, I've updated the description I hope it's more clear now.

Copy link
Member

Choose a reason for hiding this comment

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

please put the description into the commit message itself. No-one is going to look at the actual PR in a couple of years. They'll look at the commit via git show...

Copy link
Member

Choose a reason for hiding this comment

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

Btw, shouldn't we check for len(running_ctx_keys) == 1 in this case too? It might be also interface blabla vrf ..., right?

Copy link
Author

Choose a reason for hiding this comment

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

It should not make a difference in my opinion.
Because now it matches everything starting with interface which is under the "global" configuration context.

For example it will match:

interface foo

But it will also match:

interface foo vrf bar

But it will not match something like:

some context
    interface foo

lines_to_del.append((running_ctx_keys, None))

# We cannot do 'no vrf' in FRR, and so deal with it
elif running_ctx_keys[0].startswith("vrf") or running_ctx_keys[
0
].startswith("router pim"):
for line in running_ctx.lines:
lines_to_del.append((running_ctx_keys, line))

Expand Down
Loading