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
This commit is contained in:
Philippe Martin
2023-09-20 14:20:53 +02:00
committed by GitHub
parent 1ab0178cef
commit 0f828ec99f
32 changed files with 1335 additions and 76 deletions

View File

@@ -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)
}

View File

@@ -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)

View File

@@ -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 {

View File

@@ -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)
}

View File

@@ -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
}

View File

@@ -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
}

View File

@@ -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()))
}

View File

@@ -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
}

View File

@@ -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
}

View File

@@ -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)
}