Skip to content

Test: Checks for deadlocks panics in logs per each test.#3211

Merged
raybejjani merged 1 commit intocilium:masterfrom
eloycoto:CheckPanics
Mar 21, 2018
Merged

Test: Checks for deadlocks panics in logs per each test.#3211
raybejjani merged 1 commit intocilium:masterfrom
eloycoto:CheckPanics

Conversation

@eloycoto
Copy link
Copy Markdown
Member

This commit make sure that there is no panic or deadlocks in cilium logs
after executed the test.

The test uses CurrentGinkgoTestDescription().Duration to know the
current duration of the test, so the helper only search in the logs
files since test starts. (In case of any panic in log, it'll not mark
all test as failed)

The panic string example is:

Mar 15 16:35:50 runtime cilium-agent[8220]: panic: runtime error: invalid memory address or nil pointer dereference
Mar 15 16:35:50 runtime cilium-agent[8220]: [signal SIGSEGV: segmentation violation code=0x1 addr=0x2 pc=0x17e3b2c]
Mar 15 16:35:50 runtime cilium-agent[8220]: goroutine 58384 [running]:

The deadlock string example is:

Jan 03 00:40:09 cilium-master cilium-agent[32439]: POTENTIAL DEADLOCK:
Jan 03 00:40:09 cilium-master cilium-agent[32439]: Previous place where the lock was grabbed
Jan 03 00:40:09 cilium-master cilium-agent[32439]: goroutine 8974 lock 0xc4201b875c

Signed-off-by: Eloy Coto eloy.coto@gmail.com

@eloycoto eloycoto added kind/enhancement This would improve or streamline existing functionality. pending-review area/CI Continuous Integration testing issue or flake labels Mar 20, 2018
@eloycoto eloycoto requested review from a team as code owners March 20, 2018 11:07
@eloycoto
Copy link
Copy Markdown
Member Author

test-me-please

This commit make sure that there is no panic or deadlocks in cilium logs
after executed the test.

The test uses `CurrentGinkgoTestDescription().Duration` to know the
current duration of the test, so the helper only search in the logs
files since test starts. (In case of any panic in log, it'll not mark
all test as failed)

The panic string example is:

```
Mar 15 16:35:50 runtime cilium-agent[8220]: panic: runtime error: invalid memory address or nil pointer dereference
Mar 15 16:35:50 runtime cilium-agent[8220]: [signal SIGSEGV: segmentation violation code=0x1 addr=0x2 pc=0x17e3b2c]
Mar 15 16:35:50 runtime cilium-agent[8220]: goroutine 58384 [running]:
```

The deadlock string example is:
```
Jan 03 00:40:09 cilium-master cilium-agent[32439]: POTENTIAL DEADLOCK:
Jan 03 00:40:09 cilium-master cilium-agent[32439]: Previous place where the lock was grabbed
Jan 03 00:40:09 cilium-master cilium-agent[32439]: goroutine 8974 lock 0xc4201b875c
```

Signed-off-by: Eloy Coto <eloy.coto@gmail.com>
@eloycoto
Copy link
Copy Markdown
Member Author

test-me-please

1 similar comment
@eloycoto
Copy link
Copy Markdown
Member Author

test-me-please

Copy link
Copy Markdown
Member

@ianvernon ianvernon left a comment

Choose a reason for hiding this comment

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

Looks great overall, just some minor doc fixes needed, so no need to re-ask for review again once you have fixed them. Thanks for doing this! One comment -in the future, please have vendor updates as separate commits.

Comment thread test/helpers/cilium.go
}

// ValidateNoErrorsOnLogs checks in cilium logs since the given duration (By
// default `CurrentGinkgoTestDescription().Duration`) that no `panic` messages
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

... checks that the cilium logs ... do not contain panic or deadlock messages.

Comment thread test/helpers/kubectl.go

}

// ValidateNoErrorsOnLogs checks in cilium logs since the given duration (By
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

... checks that the cilium logs ... do not contain panic or deadlock messages.

@raybejjani raybejjani merged commit 5185789 into cilium:master Mar 21, 2018
@raybejjani
Copy link
Copy Markdown
Contributor

oh, crap. I merged this without waiting for the changes ian asked for. @eloycoto could you make another PR for that and I'll merge it in. sorry

@ianvernon
Copy link
Copy Markdown
Member

@raybejjani no worries! Nothing was blocking on my review.

eloycoto added a commit to eloycoto/cilium that referenced this pull request Mar 23, 2018
Addressed comments from PR cilium#3211

Signed-off-by: Eloy Coto <eloy.coto@gmail.com>
tgraf pushed a commit that referenced this pull request Mar 23, 2018
Addressed comments from PR #3211

Signed-off-by: Eloy Coto <eloy.coto@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/CI Continuous Integration testing issue or flake kind/enhancement This would improve or streamline existing functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants