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

Fix panic when checking service status #1078

Merged
merged 2 commits into from
Dec 20, 2022

Conversation

jsoriano
Copy link
Member

There seems to be a race condition since a container starts running, till its state
reports any health information. During this time the value obtained from the API
is nil. Don't try to get the status from the healtcheck if that's the case.

This fixes panics like this one:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x231d562]

goroutine 1 [running]:
github.com/elastic/elastic-package/internal/stack.newServiceStatus(0xc000541b40)
	/home/jaime/gocode/src/github.com/elastic/elastic-package/internal/stack/compose.go:224 +0x1a2
github.com/elastic/elastic-package/internal/stack.dockerComposeStatus()
	/home/jaime/gocode/src/github.com/elastic/elastic-package/internal/stack/compose.go:206 +0x165
github.com/elastic/elastic-package/internal/stack.Status()
	/home/jaime/gocode/src/github.com/elastic/elastic-package/internal/stack/status.go:16 +0x2b
github.com/elastic/elastic-package/internal/stack.(*composeProvider).Status(0x0, {0x0, {0x0, 0x0}, {0x0, 0x0, 0x0}, 0xc00045f0e0, {0x3f70698, 0xc000513800}})
	/home/jaime/gocode/src/github.com/elastic/elastic-package/internal/stack/providers.go:82 +0x17
github.com/elastic/elastic-package/cmd.setupStackCommand.func6(0xc000513800, {0x5892d28?, 0x0?, 0x0?})
	/home/jaime/gocode/src/github.com/elastic/elastic-package/cmd/stack.go:258 +0xa5
github.com/spf13/cobra.(*Command).execute(0xc000513800, {0x5892d28, 0x0, 0x0})
	/home/jaime/gocode/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:916 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0xc000729800)
	/home/jaime/gocode/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:1044 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
	/home/jaime/gocode/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:968
main.main()
	/home/jaime/gocode/src/github.com/elastic/elastic-package/main.go:29 +0xe5
exit status 2

Seen while testing #1055, but looks like it may affect current code too.

There seems to be a race condition since a container starts running,
till its state reports any health information. During this time the
value obtained from the API is nil. Don't try to get the status from
the healtcheck if that's the case.
@jsoriano jsoriano requested a review from a team December 19, 2022 16:52
@jsoriano jsoriano self-assigned this Dec 19, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 19, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-12-19T20:34:58.997+0000

  • Duration: 34 min 55 sec

Test stats 🧪

Test Results
Failed 0
Passed 870
Skipped 0
Total 870

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 19, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (34/34) 💚
Files 67.188% (86/128) 👍
Classes 61.957% (114/184) 👍
Methods 48.705% (376/772) 👍
Lines 31.925% (3409/10678) 👍 0.019
Conditionals 100.0% (0/0) 💚

internal/stack/compose.go Outdated Show resolved Hide resolved
@jsoriano jsoriano merged commit 9818f7a into elastic:main Dec 20, 2022
@jsoriano jsoriano deleted the nil-state-health branch December 20, 2022 09:31
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.

3 participants