Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry exec commands on text file busy #763

Merged
merged 1 commit into from
Apr 22, 2020

Conversation

saschagrunert
Copy link
Contributor

This makes the plugin validation a bit more robust by retrying the
execution if the plugin is in text file busy state. This can happen if
we write the config and the plugin at once. To avoid such races we now
try up to 5 seconds for the plugin validation.

@coveralls
Copy link

coveralls commented Apr 3, 2020

Coverage Status

Coverage increased (+0.01%) to 75.044% when pulling e2a7366 on saschagrunert:version-retry into bf84331 on containernetworking:master.

libcni/api.go Outdated Show resolved Hide resolved
@squeed
Copy link
Member

squeed commented Apr 15, 2020

As was mentioned in the discussion above, can you move this logic to RawExec (good point, @mars1024). Then we can get this in.

@saschagrunert saschagrunert force-pushed the version-retry branch 3 times, most recently from ab19ddf to dbed933 Compare April 16, 2020 07:10
@saschagrunert saschagrunert changed the title Retry validation on text file busy Retry exec commands on text file busy Apr 16, 2020
This makes the plugin validation a bit more robust by retrying the
execution if the plugin is in text file busy state. This can happen if
we write the config and the plugin at once. To avoid such races we now
try up to 5 seconds for the plugin validation.

Signed-off-by: Sascha Grunert <sgrunert@suse.com>
Comment on lines +43 to 61
// Retry the command on "text file busy" errors
for i := 0; i <= 5; i++ {
err := c.Run()

// Command succeeded
if err == nil {
break
}

// If the plugin is currently about to be written, then we wait a
// second and try it again
if strings.Contains(err.Error(), "text file busy") {
time.Sleep(time.Second)
continue
}

// All other errors except than the busy text file
return nil, e.pluginErr(err, stdout.Bytes(), stderr.Bytes())
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dcbw I moved the logic over here, should me make it configurable somehow?

Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@bboreham bboreham merged commit 7743645 into containernetworking:master Apr 22, 2020
@saschagrunert saschagrunert deleted the version-retry branch April 22, 2020 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants