Skip to content

Commit b3c84cc

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 1c54ec0 commit b3c84cc

File tree

12 files changed

+570
-10
lines changed

12 files changed

+570
-10
lines changed

pkg/cli/rsync/copy_rsync.go

Lines changed: 35 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,44 @@ 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+
klog.V(3).Infof("Applied --last=%d to rsync strategy: using %d files", r.Last, len(filenames))
107+
}
108+
109+
// Make dst compatible with what rsync does without --last.
110+
dst = filepath.Join(dst, filepath.Base(source.Path))
111+
}
112+
83113
cmd := append([]string{"rsync"}, r.Flags...)
84-
cmd = append(cmd, "-e", r.RshCommand, source.RsyncPath(), destination.RsyncPath())
114+
if in != nil {
115+
cmd = append(cmd, "--files-from", "-")
116+
}
117+
cmd = append(cmd, "-e", r.RshCommand, source.RsyncPath(), dst)
85118
errBuf := &bytes.Buffer{}
86-
err := r.LocalExecutor.Execute(cmd, nil, out, errBuf)
119+
err := r.LocalExecutor.Execute(cmd, in, out, errBuf)
87120
if isExitError(err) {
88121
// Check if pod exists
89122
if podCheckErr := r.podChecker.CheckPod(); podCheckErr != nil {

pkg/cli/rsync/copy_rsyncd.go

Lines changed: 42 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,53 @@ 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+
klog.V(3).Infof("Applied --last=%d to rsync-daemon strategy: using %d files", s.Last, len(filenames))
250+
}
251+
252+
// Make dst compatible with what rsync does without --last.
253+
dst = filepath.Join(dst, filepath.Base(source.Path))
254+
}
255+
223256
cmd := append([]string{"rsync"}, s.Flags...)
257+
if in != nil {
258+
cmd = append(cmd, "--files-from", "-")
259+
}
260+
224261
var sourceArg, destinationArg string
225262
if source.Local() {
226263
sourceArg = source.RsyncPath()
227264
} else {
228265
sourceArg = localRsyncURL(s.localPort, remoteLabel, source.Path)
229266
}
230267
if destination.Local() {
231-
destinationArg = destination.RsyncPath()
268+
destinationArg = dst
232269
} else {
233-
destinationArg = localRsyncURL(s.localPort, remoteLabel, destination.Path)
270+
destinationArg = localRsyncURL(s.localPort, remoteLabel, dst)
234271
}
235272
cmd = append(cmd, sourceArg, destinationArg)
236-
err := s.LocalExecutor.Execute(cmd, nil, out, errOut)
273+
err := s.LocalExecutor.Execute(cmd, in, out, errOut)
237274
if err != nil {
238275
// Determine whether rsync is present in the pod container
239276
testRsyncErr := executeWithLogging(s.RemoteExecutor, testRsyncCommand)
@@ -297,6 +334,8 @@ func NewRsyncDaemonStrategy(o *RsyncOptions) CopyStrategy {
297334
RemoteExecutor: remoteExec,
298335
LocalExecutor: newLocalExecutor(),
299336
PortForwarder: forwarder,
337+
Last: o.Last,
338+
fileDiscovery: o.fileDiscovery,
300339
}
301340
}
302341

pkg/cli/rsync/copy_tar.go

Lines changed: 29 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+
Last uint
3334
IgnoredFlags []string
3435
Flags []string
3536
}
@@ -44,15 +45,35 @@ 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+
if o.Last > 0 {
51+
if o.Source.Local() {
52+
klog.Info("Warning: --last flag is ignored when creating a local tar file")
53+
} else {
54+
filenames, err := o.fileDiscovery.DiscoverFiles(o.Source.Path, o.Last)
55+
if err != nil {
56+
klog.Infof("Warning: failed to apply --last filtering: %v", err)
57+
} else {
58+
// Replace any existing includes with our filtered list.
59+
if len(filenames) > 0 {
60+
includes = filenames
61+
klog.V(3).Infof("Applied --last=%d to tar strategy: using %d files", o.Last, len(filenames))
62+
}
63+
}
64+
}
65+
}
66+
4767
return &tarStrategy{
4868
Quiet: o.Quiet,
4969
Delete: o.Delete,
50-
Includes: o.RsyncInclude,
70+
Includes: includes,
5171
Excludes: o.RsyncExclude,
5272
Tar: tarHelper,
5373
RemoteExecutor: remoteExec,
5474
IgnoredFlags: ignoredFlags,
5575
Flags: tarFlagsFromOptions(o),
76+
Last: o.Last,
5677
}
5778
}
5879

@@ -145,7 +166,7 @@ func (r *tarStrategy) Copy(source, destination *PathSpec, out, errOut io.Writer)
145166
} else {
146167
klog.V(4).Infof("Creating local tar file %s from remote path %s", tmp.Name(), source.Path)
147168
errBuf := &bytes.Buffer{}
148-
err = tarRemote(r.RemoteExecutor, source.Path, r.Includes, r.Excludes, tmp, errBuf)
169+
err = tarRemote(r.RemoteExecutor, source.Path, r.Includes, r.Excludes, r.Last > 0, tmp, errBuf)
149170
if err != nil {
150171
if checkTar(r.RemoteExecutor) != nil {
151172
return strategySetupError("tar not available in container")
@@ -198,7 +219,7 @@ func (r *tarStrategy) String() string {
198219
return "tar"
199220
}
200221

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

204225
exclude := []string{}
@@ -220,7 +241,11 @@ func tarRemote(exec executor, sourceDir string, includes, excludes []string, out
220241
include = append(include, path.Join(path.Base(sourceDir), pattern))
221242
}
222243

223-
cmd = []string{"tar", "-C", path.Dir(sourceDir), "-c", path.Base(sourceDir)}
244+
cmd = []string{"tar", "-C", path.Dir(sourceDir)}
245+
if noRecursion {
246+
cmd = append(cmd, "--no-recursion")
247+
}
248+
cmd = append(cmd, "-c", path.Base(sourceDir))
224249
cmd = append(cmd, append(include, exclude...)...)
225250
}
226251
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)