Skip to content

Commit 0062ecf

Browse files
committed
oc rsync: Add --last flag
The flag can be used to only sync N most recently modified files. The flag is mutually exclusive to: * --include * --exclude * --delete * --watch It is also ignored when the source is a local directory when using tar, because the implementation doesn't allow to select particular files. Generally when there is any problem when using --last, the error is ignored and sync happens as if the flag was not specified. Regarding implementation details, oc rsync performs an extras step when --last is specified, and that is discovering relevant files to select. This is done using manual directory walking when local, for remote the remote executor is used to invoke a shell using find+sort+head. The resulting filenames are then passed to --files-from for rsync, for tar they are simply passed to the command as arguments. Tests were added for testing the discovery mechanism, the rest has been tested manually. oc rsync is poorly unit-tested in general. Assisted-by: Claude Code
1 parent 6d61f76 commit 0062ecf

12 files changed

Lines changed: 597 additions & 10 deletions

pkg/cli/rsync/copy_rsync.go

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"io"
8+
"path/filepath"
89
"strings"
910

1011
"github.com/spf13/cobra"
@@ -27,6 +28,9 @@ type rsyncStrategy struct {
2728
LocalExecutor executor
2829
RemoteExecutor executor
2930
podChecker podChecker
31+
32+
Last uint
33+
fileDiscovery fileDiscoverer
3034
}
3135

3236
// DefaultRsyncRemoteShellToUse generates an command to create a remote shell.
@@ -75,15 +79,45 @@ func NewRsyncStrategy(o *RsyncOptions) CopyStrategy {
7579
RemoteExecutor: newRemoteExecutor(o),
7680
LocalExecutor: newLocalExecutor(),
7781
podChecker: podAPIChecker{o.Client, o.Namespace, podName, o.ContainerName, o.Quiet, o.ErrOut},
82+
Last: o.Last,
83+
fileDiscovery: o.fileDiscovery,
7884
}
7985
}
8086

8187
func (r *rsyncStrategy) Copy(source, destination *PathSpec, out, errOut io.Writer) error {
8288
klog.V(3).Infof("Copying files with rsync")
89+
90+
// In case --last is specified, discover the right files and pass them to rsync as an explicit list.
91+
var (
92+
in io.Reader
93+
dst = destination.RsyncPath()
94+
)
95+
if r.Last > 0 {
96+
filenames, err := r.fileDiscovery.DiscoverFiles(source.Path, r.Last)
97+
if err != nil {
98+
klog.Infof("Warning: failed to apply --last filtering: %v", err)
99+
} else {
100+
var b bytes.Buffer
101+
for _, filename := range filenames {
102+
b.WriteString(filename)
103+
b.WriteRune('\n')
104+
}
105+
in = &b
106+
107+
// Make dst compatible with what rsync does without --last.
108+
dst = filepath.Join(dst, filepath.Base(source.Path))
109+
110+
klog.V(3).Infof("Applied --last=%d to rsync strategy: using %d files", r.Last, len(filenames))
111+
}
112+
}
113+
83114
cmd := append([]string{"rsync"}, r.Flags...)
84-
cmd = append(cmd, "-e", r.RshCommand, source.RsyncPath(), destination.RsyncPath())
115+
if in != nil {
116+
cmd = append(cmd, "--files-from", "-")
117+
}
118+
cmd = append(cmd, "-e", r.RshCommand, source.RsyncPath(), dst)
85119
errBuf := &bytes.Buffer{}
86-
err := r.LocalExecutor.Execute(cmd, nil, out, errBuf)
120+
err := r.LocalExecutor.Execute(cmd, in, out, errBuf)
87121
if isExitError(err) {
88122
// Check if pod exists
89123
if podCheckErr := r.podChecker.CheckPod(); podCheckErr != nil {

pkg/cli/rsync/copy_rsyncd.go

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"io"
88
"math/rand"
99
"net"
10+
"path/filepath"
1011
"strconv"
1112
"strings"
1213
"time"
@@ -67,6 +68,9 @@ type rsyncDaemonStrategy struct {
6768
PortForwarder forwarder
6869
LocalExecutor executor
6970

71+
Last uint
72+
fileDiscovery fileDiscoverer
73+
7074
daemonPIDFile string
7175
daemonPort int
7276
localPort int
@@ -220,20 +224,54 @@ func (s *rsyncDaemonStrategy) stopPortForward() {
220224

221225
func (s *rsyncDaemonStrategy) copyUsingDaemon(source, destination *PathSpec, out, errOut io.Writer) error {
222226
klog.V(3).Infof("Copying files with rsync daemon")
227+
228+
// In case --last is specified, discover the right files and pass them to rsync as an explicit list.
229+
var (
230+
in io.Reader
231+
dst string
232+
)
233+
if destination.Local() {
234+
dst = destination.RsyncPath()
235+
} else {
236+
dst = destination.Path
237+
}
238+
if s.Last > 0 {
239+
filenames, err := s.fileDiscovery.DiscoverFiles(source.Path, s.Last)
240+
if err != nil {
241+
klog.Infof("Warning: failed to apply --last filtering: %v", err)
242+
} else {
243+
var b bytes.Buffer
244+
for _, filename := range filenames {
245+
b.WriteString(filename)
246+
b.WriteRune('\n')
247+
}
248+
in = &b
249+
250+
// Make dst compatible with what rsync does without --last.
251+
dst = filepath.Join(dst, filepath.Base(source.Path))
252+
253+
klog.V(3).Infof("Applied --last=%d to rsync-daemon strategy: using %d files", s.Last, len(filenames))
254+
}
255+
}
256+
223257
cmd := append([]string{"rsync"}, s.Flags...)
258+
if in != nil {
259+
cmd = append(cmd, "--files-from", "-")
260+
}
261+
224262
var sourceArg, destinationArg string
225263
if source.Local() {
226264
sourceArg = source.RsyncPath()
227265
} else {
228266
sourceArg = localRsyncURL(s.localPort, remoteLabel, source.Path)
229267
}
230268
if destination.Local() {
231-
destinationArg = destination.RsyncPath()
269+
destinationArg = dst
232270
} else {
233-
destinationArg = localRsyncURL(s.localPort, remoteLabel, destination.Path)
271+
destinationArg = localRsyncURL(s.localPort, remoteLabel, dst)
234272
}
235273
cmd = append(cmd, sourceArg, destinationArg)
236-
err := s.LocalExecutor.Execute(cmd, nil, out, errOut)
274+
err := s.LocalExecutor.Execute(cmd, in, out, errOut)
237275
if err != nil {
238276
// Determine whether rsync is present in the pod container
239277
testRsyncErr := executeWithLogging(s.RemoteExecutor, testRsyncCommand)
@@ -297,6 +335,8 @@ func NewRsyncDaemonStrategy(o *RsyncOptions) CopyStrategy {
297335
RemoteExecutor: remoteExec,
298336
LocalExecutor: newLocalExecutor(),
299337
PortForwarder: forwarder,
338+
Last: o.Last,
339+
fileDiscovery: o.fileDiscovery,
300340
}
301341
}
302342

pkg/cli/rsync/copy_tar.go

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type tarStrategy struct {
3030
RemoteExecutor executor
3131
Includes []string
3232
Excludes []string
33+
NoRecursion bool
3334
IgnoredFlags []string
3435
Flags []string
3536
}
@@ -44,15 +45,37 @@ func NewTarStrategy(o *RsyncOptions) CopyStrategy {
4445

4546
remoteExec := newRemoteExecutor(o)
4647

48+
// Handle --last option by discovering N most recently modified files.
49+
includes := append([]string(nil), o.RsyncInclude...)
50+
noRecursion := false
51+
if o.Last > 0 {
52+
if o.Source.Local() {
53+
klog.Info("Warning: --last flag is ignored when creating a local tar file")
54+
} else {
55+
filenames, err := o.fileDiscovery.DiscoverFiles(o.Source.Path, o.Last)
56+
if err != nil {
57+
klog.Infof("Warning: failed to apply --last filtering: %v", err)
58+
} else {
59+
// Replace any existing includes with our filtered list.
60+
if len(filenames) > 0 {
61+
includes = filenames
62+
noRecursion = true
63+
klog.V(3).Infof("Applied --last=%d to tar strategy: using %d files", o.Last, len(filenames))
64+
}
65+
}
66+
}
67+
}
68+
4769
return &tarStrategy{
4870
Quiet: o.Quiet,
4971
Delete: o.Delete,
50-
Includes: o.RsyncInclude,
72+
Includes: includes,
5173
Excludes: o.RsyncExclude,
5274
Tar: tarHelper,
5375
RemoteExecutor: remoteExec,
5476
IgnoredFlags: ignoredFlags,
5577
Flags: tarFlagsFromOptions(o),
78+
NoRecursion: noRecursion,
5679
}
5780
}
5881

@@ -145,7 +168,7 @@ func (r *tarStrategy) Copy(source, destination *PathSpec, out, errOut io.Writer)
145168
} else {
146169
klog.V(4).Infof("Creating local tar file %s from remote path %s", tmp.Name(), source.Path)
147170
errBuf := &bytes.Buffer{}
148-
err = tarRemote(r.RemoteExecutor, source.Path, r.Includes, r.Excludes, tmp, errBuf)
171+
err = tarRemote(r.RemoteExecutor, source.Path, r.Includes, r.Excludes, r.NoRecursion, tmp, errBuf)
149172
if err != nil {
150173
if checkTar(r.RemoteExecutor) != nil {
151174
return strategySetupError("tar not available in container")
@@ -198,7 +221,7 @@ func (r *tarStrategy) String() string {
198221
return "tar"
199222
}
200223

201-
func tarRemote(exec executor, sourceDir string, includes, excludes []string, out, errOut io.Writer) error {
224+
func tarRemote(exec executor, sourceDir string, includes, excludes []string, noRecursion bool, out, errOut io.Writer) error {
202225
klog.V(4).Infof("Tarring %s remotely", sourceDir)
203226

204227
exclude := []string{}
@@ -220,7 +243,11 @@ func tarRemote(exec executor, sourceDir string, includes, excludes []string, out
220243
include = append(include, path.Join(path.Base(sourceDir), pattern))
221244
}
222245

223-
cmd = []string{"tar", "-C", path.Dir(sourceDir), "-c", path.Base(sourceDir)}
246+
cmd = []string{"tar", "-C", path.Dir(sourceDir)}
247+
if noRecursion {
248+
cmd = append(cmd, "--no-recursion")
249+
}
250+
cmd = append(cmd, "-c", path.Base(sourceDir))
224251
cmd = append(cmd, append(include, exclude...)...)
225252
}
226253
klog.V(4).Infof("Remote tar command: %s", strings.Join(cmd, " "))

pkg/cli/rsync/copy_tar_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package rsync
2+
3+
import (
4+
"errors"
5+
"sort"
6+
"testing"
7+
8+
"github.com/google/go-cmp/cmp"
9+
)
10+
11+
// TestNewTarStrategy_FileDiscovery tests the specific file discovery logic in NewTarStrategy.
12+
func TestNewTarStrategy_FileDiscovery(t *testing.T) {
13+
testCases := []struct {
14+
name string
15+
originalIncludes []string
16+
discoveredFiles []string
17+
discoveryError error
18+
expectedIncludes []string
19+
}{
20+
{
21+
name: "discovery finds files - replaces original includes",
22+
originalIncludes: []string{"*.log", "*.txt"},
23+
discoveredFiles: []string{"newest.log", "middle.log", "oldest.log"},
24+
expectedIncludes: []string{"newest.log", "middle.log", "oldest.log"},
25+
},
26+
{
27+
name: "discovery finds no files - keeps original includes",
28+
originalIncludes: []string{"*.log", "*.txt"},
29+
discoveredFiles: []string{},
30+
expectedIncludes: []string{"*.log", "*.txt"},
31+
},
32+
{
33+
name: "discovery fails - keeps original includes",
34+
originalIncludes: []string{"*.log", "*.txt"},
35+
discoveryError: errors.New("command failed"),
36+
expectedIncludes: []string{"*.log", "*.txt"},
37+
},
38+
{
39+
name: "no original includes but discovery finds files",
40+
originalIncludes: []string{},
41+
discoveredFiles: []string{"file1.txt", "file2.txt"},
42+
expectedIncludes: []string{"file1.txt", "file2.txt"},
43+
},
44+
{
45+
name: "no original includes and no discovery",
46+
originalIncludes: []string{},
47+
discoveredFiles: []string{},
48+
expectedIncludes: nil,
49+
},
50+
}
51+
52+
for _, tc := range testCases {
53+
t.Run(tc.name, func(t *testing.T) {
54+
// Init the strategy.
55+
options := &RsyncOptions{
56+
RsyncInclude: tc.originalIncludes,
57+
Last: 3, // Enable file discovery
58+
Source: &PathSpec{PodName: "test-pod", Path: "/test/path"},
59+
Destination: &PathSpec{Path: "/local/path"},
60+
fileDiscovery: &mockFileDiscoverer{
61+
files: tc.discoveredFiles,
62+
err: tc.discoveryError,
63+
},
64+
}
65+
66+
strategy := NewTarStrategy(options).(*tarStrategy)
67+
68+
// Verify the result matches expectations.
69+
sort.Strings(strategy.Includes)
70+
sort.Strings(tc.expectedIncludes)
71+
if !cmp.Equal(strategy.Includes, tc.expectedIncludes) {
72+
t.Errorf("expected includes mismatch: \n%s\n",
73+
cmp.Diff(tc.expectedIncludes, strategy.Includes))
74+
}
75+
})
76+
}
77+
}

pkg/cli/rsync/discovery.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package rsync
2+
3+
// fileDiscoverer discovers files at the given path,
4+
// limiting the list to lastN most recently modified files.
5+
type fileDiscoverer interface {
6+
DiscoverFiles(basePath string, lastN uint) ([]string, error)
7+
}

pkg/cli/rsync/discovery_local.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package rsync
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"sort"
7+
"time"
8+
9+
"k8s.io/klog/v2"
10+
)
11+
12+
// localFileDiscoverer implements fileDiscoverer interface for local directories.
13+
type localFileDiscoverer struct{}
14+
15+
func newLocalFileDiscoverer() localFileDiscoverer {
16+
return localFileDiscoverer{}
17+
}
18+
19+
func (discoverer localFileDiscoverer) DiscoverFiles(basePath string, last uint) ([]string, error) {
20+
klog.V(4).Infof("Discovering files in local directory %s (last = %d)", basePath, last)
21+
22+
entries, err := os.ReadDir(basePath)
23+
if err != nil {
24+
return nil, fmt.Errorf("failed to read directory %s: %w", basePath, err)
25+
}
26+
27+
type fileInfo struct {
28+
name string
29+
modTime time.Time
30+
}
31+
32+
files := make([]fileInfo, 0, len(entries))
33+
for _, entry := range entries {
34+
if entry.IsDir() {
35+
continue // Skip directories, only process regular files.
36+
}
37+
38+
info, err := entry.Info()
39+
if err != nil {
40+
return nil, fmt.Errorf("failed to get file info for %s: %w", entry.Name(), err)
41+
}
42+
43+
files = append(files, fileInfo{
44+
name: entry.Name(),
45+
modTime: info.ModTime(),
46+
})
47+
}
48+
49+
// Sort by modification time (newest first).
50+
sort.Slice(files, func(i, j int) bool {
51+
return files[i].modTime.After(files[j].modTime)
52+
})
53+
54+
// Limit to the latest N files.
55+
if len(files) > int(last) {
56+
files = files[:last]
57+
}
58+
59+
// Extract just the file names (relative paths).
60+
result := make([]string, len(files))
61+
for i, file := range files {
62+
result[i] = file.name
63+
}
64+
return result, nil
65+
}

0 commit comments

Comments
 (0)