Skip to content

Fix issue when many containers are created but only one is deleted#586

Merged
antho1404 merged 12 commits intodevfrom
bug/fix-pending-containers
Nov 29, 2018
Merged

Fix issue when many containers are created but only one is deleted#586
antho1404 merged 12 commits intodevfrom
bug/fix-pending-containers

Conversation

@antho1404
Copy link
Copy Markdown
Member

If a MESG service is created but has some errors like for example like that:

  • A service that is connecting to a database
  • a dependency that is running the database (influxdb for example)

When the MESG service starts with these two docker services, the first service will crash because it cannot access the database that takes time to start and will restart.

When it restarts docker stop the initial container but without destroying it so we needed to delete this container. If the initial service crashes multiple times, multiples containers are stopped but not deleted. This PR is fixing that by ensuring that all the remaining containers are actually deleted and doesn't block the execution of a new service and doesn't stuck the action to stop the MESG service (pending indefinitely because a container is still existing)

@antho1404 antho1404 added the bug Something isn't working label Nov 27, 2018
NicolasMahe
NicolasMahe previously approved these changes Nov 28, 2018
@ilgooz
Copy link
Copy Markdown
Contributor

ilgooz commented Nov 28, 2018

Tests are not passing.

@NicolasMahe
Copy link
Copy Markdown
Member

Tests are not passing.

@ilgooz When the test are failing with the following type of error, you can go on circleci and press rerun workflow. thx

panic: starting container failed: No such network: core [recovered]
	panic: starting container failed: No such network: core

@krhubert
Copy link
Copy Markdown
Contributor

But the tests are not passing due to this error:

=== RUN   TestStopService
coverage: 87.3% of statements
panic: test timed out after 5m0s

goroutine 506 [running]:
testing.(*M).startAlarm.func1()
	/usr/local/go/src/testing/testing.go:1240 +0xfc
created by time.goFunc
	/usr/local/go/src/time/sleep.go:172 +0x44

It's timeout on test stop

@NicolasMahe
Copy link
Copy Markdown
Member

Where do you see this test failing?
On circle ci, I see:

--- FAIL: TestIsRunning (0.61s)
panic: starting container failed: No such network: core [recovered]
	panic: starting container failed: No such network: core

goroutine 68 [running]:
testing.tRunner.func1(0xc42060c000)
	/usr/local/go/src/testing/testing.go:742 +0x29d
panic(0x7f0d20, 0xc4203b9010)
	/usr/local/go/src/runtime/panic.go:502 +0x229
github.com/mesg-foundation/core/daemon.startForTest()
	/go/src/github.com/mesg-foundation/core/daemon/start_test.go:31 +0x21c
github.com/mesg-foundation/core/daemon.TestIsRunning(0xc42060c000)
	/go/src/github.com/mesg-foundation/core/daemon/status_test.go:51 +0x26
testing.tRunner(0xc42060c000, 0x895ee8)
	/usr/local/go/src/testing/testing.go:777 +0xd0
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:824 +0x2e0
FAIL	github.com/mesg-foundation/core/daemon	3.552s

@ilgooz
Copy link
Copy Markdown
Contributor

ilgooz commented Nov 28, 2018

@NicolasMahe It's there, near to beginning of logs: https://circleci.com/gh/mesg-foundation/core/4541?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Tests should be updated to meet with the new behavior that this PR introduces.

@NicolasMahe NicolasMahe dismissed their stale review November 28, 2018 11:21

tests fail

Copy link
Copy Markdown
Member

@NicolasMahe NicolasMahe left a comment

Choose a reason for hiding this comment

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

Tests are failing

@NicolasMahe
Copy link
Copy Markdown
Member

NicolasMahe commented Nov 29, 2018

@ilgooz could you take a look and fix the test please?

Maybe we could do the test using docker mock? #588

@ilgooz
Copy link
Copy Markdown
Contributor

ilgooz commented Nov 29, 2018

@ilgooz could you take a look and fix the test please?

Maybe we could do the test using docker mock? #588

@NicolasMahe Of course, by the way, let's merge testify mocks: #588

@NicolasMahe NicolasMahe self-requested a review November 29, 2018 09:36
@ilgooz ilgooz requested review from NicolasMahe and removed request for ilgooz November 29, 2018 10:10
@antho1404 antho1404 merged commit 1278f6b into dev Nov 29, 2018
@antho1404 antho1404 deleted the bug/fix-pending-containers branch November 29, 2018 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants