Skip to content

Commit 118d1f0

Browse files
committed
just windows fix
1 parent 0782305 commit 118d1f0

1 file changed

Lines changed: 34 additions & 84 deletions

File tree

pkg/collect/result.go

Lines changed: 34 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"path/filepath"
1212
"runtime"
1313
"strings"
14-
"time"
1514

1615
"github.com/pkg/errors"
1716
"k8s.io/klog/v2"
@@ -191,7 +190,18 @@ func (r CollectorResult) ReplaceResult(bundlePath string, relativePath string, r
191190
}
192191

193192
// Create a temporary file in the same directory as the target file to prevent cross-device issues
194-
tmpFile, err := os.CreateTemp("", "replace-")
193+
var tmpFile *os.File
194+
var err error
195+
196+
if runtime.GOOS == "windows" {
197+
// Windows-only: Use destination directory to avoid antivirus issues
198+
destDir := filepath.Dir(filepath.Join(bundlePath, relativePath))
199+
os.MkdirAll(destDir, 0755)
200+
tmpFile, err = os.CreateTemp(destDir, "replace-")
201+
} else {
202+
// Linux/macOS: EXACT original behavior - system temp
203+
tmpFile, err = os.CreateTemp("", "replace-")
204+
}
195205
if err != nil {
196206
return errors.Wrap(err, "failed to create temp file")
197207
}
@@ -205,13 +215,19 @@ func (r CollectorResult) ReplaceResult(bundlePath string, relativePath string, r
205215
// Close the file to ensure all data is written
206216
tmpFile.Close()
207217

208-
// Use Windows-specific file replacement to handle file locking issues
209-
finalPath := filepath.Join(bundlePath, relativePath)
218+
// This rename should always be in /tmp, so no cross-partition copying will happen
210219
if runtime.GOOS == "windows" {
211-
err = replaceFileWindows(tmpFile.Name(), finalPath)
220+
// Windows-specific handling with delete-first approach
221+
finalPath := filepath.Join(bundlePath, relativePath)
222+
223+
// Delete target file first (Windows requirement)
224+
os.Remove(finalPath)
225+
226+
// Windows: Use copy+delete instead of rename (more reliable)
227+
err = copyFileWindows(tmpFile.Name(), finalPath)
212228
} else {
213-
// Linux/macOS: Keep original behavior (rename in /tmp, no cross-partition copying)
214-
err = os.Rename(tmpFile.Name(), finalPath)
229+
// Linux/macOS: EXACT original behavior - DO NOT CHANGE
230+
err = os.Rename(tmpFile.Name(), filepath.Join(bundlePath, relativePath))
215231
}
216232
if err != nil {
217233
return errors.Wrap(err, "failed to rename tmp file")
@@ -426,93 +442,27 @@ func TarSupportBundleDir(bundlePath string, input CollectorResult, outputFilenam
426442
return input.ArchiveBundle(bundlePath, outputFilename)
427443
}
428444

429-
// replaceFileWindows handles Windows-specific file replacement using copy+delete strategy
430-
// This approach is more reliable than os.Rename() on Windows systems with aggressive file scanning
431-
func replaceFileWindows(srcPath, dstPath string) error {
432-
const maxRetries = 15
433-
const baseDelay = 100 * time.Millisecond
434-
435-
klog.V(2).Infof("Windows file replacement (copy+delete): %s -> %s", srcPath, dstPath)
436-
437-
for attempt := 0; attempt < maxRetries; attempt++ {
438-
// Copy + delete approach for Windows file locking compatibility
439-
err := copyAndDeleteWindows(srcPath, dstPath)
440-
if err == nil {
441-
if attempt > 0 {
442-
klog.V(1).Infof("Windows file replacement succeeded after %d retries", attempt+1)
443-
}
444-
return nil // Success!
445-
}
446-
447-
// Check if it's a retryable Windows file locking error
448-
if isWindowsFileLockError(err) {
449-
if attempt < maxRetries-1 {
450-
// Exponential backoff: 100ms, 200ms, 400ms, 800ms, 1600ms...
451-
delay := baseDelay * time.Duration(1<<attempt)
452-
klog.V(2).Infof("Windows file lock detected (attempt %d/%d): %v - retrying in %v",
453-
attempt+1, maxRetries, err, delay)
454-
time.Sleep(delay)
455-
continue
456-
}
457-
}
458-
459-
// Non-retryable error or max retries reached
460-
klog.V(2).Infof("Windows file operation failed: %v", err)
461-
return errors.Wrap(err, "Windows file replacement failed")
462-
}
463-
464-
return errors.New("Windows file replacement failed after maximum retries")
465-
}
466-
467-
// copyAndDeleteWindows performs file replacement using copy + delete instead of rename
468-
func copyAndDeleteWindows(srcPath, dstPath string) error {
469-
// Delete target file if it exists
470-
if _, err := os.Stat(dstPath); err == nil {
471-
if removeErr := os.Remove(dstPath); removeErr != nil {
472-
return errors.Wrap(removeErr, "failed to remove existing target file")
473-
}
474-
}
475-
476-
// Copy source to destination
477-
srcFile, err := os.Open(srcPath)
445+
// copyFileWindows performs copy+delete for Windows file operations
446+
func copyFileWindows(src, dst string) error {
447+
srcFile, err := os.Open(src)
478448
if err != nil {
479-
return errors.Wrap(err, "failed to open source file")
449+
return err
480450
}
481451
defer srcFile.Close()
482452

483-
dstFile, err := os.Create(dstPath)
453+
dstFile, err := os.Create(dst)
484454
if err != nil {
485-
return errors.Wrap(err, "failed to create destination file")
455+
return err
486456
}
457+
defer dstFile.Close()
487458

488-
// Copy data with proper cleanup on failure
489459
_, err = io.Copy(dstFile, srcFile)
490-
closeErr := dstFile.Close()
491460
if err != nil {
492-
os.Remove(dstPath) // Clean up on copy failure
493-
return errors.Wrap(err, "failed to copy file data")
494-
}
495-
if closeErr != nil {
496-
os.Remove(dstPath) // Clean up on close failure
497-
return errors.Wrap(closeErr, "failed to close destination file")
461+
return err
498462
}
499463

500-
// Delete source file
501-
if err := os.Remove(srcPath); err != nil {
502-
return errors.Wrap(err, "failed to remove source file")
503-
}
464+
dstFile.Close()
465+
srcFile.Close()
504466

505-
return nil
506-
}
507-
508-
// isWindowsFileLockError detects Windows-specific file locking errors
509-
func isWindowsFileLockError(err error) bool {
510-
if err == nil {
511-
return false
512-
}
513-
errStr := strings.ToLower(err.Error())
514-
return strings.Contains(errStr, "access is denied") ||
515-
strings.Contains(errStr, "being used by another process") ||
516-
strings.Contains(errStr, "sharing violation") ||
517-
strings.Contains(errStr, "the process cannot access the file")
467+
return os.Remove(src)
518468
}

0 commit comments

Comments
 (0)