Remove stack when directory is renamed #5250
Conversation
It's the only required knob from the configuration. Using it directly makes that clear. Signed-off-by: Tom Wieczorek <[email protected]>
A directory rename is signaled as a rename with the old name followed by a create with the new name. The applier manager didn't honor this, effectively running the old stack along with the new one. Signed-off-by: Tom Wieczorek <[email protected]>
See: aa704c5 ("Also remove stack when directory is renamed") Signed-off-by: Tom Wieczorek <[email protected]>
twz123
added
bug
| @@ -151,7 +150,7 @@ func (m *Manager) runWatchers(ctx context.Context) { | |||
| if dir.IsDirectory(event.Name) { | |||
| m.createStack(ctx, stacks, event.Name) | |||
| } | |||
| case fsnotify.Remove: | |||
There was a problem hiding this comment.
Interesting. The unit test fails on Windows, whereas it works well on Linux. I suspect this is due to the implicit event order which is probably different between Windows and Linux. I can imagine how the Create event for the new name happening before the Rename event for the old name will screw things up. Maybe the fix is more involved than simply reacting on Rename events, as well.
| @@ -151,7 +150,7 @@ func (m *Manager) runWatchers(ctx context.Context) { | |||
| if dir.IsDirectory(event.Name) { | |||
| m.createStack(ctx, stacks, event.Name) | |||
There was a problem hiding this comment.
In the Windows unit test logs, I see paths in the logs like this: stack="C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\TestManager_Rename2408267215\\001/lorem". Note the last slash, which should IMO be a backslash. IIUC the logged value is coming from fsnotify, so something is not normalizing path names there. Note that the slash is a valid separator on Windows, too, albeit not the canonical one.
|
The PR is marked as stale since no activity has been recorded in 30 days |
Description
A directory rename is signaled as a rename with the old name followed by a create with the new name. The applier manager didn't honor this, effectively running the old stack along with the new one.
Type of change
How Has This Been Tested?
Checklist: