host-local: add ifname to file tracking IP address used #203
Conversation
…ine of file tracking the IP address used by ContainerID Update host-local tests to use ifname along with ContainerID in store file Signed-off-by: Michael Cambria <mcambria@redhat.com>
|
We also have to handle upgrades over an existing store. So, when you get a Delete, and there is a reservation for the container ID without the interface, we should delete it too. |
…nerID without the interface
| @@ -99,7 +99,9 @@ func (s *Store) Release(ip net.IP) error { | |||
|
|
|||
| // N.B. This function eats errors to be tolerant and | |||
| // release as much as possible | |||
| func (s *Store) ReleaseByID(id string) error { | |||
| func (s *Store) ReleaseByID(id string, ifname string) error { | |||
|
|
|||
| @@ -65,7 +65,7 @@ func (s *Store) Reserve(id string, ip net.IP, rangeID string) (bool, error) { | |||
| if err != nil { | |||
| return false, err | |||
| } | |||
| if _, err := f.WriteString(strings.TrimSpace(id)); err != nil { | |||
| if _, err := f.WriteString(strings.TrimSpace(id) + "\n" + ifname); err != nil { | |||
There was a problem hiding this comment.
@mccv1r0 your concerns WRT Windows are probably valid. We could use \r\n if you want to be more compatible there.
There was a problem hiding this comment.
Nobody seemed to like my suggestion of using ASCII BEL :-)
| } | ||
| return nil | ||
| }) | ||
|
|
||
| if (!found) && (err == nil) { |
There was a problem hiding this comment.
You actually don't need the () around !found since it's a bool already.
Also, maybe a comment here explaining that this block is for backwards compatibility when reading files written by a previous version of the plugin that didn't store interface name?
There was a problem hiding this comment.
Please ensure you've run go fmt. It should remove those unnecessary parens (and help prevent other stylistic inconsistencies).
| } | ||
| return nil | ||
| }) | ||
|
|
||
| if (!found) && (err == nil) { | ||
| err = filepath.Walk(s.dataDir, func(path string, info os.FileInfo, err error) error { |
There was a problem hiding this comment.
Another approach would be to pull the directory Walk/data compare/file remove code out to a new function, and then just call it twice; once for id+\n+iface, and if that wasn't found a second time for just 'id'.
Add two interfaces (e.g. eth0, eth1) to the same container. Ensure each file now has ContainerID and ifname. Delete one, ensure that the right file was deleted. Add an interface using just ContainerID in the file. Delete to verify we are still backwards compatible with any files created using earlier verison of host-local plugin.
|
Testcase failures are unrelated and will be fixed by #207 |
…ine of file tracking the IP address used by ContainerID Update host-local tests to use ifname along with ContainerID in store file Signed-off-by: Michael Cambria <mcambria@redhat.com>
…nerID without the interface
Add two interfaces (e.g. eth0, eth1) to the same container. Ensure each file now has ContainerID and ifname. Delete one, ensure that the right file was deleted. Add an interface using just ContainerID in the file. Delete to verify we are still backwards compatible with any files created using earlier verison of host-local plugin.
host-local: add ifname to file tracking IP address used
Obtain ifname from CmdArgs and pass to backend
In the file tracking the IP address in use, add ifname to the line after ContainerID
Update host-local tests to use ifname along with ContainerID in storage file
Fixes #164