Skip to content

Commit

Permalink
chore: handle remove log file errors properly
Browse files Browse the repository at this point in the history
  • Loading branch information
mdelapenya committed Dec 16, 2024
1 parent a8824c0 commit 953518e
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 11 deletions.
19 changes: 8 additions & 11 deletions modules/ollama/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"net"
"os"
"os/exec"
Expand Down Expand Up @@ -496,18 +497,14 @@ func (c *OllamaContainer) Terminate(ctx context.Context) error {
return nil
}

// remove the log file if it exists
if _, err = os.Stat(c.localCtx.logFile.Name()); err == nil {
err = c.localCtx.logFile.Close()
if err != nil {
return err
}
var errs []error
if err = c.localCtx.logFile.Close(); err != nil {
errs = append(errs, fmt.Errorf("close log: %w", err))
}

err = os.Remove(c.localCtx.logFile.Name())
if err != nil {
return err
}
if err = os.Remove(c.localCtx.logFile.Name()); err != nil && !errors.Is(err, fs.ErrNotExist) {
errs = append(errs, fmt.Errorf("remove log: %w", err))
}

return nil
return errors.Join(errs...)
}
55 changes: 55 additions & 0 deletions modules/ollama/local_unit_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package ollama

import (
"context"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
)

func TestRun_localWithCustomLogFileError(t *testing.T) {
t.Run("terminate/close-log-error", func(t *testing.T) {
// Create a temporary file for testing
f, err := os.CreateTemp(t.TempDir(), "test-log-*")
require.NoError(t, err)

// Close the file before termination to force a "file already closed" error
err = f.Close()
require.NoError(t, err)

c := &OllamaContainer{
localCtx: &localContext{
logFile: f,
},
}
err = c.Terminate(context.Background())
require.Error(t, err)
require.ErrorContains(t, err, "close log:")
})

t.Run("terminate/log-file-not-removable", func(t *testing.T) {
// Create a temporary file for testing
f, err := os.CreateTemp(t.TempDir(), "test-log-*")
require.NoError(t, err)
defer func() {
// Cleanup: restore permissions
os.Chmod(filepath.Dir(f.Name()), 0700)
}()

// Make the file read-only and its parent directory read-only
// This should cause removal to fail on most systems
dir := filepath.Dir(f.Name())
require.NoError(t, os.Chmod(dir, 0500))

c := &OllamaContainer{
localCtx: &localContext{
logFile: f,
},
}
err = c.Terminate(context.Background())
require.Error(t, err)
require.ErrorContains(t, err, "remove log:")
})
}

0 comments on commit 953518e

Please sign in to comment.