[ovs-dev] [PATCH] system-traffic: Check namespace exists befoe delete.

[Darrell Ball]

On Mon, May 2, 2016 at 11:12 AM, Joe Stringer <joe at ovn.org> wrote:

> On 2 May 2016 at 09:04, Darrell Ball <dlu998 at gmail.com> wrote:
> > On Mon, May 2, 2016 at 8:39 AM, William Tu <u9012063 at gmail.com> wrote:
> >
> >> Hi Darrel,
> >>
> >>  # Delete namespaces from the running OS
> >>>>  m4_define([DEL_NAMESPACES],
> >>>>     [m4_foreach([ns], [$@],
> >>>> -               [ip netns del ns
> >>>> -])
> >>>> +               [if ip netns list | grep ns > /dev/null; then
> >>>> +                   ip netns del ns
> >>>> +                fi
> >>>> +               ])
> >>>>     ]
> >>>>  )
> >>>>
> >>>
> >>> Do we want to suppress an error on deletion in general ?
> >>>
> >>>
> >> No, I think it won't suppress errors on deletion.
> >>
> >
> >
> > Just to be clear, what the comment means is that if:
> > 1) DEL_NAMESPACE is called and there is no such ns, then this may be an
> > error with the surrounding code (i.e. a bug) or maybe the test itself.
> >
> > 2) Hence the above code in DEL_NAMESPACE would make the bug less
> > visible since there would be no visible complaint on trying to delete a
> ns
> > that does not exist
>
> I was actually just wondering about why we need DEL_NAMESPACES.
> Originally, if you did a CTRL+C in the middle of the test, then
> cleanup would not properly occur so you'd end up with all of these
> test namespaces still existing. By deleting all of the specified
> namespaces at the start of ADD_NAMESPACES, it would allow the test to
> proceed without forcing the user to go through and delete all of the
> namespaces.
>

For the purposes of ADD_NAMESPACES, I agree - DEL_NAMESPACES
is not essential.


>
> However, if we were to queue up namespace deletion using on_exit "ip
> netns delete foo" immediately after creation, then the above issue
> should not exist, so maybe we could get rid of DELETE_NAMESPACES?
>

However, testing delete namespaces within the testsuite in other respects
(in future) seems useful to catch bugs as opposed to just cleanup.
DEL_NAMESPACES
seems like one small wrapper that could be used to make this cleaner,
although
it is not essential.


>
> In general I've advocated in the tests that while the test-writer
> needs to specify things like ADD_NAMESPACES(), those commands will
> queue up the cleanup to ensure that whether the test passes or fails,
> the system is left in a tidy state. This means that it is not
> necessary inside of tests to add the DELETE_NAMESPACES() towards the
> end (which would only execute if the rest of the test was successful).
>