Forbid adding duplicated namespace

[yummypeng]

We now forbid duplicated namespace in spec(#1150), if there is a duplicated namespace, then it'll just return with error in CreateLibcontainerConfig():

for _, ns := range spec.Linux.Namespaces {
                t, exists := namespaceMapping[ns.Type]
                if !exists {
                        return nil, fmt.Errorf("namespace %q does not exist", ns)
                }
                if config.Namespaces.Contains(t) {
                        return nil, fmt.Errorf("malformed spec file: duplicated ns %q", ns)
                }
                config.Namespaces.Add(t, ns.Path)
        }

So, as far as I can see, the last line of code in Add() will never be reached here. Therefore, I just modified this func to be consistent with Remove(). Besides, this modification also forbids adding duplicated namespace to make the logic more clear.

Signed-off-by: Yuanhong Peng [email protected]

[cyphar]

You haven't changed any of the callers of .Add to check the return value, so we're now ignoring duplicates. Please fix that.

[WeiZhang555]

I don't think this is right.

This function is not only for adding new namespaces, but also for modifying path of existing namespace, you shouldn't disable this function. Also as @cyphar said, you didn't modify any caller so the return false/true is meaningless.

[yummypeng]

Except the code I quoted before, I only find the callers of Add() in some test code. I know that this function is also used for modifying path of existing namespace, but I still think it would be better that if we want to modify path of existing namespace, we should remove it first, and then add it. If this idea is acceptable, I will fix those test functions then.

[yummypeng]

Hmmmm.....If we must remove and then add to modify path of an existing namespace, then I think we won't need any check in Add().

[cyphar]

Actually, I'm confused. The code you quoted already doesn't allow for duplicate namespaces -- what's the benefit of this PR (it also appears to have broken the tests)?

[yummypeng]

At the beginning, I just mistakenly thought the last line of code in Add() is redundant. But now it seems that my PR is meaningless and it's better to remain unchanged. Thanks for all your comments. I will close this PR later.