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

Test if error is nil before to log it #2720

Merged
merged 1 commit into from
Aug 1, 2018

Conversation

nmengin
Copy link
Contributor

@nmengin nmengin commented Jul 31, 2018

Signed-off-by: nmengin [email protected]

- What I did

This PR adds a test on the error sent by the function store.CreateNetwork() when a manager becomes leader in the way to avoid the creation of an error message for a error which is nil.

- How I did it

The if statement test if the error is not nil before to log a message.

- How to test it

Start a swarmkit cluster with a log level >= logLevel.Error.
Thanks to the modification the errors described below sould not appear:

level=error msg="failed to create predefined network bridge" error="<nil>"
level=error msg="failed to create predefined network host" error="<nil>"

- Description for the changelog

Delete the error logs generated when prefefined networks are initiliazed successfully, Fixes #2719 .

@codecov
Copy link

codecov bot commented Jul 31, 2018

Codecov Report

Merging #2720 into master will decrease coverage by 0.11%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2720      +/-   ##
==========================================
- Coverage   61.99%   61.87%   -0.12%     
==========================================
  Files         134      134              
  Lines       21771    21771              
==========================================
- Hits        13496    13471      -25     
- Misses       6818     6850      +32     
+ Partials     1457     1450       -7

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

ping @fcrisciani

Copy link

@fcrisciani fcrisciani left a comment

Choose a reason for hiding this comment

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

LGTM

@dperny
Copy link
Collaborator

dperny commented Aug 1, 2018

Wow, this is such a dumb oversight by us. Yeah, LGTM, much better, thanks.

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.

Error log when error is nil.
4 participants