Add folders without restart (fixes #2063)

cmd/syncthing/main.go

@@ -628,10 +628,8 @@ func syncthingMain() {
 		// Routine to pull blocks from other devices to synchronize the local
 		// folder. Does not run when we are in read only (publish only) mode.
 		if folderCfg.ReadOnly {
-			l.Okf("Ready to synchronize %s (read only; no external updates accepted)", folderCfg.ID)
 			m.StartFolderRO(folderCfg.ID)
 		} else {
-			l.Okf("Ready to synchronize %s (read-write)", folderCfg.ID)
 			m.StartFolderRW(folderCfg.ID)
 		}
 	}

internal/model/model.go

@@ -65,12 +65,13 @@ type service interface {
 type Model struct {
 	*suture.Supervisor
 
-	cfg             *config.Wrapper
-	db              *leveldb.DB
-	finder          *db.BlockFinder
-	progressEmitter *ProgressEmitter
-	id              protocol.DeviceID
-	shortID         uint64
+	cfg               *config.Wrapper
+	db                *leveldb.DB
+	finder            *db.BlockFinder
+	progressEmitter   *ProgressEmitter
+	id                protocol.DeviceID
+	shortID           uint64
+	cacheIgnoredFiles bool
 
 	deviceName    string
 	clientName    string
@@ -91,8 +92,6 @@ type Model struct {
 	deviceVer map[protocol.DeviceID]string
 	pmut      sync.RWMutex // protects protoConn and rawConn
 
-	started bool
-
 	reqValidationCache map[string]time.Time // folder / file name => time when confirmed to exist
 	rvmut              sync.RWMutex         // protects reqValidationCache
 }
@@ -119,6 +118,7 @@ func NewModel(cfg *config.Wrapper, id protocol.DeviceID, deviceName, clientName,
 		progressEmitter:    NewProgressEmitter(cfg),
 		id:                 id,
 		shortID:            id.Short(),
+		cacheIgnoredFiles:  cfg.Options().CacheIgnoredFiles,
 		deviceName:         deviceName,
 		clientName:         clientName,
 		clientVersion:      clientVersion,
@@ -190,6 +190,8 @@ func (m *Model) StartFolderRW(folder string) {
 	}
 
 	m.Add(p)
+
+	l.Okln("Ready to synchronize", folder, "(read-write)")
 }
 
 // StartFolderRO starts read only processing on the current model. When in
@@ -210,7 +212,9 @@ func (m *Model) StartFolderRO(folder string) {
 	m.folderRunners[folder] = s
 	m.fmut.Unlock()
 
-	go s.Serve()
+	m.Add(s)
+
+	l.Okln("Ready to synchronize", folder, "(read only; no external updates accepted)")
 }
 
 type ConnectionInfo struct {
@@ -1121,9 +1125,6 @@ func (m *Model) requestGlobal(deviceID protocol.DeviceID, folder, name string, o
 }
 
 func (m *Model) AddFolder(cfg config.FolderConfiguration) {
-	if m.started {
-		panic("cannot add folder to started model")
-	}
 	if len(cfg.ID) == 0 {
 		panic("cannot add empty folder id")
 	}
@@ -1138,7 +1139,7 @@ func (m *Model) AddFolder(cfg config.FolderConfiguration) {
 		m.deviceFolders[device.DeviceID] = append(m.deviceFolders[device.DeviceID], cfg.ID)
 	}
 
-	ignores := ignore.New(m.cfg.Options().CacheIgnoredFiles)
+	ignores := ignore.New(m.cacheIgnoredFiles)
 	_ = ignores.Load(filepath.Join(cfg.Path(), ".stignore")) // Ignore error, there might not be an .stignore
 	m.folderIgnores[cfg.ID] = ignores
 
@@ -1729,13 +1730,28 @@ func (m *Model) CommitConfiguration(from, to config.Configuration) bool {
 
 	fromFolders := mapFolders(from.Folders)
 	toFolders := mapFolders(to.Folders)
-	for folderID := range toFolders {
+	for folderID, cfg := range toFolders {
 		if _, ok := fromFolders[folderID]; !ok {
-			// A folder was added. Requires restart.
+			// A folder was added.
 			if debug {
-				l.Debugln(m, "requires restart, adding folder", folderID)
+				l.Debugln(m, "adding folder", folderID)
 			}
-			return false
+			m.AddFolder(cfg)
+			if cfg.ReadOnly {
+				m.StartFolderRO(folderID)
+			} else {
+				m.StartFolderRW(folderID)
+			}
+
+			// Drop connections to all devices that can now share the new
+			// folder.
+			m.pmut.Lock()
+			for _, dev := range cfg.DeviceIDs() {
+				if conn, ok := m.rawConn[dev]; ok {
+					closeRawConn(conn)
+				}
+			}
+			m.pmut.Unlock()
 		}
 	}
 

internal/model/model_test.go

@@ -96,8 +96,8 @@ func TestRequest(t *testing.T) {
 	// device1 shares default, but device2 doesn't
 	m.AddFolder(defaultFolderConfig)
 	m.StartFolderRO("default")
-	m.ScanFolder("default")
 	m.ServeBackground()
+	m.ScanFolder("default")
 
 	// Existing, shared file
 	bs, err := m.Request(device1, "default", "foo", 0, 6, nil, 0, nil)

test/norestart_test.go

@@ -83,3 +83,135 @@ func TestAddDeviceWithoutRestart(t *testing.T) {
 
 	rc.AwaitSync("default", p1, p4)
 }
+
+func TestFolderWithoutRestart(t *testing.T) {
+	log.Println("Cleaning...")
+	err := removeAll("testfolder-p1", "testfolder-p4", "h1/index*", "h4/index*")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer removeAll("testfolder-p1", "testfolder-p4")
+
+	if err := generateFiles("testfolder-p1", 50, 18, "../LICENSE"); err != nil {
+		t.Fatal(err)
+	}
+
+	p1 := startInstance(t, 1)
+	defer checkedStop(t, p1)
+
+	p4 := startInstance(t, 4)
+	defer checkedStop(t, p4)
+
+	if ok, err := p1.ConfigInSync(); err != nil || !ok {
+		t.Fatal("p1 should be in sync;", ok, err)
+	}
+
+	if ok, err := p4.ConfigInSync(); err != nil || !ok {
+		t.Fatal("p4 should be in sync;", ok, err)
+	}
+
+	// Add a new folder to p1, shared with p4. Back up and restore the config
+	// first.
+
+	log.Println("Adding testfolder to p1...")
+
+	os.Remove("h1/config.xml.orig")
+	os.Rename("h1/config.xml", "h1/config.xml.orig")
+	defer os.Rename("h1/config.xml.orig", "h1/config.xml")
+
+	cfg, err := p1.GetConfig()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	newFolder := config.FolderConfiguration{
+		ID:              "testfolder",
+		RawPath:         "testfolder-p1",
+		RescanIntervalS: 86400,
+		Copiers:         1,
+		Hashers:         1,
+		Pullers:         1,
+		Devices:         []config.FolderDeviceConfiguration{{DeviceID: p4.ID()}},
+	}
+	newDevice := config.DeviceConfiguration{
+		DeviceID:    p4.ID(),
+		Name:        "p4",
+		Addresses:   []string{"dynamic"},
+		Compression: protocol.CompressMetadata,
+	}
+
+	cfg.Folders = append(cfg.Folders, newFolder)
+	cfg.Devices = append(cfg.Devices, newDevice)
+
+	if err = p1.PostConfig(cfg); err != nil {
+		t.Fatal(err)
+	}
+
+	// Add a new folder to p4, shared with p1. Back up and restore the config
+	// first.
+
+	log.Println("Adding testfolder to p4...")
+
+	os.Remove("h4/config.xml.orig")
+	os.Rename("h4/config.xml", "h4/config.xml.orig")
+	defer os.Rename("h4/config.xml.orig", "h4/config.xml")
+
+	cfg, err = p4.GetConfig()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	newFolder.RawPath = "testfolder-p4"
+	newFolder.Devices = []config.FolderDeviceConfiguration{{DeviceID: p1.ID()}}
+	newDevice.DeviceID = p1.ID()
+	newDevice.Name = "p1"
+	newDevice.Addresses = []string{"127.0.0.1:22001"}
+
+	cfg.Folders = append(cfg.Folders, newFolder)
+	cfg.Devices = append(cfg.Devices, newDevice)
+
+	if err = p4.PostConfig(cfg); err != nil {
+		t.Fatal(err)
+	}
+
+	// The change should not require a restart, so the config should be "in sync"
+
+	if ok, err := p1.ConfigInSync(); err != nil || !ok {
+		t.Fatal("p1 should be in sync;", ok, err)
+	}
+	if ok, err := p4.ConfigInSync(); err != nil || !ok {
+		t.Fatal("p4 should be in sync;", ok, err)
+	}
+
+	// The folder should start and scan - wait for the event that signals this
+	// has happened.
+
+	log.Println("Waiting for testfolder to scan...")
+
+	since := 0
+outer:
+	for {
+		events, err := p4.Events(since)
+		if err != nil {
+			t.Fatal(err)
+		}
+		for _, event := range events {
+			if event.Type == "StateChanged" {
+				data := event.Data.(map[string]interface{})
+				folder := data["folder"].(string)
+				from := data["from"].(string)
+				to := data["to"].(string)
+				if folder == "testfolder" && from == "scanning" && to == "idle" {
+					break outer
+				}
+			}
+			since = event.ID
+		}
+	}
+
+	// It should sync to the other side successfully
+
+	log.Println("Waiting for p1 and p4 to connect and sync...")
+
+	rc.AwaitSync("testfolder", p1, p4)
+}