Add type for namespaces for better UI

[crosbymichael]

This adds type Namespaces []Namespace so that methods can be added to
this slice so that it is easier for consumers to work with the values.

Signed-off-by: Michael Crosby [email protected]

[crosbymichael]

@LK4D4 please review this new UI

[crosbymichael]

@LK4D4 @vmarmol do you think the Names for the namespaces should be consts to enforce types and values?

[LK4D4]

I like consts.

[LK4D4]

I think this probably should be Add and current Add should be private or removed. I don't see case when I don't want to check for existing namespace on Add.

[mrunalp]

How about SetPath just sets the path or updates it? Add could check for existence.

[vmarmol]

+1 to names being consts.

[crosbymichael]

@LK4D4 @vmarmol I updated to use types for the ns consts

[mrunalp]

"Contains" as a wrapper to do this check?

[vmarmol]

LGTM, outside of the "Contains" wrapper which sounds like a good idea.

[vmarmol]

btw, the change of the type of Namespace broke backwards compatibility (newer libcontainers being able to understand the config of older libcontainers). We've worked around it in cAdvisor, but Docker may be affected too when we update the version of libcontainer.

[crosbymichael]

@vmarmol yes, that is true. Hopefully we will not have to make the change again and this will be a more robust structure that will allow for changes without breaking compat. Do you still think it's the right thing to do?

[vmarmol]

It is definitely more extensible. As long as we can handle the change in Docker, we should be fine.

[LK4D4]

@vmarmol @rjnagal can we merge this? Or maybe you want me to care this PR to add Contains?

[rjnagal]

LGTM. Let's get this in and we can add Contains as a separate PR.

[rjnagal]

Actually, there is no use of index other than to check Contains(). Can we do that in the same PR then :)

[LK4D4]

@rjnagal I'll prepare PR little later, Michael now on vacation.

[mrunalp]

Yep, please add Contains if you can :)