[BUG]: Fixed docker start command bug #1586

Yaxhveer · 2024-02-16T18:04:40Z

Related Issue

Info about Issue or bug

Closes: #1539

Describe the changes you've made

Users can use docker start command to start the application. One have to specify the network and container name of the application using --networkName and --containerName tags.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Code style update (formatting, local variables)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Please let us know if any test cases are added

Please describe the tests(if any). Provide instructions how its affecting the coverage.

Describe if there is any unusual behaviour of your code(Write `NA` if there isn't)

A clear and concise description of it.

Checklist:

My code follows the style guidelines of this project.
I have performed a self-review of my own code.
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.
My changes generate no new warnings.
I have added tests that prove my fix is effective or that my feature works.
New and existing unit tests pass locally with my changes.

Screenshots (if any)

Original	Updated
original screenshot	updated screenshot

Yaxhveer · 2024-02-18T04:58:21Z

@charankamarapu Kindly review this PR.

charankamarapu · 2024-02-19T18:05:36Z

will review by 20 Feb EOD.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

…ploy#1590) Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

charankamarapu · 2024-02-21T17:09:15Z

Hey @Yaxhveer are the changes done or anything let over..?

Yaxhveer · 2024-02-21T17:34:25Z

@charankamarapu Yes they are done.

charankamarapu

Please address the comments.

charankamarapu · 2024-02-22T05:04:49Z

pkg/service/test/test.go

 	if !initialisedValues.AbortStopHooksForcefully {
 		initialisedValues.AbortStopHooksInterrupt <- true
 		// stop listening for the eBPF events
-		initialisedValues.LoadedHooks.Stop(true)
+		initialisedValues.LoadedHooks.Stop(true, removeContainer)


This is not a preferred approach, We shouldn't signal things from service layer a lot.

charankamarapu · 2024-02-22T05:07:14Z

pkg/clients/clients.go

@@ -10,7 +10,7 @@ type InternalDockerClient interface {
 	ExtractNetworksForContainer(containerName string) (map[string]*network.EndpointSettings, error)
 	ConnectContainerToNetworks(containerName string, settings map[string]*network.EndpointSettings) error
 	ConnectContainerToNetworksByNames(containerName string, networkName []string) error
-	StopAndRemoveDockerContainer() error
+	StopDockerContainer(removeContainer bool) error


try to keep the same function and and use some param like skipRemove. It will be good if this skipRemove is give by hooks only and service shouldn't give this param to hooks and hooks to internalDockerClient.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer · 2024-02-24T10:10:26Z

Hey @charankamarapu
Just wanted to clarify that here we are checking if command is docker related or not, since in docker start as well in docker detached mode the process is exited but still we have to remove the container.

charankamarapu · 2024-02-28T04:54:19Z

The main idea is we have return to a previous state where user didn't use keploy. Like if the container was created before running keploy we shouldn't remove it after running with keploy. If keploy has created that container for him then we should remove it.

Yaxhveer · 2024-02-28T07:04:52Z

@charankamarapu Yes for the same logic we check if the command uses docker start or not. Also please review the changes and let me know if the implementation works.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

…into docker-start-cmd

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer · 2024-04-21T13:55:02Z

@gouravkrosx Please review the changes.

gouravkrosx · 2024-04-29T11:03:14Z

@Yaxhveer, I will try to review this EOD.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer · 2024-05-13T15:39:41Z

@gouravkrosx Reminder! Review for the PR is pending.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

gouravkrosx · 2024-05-21T06:11:37Z

pkg/core/app/app.go

@@ -69,12 +69,13 @@ func (a *App) Setup(_ context.Context) error {
 	}
 	a.docker = d

-	if (a.kind == utils.Docker || a.kind == utils.DockerCompose) && IsDetachMode(a.cmd) {
-		return fmt.Errorf("detach mode is not allowed in Keploy command")
+	if (a.kind == utils.DockerStart || a.kind == utils.DockerRun || a.kind == utils.DockerCompose) && isDetachMode(a.logger, a.cmd, a.kind) {


rather than this use a separate function for eg:

func isDockerKind(kind utils.CmdType) bool { return kind == utils.DockerStart || kind == utils.DockerRun || kind == utils.DockerCompose }

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

gouravkrosx · 2024-05-21T06:30:28Z

pkg/service/replay/replay.go

@@ -339,7 +339,7 @@ func (r *Replayer) RunTestSet(ctx context.Context, testSetID string, testRunID s

 	cmdType := utils.FindDockerCmd(r.config.Command)


Instead of using this function utils.FindDockerCmd() Can you please take the cmdType from r.Config.CommandType where it is being used.

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

gouravkrosx · 2024-05-21T06:49:49Z

pkg/service/record/record.go

@@ -349,7 +349,7 @@ func (r *Recorder) ReRecord(ctx context.Context, appID uint64) error {

 	}
 	cmdType := utils.FindDockerCmd(r.config.Command)


Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

gouravkrosx · 2024-05-21T08:04:09Z

pkg/core/app/app.go

@@ -117,6 +119,16 @@ func (a *App) SetupDocker() error {
 		a.logger.Warn(fmt.Sprintf("given docker network:(%v) is different from parsed docker network:(%v)", a.containerNetwork, net))
 	}

+	if a.kind == utils.DockerStart {
+		running, err := a.docker.IsContainerRunning(a.container)


Even if the given container is different from the parsed container, we should provide the parsed container. i.e cont here. We are already logging a warning msg.

running, err := a.docker.IsContainerRunning(a.container)

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

…into docker-start-cmd

gouravkrosx

LGTM!

Yaxhveer and others added 7 commits February 19, 2024 23:41

fixed docker start command bug

287bec0

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

changes

9349a79

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

changes

c2379e8

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

[fix]: update permission to keploy directory if it exists (keploy#1585)

5f50192

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

chore: replace deprecated io/ioutil package (keploy#1350)

e2af67e

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

[refactor]: hide complete test summary in case of app not running (ke…

7294389

…ploy#1590) Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Dummy commit

a834839

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer force-pushed the docker-start-cmd branch from 79bcfca to a834839 Compare February 19, 2024 18:12

Yaxhveer and others added 3 commits February 20, 2024 20:17

code refactored

215d3cf

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

code refactored

4466e0a

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'main' into docker-start-cmd

ac0e8a3

charankamarapu reviewed Feb 22, 2024

View reviewed changes

Yaxhveer and others added 7 commits February 22, 2024 11:17

updated the changes

40516aa

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'main' into docker-start-cmd

2538a33

dummy commit

f3ddde9

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

dummy commit

24bf9b6

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

changes

5df1351

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

dummy commit

cc4e01f

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

linter issue fixed

cc61b00

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer requested a review from charankamarapu February 22, 2024 17:33

Yaxhveer and others added 2 commits February 23, 2024 18:32

Merge branch 'main' into docker-start-cmd

cf471f4

updated changes

6db5e69

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'main' into docker-start-cmd

618b9b0

gouravkrosx and others added 8 commits April 20, 2024 20:54

Merge branch 'main' into docker-start-cmd

a7cefef

refactored changes

cd2dc5d

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

fixed linter issue

0bc1384

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

fixed linter issue

4be8be6

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'main' into docker-start-cmd

5c33062

added parse command for docker start

bc07952

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'docker-start-cmd' of https://github.com/Yaxhveer/keploy …

8c74e63

…into docker-start-cmd

fixed linter issue

7856a93

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Yaxhveer requested a review from gouravkrosx April 21, 2024 13:24

Merge branch 'main' into docker-start-cmd

f5fc4cb

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

gouravkrosx and others added 3 commits May 21, 2024 11:29

Merge branch 'main' into docker-start-cmd

57e1847

updated the changes

c6899ee

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'main' into docker-start-cmd

1d334e2

gouravkrosx reviewed May 21, 2024

View reviewed changes

added IsDockerKind function

fb2d240

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

gouravkrosx reviewed May 21, 2024

View reviewed changes

updated commandtype

7c37c68

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

gouravkrosx reviewed May 21, 2024

View reviewed changes

Yaxhveer and others added 2 commits May 21, 2024 12:38

updated commandtype

98d4c43

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'main' into docker-start-cmd

dcb8ee7

gouravkrosx self-requested a review May 21, 2024 07:48

gouravkrosx reviewed May 21, 2024

View reviewed changes

Yaxhveer added 2 commits May 21, 2024 14:36

corrected parse container

358cefa

Signed-off-by: Yaxhveer <yaxhcod@gmail.com>

Merge branch 'docker-start-cmd' of https://github.com/Yaxhveer/keploy …

0989f0a

…into docker-start-cmd

gouravkrosx approved these changes May 21, 2024

View reviewed changes

gouravkrosx merged commit 8f0abd6 into keploy:main May 21, 2024
18 checks passed

github-actions bot locked and limited conversation to collaborators May 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG]: Fixed docker start command bug #1586

[BUG]: Fixed docker start command bug #1586

Yaxhveer commented Feb 16, 2024 •

edited

Loading

Yaxhveer commented Feb 18, 2024

charankamarapu commented Feb 19, 2024

charankamarapu commented Feb 21, 2024

Yaxhveer commented Feb 21, 2024

charankamarapu left a comment

charankamarapu Feb 22, 2024

Yaxhveer Feb 22, 2024

charankamarapu Feb 22, 2024

Yaxhveer Feb 22, 2024

Yaxhveer commented Feb 24, 2024 •

edited

Loading

charankamarapu commented Feb 28, 2024 •

edited

Loading

Yaxhveer commented Feb 28, 2024 •

edited

Loading

Yaxhveer commented Apr 21, 2024

gouravkrosx commented Apr 29, 2024

Yaxhveer commented May 13, 2024

gouravkrosx May 21, 2024

Yaxhveer May 21, 2024

gouravkrosx May 21, 2024

gouravkrosx May 21, 2024

gouravkrosx May 21, 2024 •

edited

Loading

Yaxhveer May 21, 2024

gouravkrosx left a comment

		@@ -339,7 +339,7 @@ func (r *Replayer) RunTestSet(ctx context.Context, testSetID string, testRunID s

		cmdType := utils.FindDockerCmd(r.config.Command)

		@@ -349,7 +349,7 @@ func (r *Recorder) ReRecord(ctx context.Context, appID uint64) error {

		}
		cmdType := utils.FindDockerCmd(r.config.Command)

[BUG]: Fixed docker start command bug #1586

[BUG]: Fixed docker start command bug #1586

Conversation

Yaxhveer commented Feb 16, 2024 • edited Loading

Related Issue

Describe the changes you've made

Type of change

Please let us know if any test cases are added

Describe if there is any unusual behaviour of your code(Write NA if there isn't)

Checklist:

Screenshots (if any)

Yaxhveer commented Feb 18, 2024

charankamarapu commented Feb 19, 2024

charankamarapu commented Feb 21, 2024

Yaxhveer commented Feb 21, 2024

charankamarapu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yaxhveer commented Feb 24, 2024 • edited Loading

charankamarapu commented Feb 28, 2024 • edited Loading

Yaxhveer commented Feb 28, 2024 • edited Loading

Yaxhveer commented Apr 21, 2024

gouravkrosx commented Apr 29, 2024

Yaxhveer commented May 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gouravkrosx May 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gouravkrosx left a comment

Choose a reason for hiding this comment

Yaxhveer commented Feb 16, 2024 •

edited

Loading

Describe if there is any unusual behaviour of your code(Write `NA` if there isn't)

Yaxhveer commented Feb 24, 2024 •

edited

Loading

charankamarapu commented Feb 28, 2024 •

edited

Loading

Yaxhveer commented Feb 28, 2024 •

edited

Loading

gouravkrosx May 21, 2024 •

edited

Loading