Use vol plugin creator instead of inserting spec #29894
Conversation
This makes the test a bit more robust to change and is a bit cleaner. As implemented before this commit, we have two named plugins pointing to the same http service. If the daemon makes any unexpected calls to the plugin (e.g. during startup) we'll get more counts on the event counter than expected since the daemon sees 2 plugins. Found this while working on moby#29877 which broke this test originally (but is no longer using V1 plugins, so is this is no longer broken there) and took some time to debug what was going on. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
| go func() { | ||
| if out, err := s.d.Cmd("run", "--rm", "--name", "test-data-retry", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", "test-external-volume-driver-retry", "busybox:latest"); err != nil { | ||
| close(started) |
There was a problem hiding this comment.
I don't understand this trick. I saw it before and still think it's extremely weird.
There was a problem hiding this comment.
This should stay running until the plugin timeout is reached (like 13 seconds or something?)
There was a problem hiding this comment.
I don't understand what should stay running :)
Code looks like:
started := make(chan struct{})
go func() {
close(started)
Now I think it's sort of mistake, because there is no way you would need this in tests.
There was a problem hiding this comment.
I'm sorry I'm not understanding what you are saying here.
There was a problem hiding this comment.
started := make(chan struct{})
go func() {
close(started)
if out, err := s.d.Cmd("run", "--rm", "--name", "test-data-retry", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", driverName, "busybox:latest"); err != nil {
errchan <- fmt.Errorf("%v:\n%s", err, out)
}
close(errchan)
}()
What the point of started chan?
There was a problem hiding this comment.
Just making sure the test doesn't flake out because the goroutine hasn't started yet, but later we are only waiting some fixed amount of time.
|
|
||
| <-started | ||
| // wait for a retry to occur, then create spec to allow plugin to register | ||
| time.Sleep(2000 * time.Millisecond) |
|
Left some comments. |
Signed-off-by: Daniel Nephin <dnephin@docker.com>
1c5b0cf to
cbf2f4d
Compare
|
Good to merge? |
|
Yes ! |
This makes the test a bit more robust to change and is a bit cleaner.
As implemented before this commit, we have two named plugins pointing to
the same http service. If the daemon makes any unexpected calls to the
plugin (e.g. during startup) we'll get more counts on the event counter
than expected since the daemon sees 2 plugins.
Found this while working on #29877 which broke this test originally (but
is no longer using V1 plugins, so is this is no longer broken there) and
took some time to debug what was going on.