From 0f828ec99f95a5866af2863697320bf1e6e710c3 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 20 Sep 2023 14:20:53 +0200 Subject: [PATCH] Ignore devstate when existing process name is not odo + delete devstate files with `odo delete component` (#7090) * Ignore devstate when existing process name is not odo * Delete orphan devstate files with odo delete component * Update unit tests * Create fake system * Add unit tests for odo delete component * Integration tests for odo dev * Troubleshooting * First process on Windows is 4 * Use go-ps lib for pidExists --- cmd/odo/common_test.go | 2 + cmd/odo/delete_test.go | 92 ++++++- docs/website/docs/troubleshooting.md | 24 ++ go.mod | 1 + go.sum | 2 + pkg/odo/cli/delete/component/component.go | 33 ++- .../cli/delete/component/component_test.go | 78 +++++- .../genericclioptions/clientset/clientset.go | 15 +- pkg/state/interface.go | 2 + pkg/state/process_unix.go | 38 --- pkg/state/process_windows.go | 17 -- pkg/state/state.go | 99 ++++++- pkg/testingutil/system/default.go | 24 ++ pkg/testingutil/system/fake.go | 41 +++ pkg/testingutil/system/system.go | 8 + tests/helper/helper_dev.go | 8 + tests/helper/helper_nowindows.go | 4 + tests/helper/helper_windows.go | 4 + tests/integration/cmd_dev_test.go | 30 ++ vendor/github.com/mitchellh/go-ps/.gitignore | 1 + vendor/github.com/mitchellh/go-ps/.travis.yml | 4 + vendor/github.com/mitchellh/go-ps/LICENSE.md | 21 ++ vendor/github.com/mitchellh/go-ps/README.md | 34 +++ vendor/github.com/mitchellh/go-ps/Vagrantfile | 43 +++ vendor/github.com/mitchellh/go-ps/process.go | 40 +++ .../mitchellh/go-ps/process_darwin.go | 138 ++++++++++ .../mitchellh/go-ps/process_freebsd.go | 260 ++++++++++++++++++ .../mitchellh/go-ps/process_linux.go | 35 +++ .../mitchellh/go-ps/process_solaris.go | 96 +++++++ .../mitchellh/go-ps/process_unix.go | 95 +++++++ .../mitchellh/go-ps/process_windows.go | 119 ++++++++ vendor/modules.txt | 3 + 32 files changed, 1335 insertions(+), 76 deletions(-) delete mode 100644 pkg/state/process_unix.go delete mode 100644 pkg/state/process_windows.go create mode 100644 pkg/testingutil/system/default.go create mode 100644 pkg/testingutil/system/fake.go create mode 100644 pkg/testingutil/system/system.go create mode 100644 vendor/github.com/mitchellh/go-ps/.gitignore create mode 100644 vendor/github.com/mitchellh/go-ps/.travis.yml create mode 100644 vendor/github.com/mitchellh/go-ps/LICENSE.md create mode 100644 vendor/github.com/mitchellh/go-ps/README.md create mode 100644 vendor/github.com/mitchellh/go-ps/Vagrantfile create mode 100644 vendor/github.com/mitchellh/go-ps/process.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_darwin.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_freebsd.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_linux.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_solaris.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_unix.go create mode 100644 vendor/github.com/mitchellh/go-ps/process_windows.go diff --git a/cmd/odo/common_test.go b/cmd/odo/common_test.go index d4e58da31..a2bfb953f 100644 --- a/cmd/odo/common_test.go +++ b/cmd/odo/common_test.go @@ -15,6 +15,7 @@ import ( "github.com/redhat-developer/odo/pkg/config" envcontext "github.com/redhat-developer/odo/pkg/config/context" "github.com/redhat-developer/odo/pkg/odo/cli" + odocontext "github.com/redhat-developer/odo/pkg/odo/context" "github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset" "github.com/redhat-developer/odo/pkg/testingutil/filesystem" ) @@ -67,6 +68,7 @@ func runCommand( t.Fatal(err) } ctx = envcontext.WithEnvConfig(ctx, *envConfig) + ctx = odocontext.WithPID(ctx, 101) for k, v := range options.env { t.Setenv(k, v) diff --git a/cmd/odo/delete_test.go b/cmd/odo/delete_test.go index ab97ec963..099e56b46 100644 --- a/cmd/odo/delete_test.go +++ b/cmd/odo/delete_test.go @@ -1,7 +1,9 @@ package main import ( + "encoding/json" "fmt" + "os" "path/filepath" "strings" "testing" @@ -14,8 +16,10 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "github.com/redhat-developer/odo/pkg/kclient" + "github.com/redhat-developer/odo/pkg/odo/commonflags" "github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset" "github.com/redhat-developer/odo/pkg/podman" + "github.com/redhat-developer/odo/pkg/state" "github.com/redhat-developer/odo/pkg/testingutil/filesystem" "github.com/redhat-developer/odo/pkg/util" "github.com/redhat-developer/odo/tests/helper" @@ -95,8 +99,9 @@ var nodeJsSourcesFsContent fscontentFunc = func(fs filesystem.Filesystem) error } type fsOptions struct { - dotOdoExists bool - generated []string + dotOdoExists bool + generated []string + devstateFiles []state.Content } var nodeJsSourcesAndDevfileFsContent = func(devfilePath string, options fsOptions) fscontentFunc { @@ -114,6 +119,21 @@ var nodeJsSourcesAndDevfileFsContent = func(devfilePath string, options fsOption gomega.Expect(err).NotTo(gomega.HaveOccurred()) } + for _, devstateFile := range options.devstateFiles { + jsonContent, err := json.MarshalIndent(devstateFile, "", " ") + if err != nil { + return err + } + dir := filepath.Dir(util.DotOdoDirectory) + err = os.MkdirAll(dir, 0750) + if err != nil { + return err + } + err = fs.WriteFile(fmt.Sprintf("./.odo/devstate.%d.json", devstateFile.PID), jsonContent, 0644) + if err != nil { + return err + } + } return nil } } @@ -250,9 +270,9 @@ func TestOdoDeleteMatrix(t *testing.T) { nameOptions map[string]nameOption wantErr string - checkOutput func(t *testing.T, s string) + checkOutput func(t *testing.T, testContext testContext, s string) checkFS func(t *testing.T, fs filesystem.Filesystem) - checkCalls func(t *testing.T, clientset clientset.Clientset, tetsContext testContext) + checkCalls func(t *testing.T, clientset clientset.Clientset, testContext testContext) }{ { name: "delete component when Devfile is not present in the directory", @@ -324,7 +344,7 @@ func TestOdoDeleteMatrix(t *testing.T) { "no": noNameOptions, }, - checkOutput: func(t *testing.T, s string) { + checkOutput: func(t *testing.T, testContext testContext, s string) { gomega.Expect(s).ToNot(gomega.ContainSubstring("devfile.yaml"), "should not list the devfile.yaml") }, checkFS: func(t *testing.T, fs filesystem.Filesystem) { @@ -353,7 +373,7 @@ func TestOdoDeleteMatrix(t *testing.T) { "no": noNameOptions, }, - checkOutput: func(t *testing.T, s string) { + checkOutput: func(t *testing.T, testContext testContext, s string) { gomega.Expect(s).To(gomega.ContainSubstring("devfile.yaml"), "should list the devfile.yaml") }, checkFS: func(t *testing.T, fs filesystem.Filesystem) { @@ -379,6 +399,17 @@ func TestOdoDeleteMatrix(t *testing.T) { fsOptions{ generated: []string{"devfile.yaml"}, }), + "nodeJS sources, Devfile and orphan devstate file": nodeJsSourcesAndDevfileFsContent( + filepath.Join("source", "devfiles", "nodejs", "devfile.yaml"), + fsOptions{ + dotOdoExists: true, + devstateFiles: []state.Content{ + { + PID: 43, + Platform: commonflags.PlatformPodman, + }, + }, + }), }, runningInOptions: allRunningInOptions, filesOptions: allFilesOptions, @@ -386,8 +417,14 @@ func TestOdoDeleteMatrix(t *testing.T) { "no": noNameOptions, }, - checkOutput: func(t *testing.T, s string) { + checkOutput: func(t *testing.T, testContext testContext, s string) { gomega.Expect(s).To(gomega.ContainSubstring("No resource found for component %q", "my-component")) + if testContext.fscontent == "nodeJS sources, Devfile and orphan devstate file" && + testContext.runningInOption != "deploy" { + gomega.Expect(s).To(gomega.ContainSubstring("devstate.43.json")) + } else { + gomega.Expect(s).To(gomega.Not(gomega.ContainSubstring("devstate.43.json"))) + } }, checkCalls: checkCallsNonDeployedComponent, }, @@ -411,6 +448,17 @@ func TestOdoDeleteMatrix(t *testing.T) { fsOptions{ generated: []string{"devfile.yaml"}, }), + "nodeJS sources, Devfile and orphan devstate file": nodeJsSourcesAndDevfileFsContent( + filepath.Join("source", "devfiles", "nodejs", "devfile.yaml"), + fsOptions{ + dotOdoExists: true, + devstateFiles: []state.Content{ + { + PID: 43, + Platform: commonflags.PlatformPodman, + }, + }, + }), }, runningInOptions: map[string]runningInOption{ "no": noRunningInOption, @@ -421,9 +469,15 @@ func TestOdoDeleteMatrix(t *testing.T) { "no": noNameOptions, }, - checkOutput: func(t *testing.T, s string) { + checkOutput: func(t *testing.T, testContext testContext, s string) { gomega.Expect(s).To(gomega.ContainSubstring("The following pods and associated volumes will get deleted from podman")) gomega.Expect(s).To(gomega.ContainSubstring("- my-component-app")) + if testContext.fscontent == "nodeJS sources, Devfile and orphan devstate file" && + testContext.runningInOption != "deploy" { + gomega.Expect(s).To(gomega.ContainSubstring("devstate.43.json")) + } else { + gomega.Expect(s).To(gomega.Not(gomega.ContainSubstring("devstate.43.json"))) + } }, checkCalls: checkCallsDeployedComponent, }, @@ -447,6 +501,17 @@ func TestOdoDeleteMatrix(t *testing.T) { fsOptions{ generated: []string{"devfile.yaml"}, }), + "nodeJS sources, Devfile and orphan devstate file": nodeJsSourcesAndDevfileFsContent( + filepath.Join("source", "devfiles", "nodejs", "devfile.yaml"), + fsOptions{ + dotOdoExists: true, + devstateFiles: []state.Content{ + { + PID: 43, + Platform: commonflags.PlatformPodman, + }, + }, + }), }, runningInOptions: map[string]runningInOption{ "no": noRunningInOption, @@ -457,9 +522,15 @@ func TestOdoDeleteMatrix(t *testing.T) { "no": noNameOptions, }, - checkOutput: func(t *testing.T, s string) { + checkOutput: func(t *testing.T, testContext testContext, s string) { gomega.Expect(s).To(gomega.ContainSubstring("The following resources will get deleted from cluster")) gomega.Expect(s).To(gomega.ContainSubstring("- Deployment: my-component-app")) + if testContext.fscontent == "nodeJS sources, Devfile and orphan devstate file" && + testContext.runningInOption != "deploy" { + gomega.Expect(s).To(gomega.ContainSubstring("devstate.43.json")) + } else { + gomega.Expect(s).To(gomega.Not(gomega.ContainSubstring("devstate.43.json"))) + } }, checkCalls: checkCallsDeployedComponent, }, @@ -514,6 +585,7 @@ func TestOdoDeleteMatrix(t *testing.T) { t.Fatalf(message) }) clientset := clientset.Clientset{} + clientset.FS = filesystem.DefaultFs{} env := map[string]string{} config := map[string]string{} platformFunc(t, env, config, &clientset) @@ -535,7 +607,7 @@ func TestOdoDeleteMatrix(t *testing.T) { } } if tt.checkOutput != nil { - tt.checkOutput(t, stdout) + tt.checkOutput(t, testCtx, stdout) } if tt.checkFS != nil { diff --git a/docs/website/docs/troubleshooting.md b/docs/website/docs/troubleshooting.md index f4a1df0e4..a3aba18a6 100644 --- a/docs/website/docs/troubleshooting.md +++ b/docs/website/docs/troubleshooting.md @@ -188,6 +188,30 @@ Various factors are responsible for this: Please refer to [Troubleshoot Storage Permission issues on managed cloud providers clusters](./user-guides/advanced/using-odo-with-other-clusters.md) for possible solutions to fix this. +### Orphan Devstate files + +An `odo dev` session creates a `.odo/devstate..json` file when the session starts, and deletes it at the end of the session. + +If the session terminates abrupty, the state file won't be deleted, and will remain in the `.odo` directory. + +You can delete such orphan devstate files using the command `odo delete component`. + +
+ Example output + +```shell +$ odo delete component +Searching resources to delete, please wait... +This will delete "go" from podman. + • The following pods and associated volumes will get deleted from podman: + • - go-app + +This will delete the following files and directories: + - /home/user/projects/go/.odo/devstate.83932.json + - /home/user/projects/go/.odo/devstate.json +``` +
+ ## Podman Issues ### `odo` says it cannot access Podman diff --git a/go.mod b/go.mod index fd904c04b..ec98c85bc 100644 --- a/go.mod +++ b/go.mod @@ -151,6 +151,7 @@ require ( github.com/mattn/go-runewidth v0.0.13 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b // indirect + github.com/mitchellh/go-ps v1.0.0 // indirect github.com/mitchellh/go-wordwrap v1.0.0 // indirect github.com/mitchellh/reflectwalk v1.0.1 // indirect github.com/moby/buildkit v0.11.6 // indirect diff --git a/go.sum b/go.sum index 596b5de1e..68bdc282f 100644 --- a/go.sum +++ b/go.sum @@ -882,6 +882,8 @@ github.com/mistifyio/go-zfs v2.1.2-0.20190413222219-f784269be439+incompatible/go github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc= github.com/mitchellh/go-homedir v1.0.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= +github.com/mitchellh/go-ps v1.0.0 h1:i6ampVEEF4wQFF+bkYfwYgY+F/uYJDktmvLPf7qIgjc= +github.com/mitchellh/go-ps v1.0.0/go.mod h1:J4lOc8z8yJs6vUwklHw2XEIiT4z4C40KtWVN3nvg8Pg= github.com/mitchellh/go-testing-interface v1.0.0/go.mod h1:kRemZodwjscx+RGhAo8eIhFbs2+BFgRtFPeD/KE+zxI= github.com/mitchellh/go-wordwrap v1.0.0 h1:6GlHJ/LTGMrIJbwgdqdl2eEH8o+Exx/0m8ir9Gns0u4= github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo= diff --git a/pkg/odo/cli/delete/component/component.go b/pkg/odo/cli/delete/component/component.go index 4494df87e..18f60246f 100644 --- a/pkg/odo/cli/delete/component/component.go +++ b/pkg/odo/cli/delete/component/component.go @@ -310,9 +310,14 @@ func (o *ComponentOptions) deleteDevfileComponent(ctx context.Context) ([]unstru hasPodmanResources = len(podmanPods) != 0 } + orphans, err := o.getOrphanDevstateFiles(o.clientset.FS, ctx) + if err != nil { + return nil, err + } + if !(hasClusterResources || hasPodmanResources) { log.Finfof(o.clientset.Stdout, messageWithPlatforms(o.clientset.KubernetesClient != nil, o.clientset.PodmanClient != nil, componentName, namespace)) - if !o.withFilesFlag { + if !o.withFilesFlag && len(orphans) == 0 { // check for resources here return remainingResources, nil } @@ -326,9 +331,15 @@ func (o *ComponentOptions) deleteDevfileComponent(ctx context.Context) ([]unstru if err != nil { return nil, err } + } + + filesToDelete = append(filesToDelete, orphans...) + + hasFilesToDelete := len(filesToDelete) != 0 + + if hasFilesToDelete { o.printFileCreatedByOdo(filesToDelete, hasClusterResources) } - hasFilesToDelete := len(filesToDelete) != 0 if !(hasClusterResources || hasPodmanResources || hasFilesToDelete) { klog.V(2).Info("no cluster resources and no files to delete") @@ -386,7 +397,7 @@ func (o *ComponentOptions) deleteDevfileComponent(ctx context.Context) ([]unstru log.Finfof(o.clientset.Stdout, "The component %q is successfully deleted from podman", componentName) } - if o.withFilesFlag { + if o.withFilesFlag || len(orphans) > 0 { // Delete files remainingFiles := o.deleteFilesCreatedByOdo(o.clientset.FS, filesToDelete) var listOfFiles []string @@ -495,6 +506,20 @@ func getFilesCreatedByOdo(filesys filesystem.Filesystem, ctx context.Context) ([ return list, nil } +// getOrphanDevstateFiles gets the list of all Devstate files for which no odo process exists +func (o *ComponentOptions) getOrphanDevstateFiles(filesys filesystem.Filesystem, ctx context.Context) ([]string, error) { + var list []string + if o.runningIn != labels.ComponentDeployMode { + var orphanDevstates []string + orphanDevstates, err := o.clientset.StateClient.GetOrphanFiles(ctx) + if err != nil { + return nil, err + } + list = append(list, orphanDevstates...) + } + return list, nil +} + func (o *ComponentOptions) printFileCreatedByOdo(files []string, hasClusterResources bool) { if len(files) == 0 { return @@ -544,7 +569,7 @@ func NewCmdComponent(ctx context.Context, name, fullName string, testClientset c componentCmd.Flags().BoolVarP(&o.withFilesFlag, "files", "", false, "Delete all files and directories generated by odo. Use with caution.") componentCmd.Flags().BoolVarP(&o.forceFlag, "force", "f", false, "Delete component without prompting") componentCmd.Flags().BoolVarP(&o.waitFlag, "wait", "w", false, "Wait for deletion of all dependent resources") - clientset.Add(componentCmd, clientset.DELETE_COMPONENT, clientset.KUBERNETES, clientset.FILESYSTEM) + clientset.Add(componentCmd, clientset.DELETE_COMPONENT, clientset.KUBERNETES, clientset.FILESYSTEM, clientset.STATE) if feature.IsEnabled(ctx, feature.GenericPlatformFlag) { clientset.Add(componentCmd, clientset.PODMAN_NULLABLE) } diff --git a/pkg/odo/cli/delete/component/component_test.go b/pkg/odo/cli/delete/component/component_test.go index 8f43ef29f..1716dc84f 100644 --- a/pkg/odo/cli/delete/component/component_test.go +++ b/pkg/odo/cli/delete/component/component_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/devfile/library/v2/pkg/devfile/parser" - "github.com/devfile/library/v2/pkg/testingutil/filesystem" "github.com/golang/mock/gomock" "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" @@ -23,7 +22,10 @@ import ( odocontext "github.com/redhat-developer/odo/pkg/odo/context" "github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset" "github.com/redhat-developer/odo/pkg/podman" + "github.com/redhat-developer/odo/pkg/state" "github.com/redhat-developer/odo/pkg/testingutil" + "github.com/redhat-developer/odo/pkg/testingutil/filesystem" + "github.com/redhat-developer/odo/pkg/testingutil/system" ) func TestComponentOptions_deleteNamedComponent(t *testing.T) { @@ -420,6 +422,7 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { remainingResources []unstructured.Unstructured wantErr bool deleteClient func(ctrl *gomock.Controller) _delete.Client + stateClient func(ctrl *gomock.Controller) state.Client }{ { name: "deleting a component with access to devfile", @@ -433,6 +436,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().DeleteResources(resources, false).Return([]unstructured.Unstructured{}) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, }, @@ -450,6 +458,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().DeleteResources(resources, false).Return([]unstructured.Unstructured{}) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, runningIn: labels.ComponentDevMode, @@ -468,6 +481,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().DeleteResources(resources, false).Return([]unstructured.Unstructured{}) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, runningIn: labels.ComponentDeployMode, @@ -484,6 +502,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { Return(resources, nil) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, runningIn: labels.ComponentDeployMode, @@ -501,6 +524,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().DeleteResources(resources, false).Return(nil) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, }, @@ -513,6 +541,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().ListClusterResourcesToDeleteFromDevfile(gomock.Any(), appName, gomock.Any(), labels.ComponentAnyMode).Return(false, nil, errors.New("some error")) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, }, @@ -525,6 +558,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().ListClusterResourcesToDeleteFromDevfile(gomock.Any(), appName, gomock.Any(), labels.ComponentDevMode).Return(false, nil, errors.New("some error")) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, runningIn: labels.ComponentDevMode, @@ -538,6 +576,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().ListClusterResourcesToDeleteFromDevfile(gomock.Any(), appName, gomock.Any(), labels.ComponentDeployMode).Return(false, nil, errors.New("some error")) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, runningIn: labels.ComponentDeployMode, @@ -552,6 +595,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().ListClusterResourcesToDelete(gomock.Any(), gomock.Any(), gomock.Any(), labels.ComponentAnyMode) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: false, }, @@ -565,6 +613,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().ListClusterResourcesToDelete(gomock.Any(), gomock.Any(), gomock.Any(), labels.ComponentDevMode) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: false, runningIn: labels.ComponentDevMode, @@ -579,6 +632,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().ListClusterResourcesToDelete(gomock.Any(), gomock.Any(), gomock.Any(), labels.ComponentDeployMode) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: false, runningIn: labels.ComponentDeployMode, @@ -594,6 +652,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().ListClusterResourcesToDelete(gomock.Any(), gomock.Any(), gomock.Any(), labels.ComponentAnyMode) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, }, @@ -608,6 +671,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().ListClusterResourcesToDelete(gomock.Any(), gomock.Any(), gomock.Any(), labels.ComponentDevMode) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, runningIn: labels.ComponentDevMode, @@ -623,6 +691,11 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { deleteClient.EXPECT().ListClusterResourcesToDelete(gomock.Any(), gomock.Any(), gomock.Any(), labels.ComponentDeployMode) return deleteClient }, + stateClient: func(ctrl *gomock.Controller) state.Client { + fs := filesystem.NewFakeFs() + system := system.Default{} + return state.NewStateClient(fs, system) + }, fields: fields{ forceFlag: true, runningIn: labels.ComponentDeployMode, @@ -640,6 +713,7 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { ctrl := gomock.NewController(t) kubeClient := prepareKubeClient(ctrl, projectName) deleteClient := tt.deleteClient(ctrl) + stateClient := tt.stateClient(ctrl) o := &ComponentOptions{ name: tt.fields.name, forceFlag: tt.fields.forceFlag, @@ -649,6 +723,7 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { Stderr: os.Stderr, KubernetesClient: kubeClient, DeleteClient: deleteClient, + StateClient: stateClient, }, } ctx := odocontext.WithNamespace(context.Background(), projectName) @@ -656,6 +731,7 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) { ctx = odocontext.WithWorkingDirectory(ctx, workingDir) ctx = odocontext.WithComponentName(ctx, compName) ctx = odocontext.WithEffectiveDevfileObj(ctx, &info) + ctx = odocontext.WithPID(ctx, 101) remainingResources, err := o.deleteDevfileComponent(ctx) if (err != nil) != tt.wantErr { t.Errorf("deleteDevfileComponent() error = %v, wantErr %v", err, tt.wantErr) diff --git a/pkg/odo/genericclioptions/clientset/clientset.go b/pkg/odo/genericclioptions/clientset/clientset.go index 49b68d833..b75bd8d43 100644 --- a/pkg/odo/genericclioptions/clientset/clientset.go +++ b/pkg/odo/genericclioptions/clientset/clientset.go @@ -44,6 +44,7 @@ import ( "github.com/redhat-developer/odo/pkg/project" "github.com/redhat-developer/odo/pkg/registry" "github.com/redhat-developer/odo/pkg/testingutil/filesystem" + "github.com/redhat-developer/odo/pkg/testingutil/system" "github.com/redhat-developer/odo/pkg/watch" ) @@ -90,6 +91,8 @@ const ( STATE = "DEP_STATE" // SYNC instantiates client for pkg/sync SYNC = "DEP_SYNC" + // SYSTEM instantiates client for pkg/testingutil/system + SYSTEM = "DEP_SYSTEM" // WATCH instantiates client for pkg/watch WATCH = "DEP_WATCH" /* Add key for new package here */ @@ -122,7 +125,7 @@ var subdeps map[string][]string = map[string][]string{ PORT_FORWARD: {KUBERNETES_NULLABLE, EXEC, STATE}, PROJECT: {KUBERNETES}, REGISTRY: {FILESYSTEM, PREFERENCE, KUBERNETES_NULLABLE}, - STATE: {FILESYSTEM}, + STATE: {FILESYSTEM, SYSTEM}, SYNC: {EXEC}, WATCH: {INFORMER, KUBERNETES_NULLABLE}, BINDING: {PROJECT, KUBERNETES_NULLABLE}, @@ -152,6 +155,7 @@ type Clientset struct { RegistryClient registry.Client StateClient state.Client SyncClient sync.Client + systemClient system.System WatchClient watch.Client /* Add client by alphabetic order */ } @@ -201,6 +205,13 @@ func Fetch(command *cobra.Command, platform string, testClientset Clientset) (*C dep.FS = filesystem.DefaultFs{} } } + if isDefined(command, SYSTEM) { + if testClientset.systemClient != nil { + dep.systemClient = testClientset.systemClient + } else { + dep.systemClient = system.Default{} + } + } if isDefined(command, INFORMER) { dep.InformerClient = informer.NewInformerClient() } @@ -289,7 +300,7 @@ func Fetch(command *cobra.Command, platform string, testClientset Clientset) (*C dep.ProjectClient = project.NewClient(dep.KubernetesClient) } if isDefined(command, STATE) { - dep.StateClient = state.NewStateClient(dep.FS) + dep.StateClient = state.NewStateClient(dep.FS, dep.systemClient) } if isDefined(command, SYNC) { switch platform { diff --git a/pkg/state/interface.go b/pkg/state/interface.go index bfec0558c..7079cef7c 100644 --- a/pkg/state/interface.go +++ b/pkg/state/interface.go @@ -24,4 +24,6 @@ type Client interface { // GetAPIServerPorts returns the port where the API servers are listening, possibly per platform. GetAPIServerPorts(ctx context.Context) ([]api.DevControlPlane, error) + + GetOrphanFiles(ctx context.Context) ([]string, error) } diff --git a/pkg/state/process_unix.go b/pkg/state/process_unix.go deleted file mode 100644 index 988f16770..000000000 --- a/pkg/state/process_unix.go +++ /dev/null @@ -1,38 +0,0 @@ -//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris || zos -// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris zos - -package state - -import ( - "fmt" - "os" - "syscall" -) - -func pidExists(pid int) (bool, error) { - if pid <= 0 { - return false, fmt.Errorf("invalid pid %v", pid) - } - proc, err := os.FindProcess(pid) - if err != nil { - return false, nil - } - err = proc.Signal(syscall.Signal(0)) - if err == nil { - return true, nil - } - if err.Error() == "os: process already finished" { - return false, nil - } - errno, ok := err.(syscall.Errno) - if !ok { - return false, err - } - switch errno { - case syscall.ESRCH: - return false, nil - case syscall.EPERM: - return true, nil - } - return false, err -} diff --git a/pkg/state/process_windows.go b/pkg/state/process_windows.go deleted file mode 100644 index 890b93909..000000000 --- a/pkg/state/process_windows.go +++ /dev/null @@ -1,17 +0,0 @@ -package state - -import ( - "fmt" - "os" -) - -func pidExists(pid int) (bool, error) { - if pid <= 0 { - return false, fmt.Errorf("invalid pid %v", pid) - } - _, err := os.FindProcess(pid) - if err != nil { - return false, nil - } - return true, nil -} diff --git a/pkg/state/state.go b/pkg/state/state.go index 332ba5bad..8840b455c 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -10,24 +10,30 @@ import ( "path/filepath" "regexp" + "github.com/mitchellh/go-ps" + "k8s.io/klog" + "github.com/redhat-developer/odo/pkg/api" "github.com/redhat-developer/odo/pkg/odo/cli/feature" "github.com/redhat-developer/odo/pkg/odo/commonflags" fcontext "github.com/redhat-developer/odo/pkg/odo/commonflags/context" odocontext "github.com/redhat-developer/odo/pkg/odo/context" "github.com/redhat-developer/odo/pkg/testingutil/filesystem" + "github.com/redhat-developer/odo/pkg/testingutil/system" ) type State struct { content Content fs filesystem.Filesystem + system system.System } var _ Client = (*State)(nil) -func NewStateClient(fs filesystem.Filesystem) *State { +func NewStateClient(fs filesystem.Filesystem, system system.System) *State { return &State{ - fs: fs, + fs: fs, + system: system, } } @@ -246,7 +252,7 @@ func (o *State) isFreeOrOwnedBy(pid int) (bool, error) { return true, nil } - exists, err := pidExists(savedContent.PID) + exists, err := o.system.PidExists(savedContent.PID) if err != nil { return false, err } @@ -292,14 +298,99 @@ func (o *State) checkFirstInPlatform(ctx context.Context) error { if content.PID == pid { continue } - exists, err := pidExists(content.PID) + exists, err := o.system.PidExists(content.PID) if err != nil { return err } if exists { + var process ps.Process + process, err = o.system.FindProcess(content.PID) + if err != nil { + klog.V(4).Infof("process %d exists but is not accessible, ignoring", content.PID) + continue + } + if process.Executable() != "odo" && process.Executable() != "odo.exe" { + klog.V(4).Infof("process %d exists but is not odo, ignoring", content.PID) + continue + } // Process exists => problem return NewErrAlreadyRunningOnPlatform(platform, content.PID) } } return nil } + +func (o *State) GetOrphanFiles(ctx context.Context) ([]string, error) { + var ( + pid = odocontext.GetPID(ctx) + result []string + ) + + re := regexp.MustCompile(`^devstate\.?[0-9]*\.json$`) + entries, err := o.fs.ReadDir(_dirpath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + // No file found => no orphan files + return nil, nil + } + return nil, err + } + for _, entry := range entries { + if !re.MatchString(entry.Name()) { + continue + } + filename, err := getFullFilename(entry) + if err != nil { + return nil, err + } + + jsonContent, err := o.fs.ReadFile(filepath.Join(_dirpath, entry.Name())) + if err != nil { + return nil, err + } + var content Content + // Ignore error, to handle empty file + _ = json.Unmarshal(jsonContent, &content) + + if content.PID == pid { + continue + } + if content.PID == 0 { + // This is devstate.json with pid=0 + continue + } + exists, err := o.system.PidExists(content.PID) + if err != nil { + return nil, err + } + if exists { + var process ps.Process + process, err = o.system.FindProcess(content.PID) + if err != nil { + klog.V(4).Infof("process %d exists but is not accessible => orphan", content.PID) + result = append(result, filename) + continue + } + if process == nil { + klog.V(4).Infof("process %d does not exist => orphan", content.PID) + result = append(result, filename) + continue + } + if process.Executable() != "odo" && process.Executable() != "odo.exe" { + klog.V(4).Infof("process %d exists but is not odo => orphan", content.PID) + result = append(result, filename) + continue + } + // Process exists => not orphan + klog.V(4).Infof("process %d exists and is odo => not orphan", content.PID) + continue + } + klog.V(4).Infof("process %d does not exist => orphan", content.PID) + result = append(result, filename) + } + return result, nil +} + +func getFullFilename(entry fs.FileInfo) (string, error) { + return filepath.Abs(filepath.Join(_dirpath, entry.Name())) +} diff --git a/pkg/testingutil/system/default.go b/pkg/testingutil/system/default.go new file mode 100644 index 000000000..2ffba69de --- /dev/null +++ b/pkg/testingutil/system/default.go @@ -0,0 +1,24 @@ +package system + +import "github.com/mitchellh/go-ps" + +type Default struct{} + +var _ System = Default{} + +func (o Default) FindProcess(pid int) (ps.Process, error) { + return ps.FindProcess(pid) +} + +func (o Default) PidExists(pid int) (bool, error) { + processes, err := ps.Processes() + if err != nil { + return false, err + } + for _, process := range processes { + if process.Pid() == pid { + return true, nil + } + } + return false, nil +} diff --git a/pkg/testingutil/system/fake.go b/pkg/testingutil/system/fake.go new file mode 100644 index 000000000..048740c2e --- /dev/null +++ b/pkg/testingutil/system/fake.go @@ -0,0 +1,41 @@ +package system + +import ( + "errors" + + "github.com/mitchellh/go-ps" +) + +type Fake struct { + ProcessId int + ParentId int + // PidTable is a map of pid => executable name of existing processes + PidTable map[int]string +} + +func (o Fake) Pid() int { + return o.ProcessId +} + +func (o Fake) PPid() int { + return o.ParentId +} + +func (o Fake) Executable() string { + return o.PidTable[o.ProcessId] +} + +var _ System = Fake{} + +func (o Fake) FindProcess(pid int) (ps.Process, error) { + if _, found := o.PidTable[pid]; found { + o.ProcessId = pid + return o, nil + } + return nil, errors.New("no process found") +} + +func (o Fake) PidExists(pid int) (bool, error) { + _, found := o.PidTable[pid] + return found, nil +} diff --git a/pkg/testingutil/system/system.go b/pkg/testingutil/system/system.go new file mode 100644 index 000000000..3d169c753 --- /dev/null +++ b/pkg/testingutil/system/system.go @@ -0,0 +1,8 @@ +package system + +import "github.com/mitchellh/go-ps" + +type System interface { + FindProcess(pid int) (ps.Process, error) + PidExists(pid int) (bool, error) +} diff --git a/tests/helper/helper_dev.go b/tests/helper/helper_dev.go index b66e2090a..1643b924e 100644 --- a/tests/helper/helper_dev.go +++ b/tests/helper/helper_dev.go @@ -133,6 +133,7 @@ type DevSessionOpts struct { APIServerPort int SyncGitDir bool ShowLogs bool + VerboseLevel string } // StartDevMode starts a dev session with `odo dev` @@ -174,6 +175,9 @@ func StartDevMode(options DevSessionOpts) (devSession DevSession, err error) { if options.ShowLogs { args = append(args, "--logs") } + if options.VerboseLevel != "" { + args = append(args, "-v", options.VerboseLevel) + } args = append(args, options.CmdlineArgs...) cmd := Cmd("odo", args...) cmd.Cmd.Stdin = c.Tty() @@ -220,6 +224,10 @@ func (o DevSession) Kill() { o.session.Kill() } +func (o DevSession) PID() int { + return o.session.Command.Process.Pid +} + // Stop a Dev session cleanly (equivalent as hitting Ctrl-c) func (o *DevSession) Stop() { if o.session == nil { diff --git a/tests/helper/helper_nowindows.go b/tests/helper/helper_nowindows.go index 6088f782b..bcf37d1d4 100644 --- a/tests/helper/helper_nowindows.go +++ b/tests/helper/helper_nowindows.go @@ -15,3 +15,7 @@ func terminateProc(session *gexec.Session) error { } func setSysProcAttr(command *exec.Cmd) {} + +func GetFirstProcess() int { + return 1 +} diff --git a/tests/helper/helper_windows.go b/tests/helper/helper_windows.go index facbcb4a1..e39c63e53 100644 --- a/tests/helper/helper_windows.go +++ b/tests/helper/helper_windows.go @@ -35,3 +35,7 @@ func setSysProcAttr(command *exec.Cmd) { CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP, } } + +func GetFirstProcess() int { + return 4 +} diff --git a/tests/integration/cmd_dev_test.go b/tests/integration/cmd_dev_test.go index 57bc31268..8b76ae3d6 100644 --- a/tests/integration/cmd_dev_test.go +++ b/tests/integration/cmd_dev_test.go @@ -590,6 +590,36 @@ ComponentSettings: }) }) + When("killing odo dev and another process replaces it", func() { + var newDevSession helper.DevSession + BeforeEach(func() { + pid := devSession.PID() + devSession.Kill() + devSession.WaitEnd() + devstate := fmt.Sprintf(".odo/devstate.%d.json", pid) + newdevstate := fmt.Sprintf(".odo/devstate.%d.json", helper.GetFirstProcess()) + helper.ReplaceString( + devstate, + fmt.Sprintf("\"pid\": %d", pid), + fmt.Sprintf("\"pid\": %d", helper.GetFirstProcess())) + err := os.Rename(devstate, newdevstate) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should restart a new session successfully", func() { + var err error + newDevSession, err = helper.StartDevMode(helper.DevSessionOpts{ + VerboseLevel: "4", + }) + Expect(err).ToNot(HaveOccurred()) + logMsg := fmt.Sprintf("process %d exists but is not odo, ignoring", helper.GetFirstProcess()) + Expect(newDevSession.ErrOut).Should(ContainSubstring(logMsg)) + + newDevSession.Stop() + newDevSession.WaitEnd() + }) + }) + When("stopping odo dev normally", func() { BeforeEach(func() { devSession.Stop() diff --git a/vendor/github.com/mitchellh/go-ps/.gitignore b/vendor/github.com/mitchellh/go-ps/.gitignore new file mode 100644 index 000000000..a977916f6 --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/.gitignore @@ -0,0 +1 @@ +.vagrant/ diff --git a/vendor/github.com/mitchellh/go-ps/.travis.yml b/vendor/github.com/mitchellh/go-ps/.travis.yml new file mode 100644 index 000000000..8f794f71d --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/.travis.yml @@ -0,0 +1,4 @@ +language: go + +go: + - 1.2.1 diff --git a/vendor/github.com/mitchellh/go-ps/LICENSE.md b/vendor/github.com/mitchellh/go-ps/LICENSE.md new file mode 100644 index 000000000..229851590 --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/LICENSE.md @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2014 Mitchell Hashimoto + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/vendor/github.com/mitchellh/go-ps/README.md b/vendor/github.com/mitchellh/go-ps/README.md new file mode 100644 index 000000000..4e3d0e146 --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/README.md @@ -0,0 +1,34 @@ +# Process List Library for Go [![GoDoc](https://godoc.org/github.com/mitchellh/go-ps?status.png)](https://godoc.org/github.com/mitchellh/go-ps) + +go-ps is a library for Go that implements OS-specific APIs to list and +manipulate processes in a platform-safe way. The library can find and +list processes on Linux, Mac OS X, Solaris, and Windows. + +If you're new to Go, this library has a good amount of advanced Go educational +value as well. It uses some advanced features of Go: build tags, accessing +DLL methods for Windows, cgo for Darwin, etc. + +How it works: + + * **Darwin** uses the `sysctl` syscall to retrieve the process table. + * **Unix** uses the procfs at `/proc` to inspect the process tree. + * **Windows** uses the Windows API, and methods such as + `CreateToolhelp32Snapshot` to get a point-in-time snapshot of + the process table. + +## Installation + +Install using standard `go get`: + +``` +$ go get github.com/mitchellh/go-ps +... +``` + +## TODO + +Want to contribute? Here is a short TODO list of things that aren't +implemented for this library that would be nice: + + * FreeBSD support + * Plan9 support diff --git a/vendor/github.com/mitchellh/go-ps/Vagrantfile b/vendor/github.com/mitchellh/go-ps/Vagrantfile new file mode 100644 index 000000000..61662ab1e --- /dev/null +++ b/vendor/github.com/mitchellh/go-ps/Vagrantfile @@ -0,0 +1,43 @@ +# -*- mode: ruby -*- +# vi: set ft=ruby : + +# Vagrantfile API/syntax version. Don't touch unless you know what you're doing! +VAGRANTFILE_API_VERSION = "2" + +Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| + config.vm.box = "chef/ubuntu-12.04" + + config.vm.provision "shell", inline: $script + + ["vmware_fusion", "vmware_workstation"].each do |p| + config.vm.provider "p" do |v| + v.vmx["memsize"] = "1024" + v.vmx["numvcpus"] = "2" + v.vmx["cpuid.coresPerSocket"] = "1" + end + end +end + +$script = <