Merge pull request #646 from vania-pooh/master

Logging config and docs improvements

config/config.go

@@ -102,12 +102,12 @@ func (config *Config) Load(browsers, containerLogs string) error {
 		return fmt.Errorf("browsers config: %v", err)
 	}
 	log.Printf("[-] [INIT] [Loaded configuration from %s]", browsers)
-	var cl *container.LogConfig
-	err = loadJSON(containerLogs, &cl)
-	if err != nil {
-		log.Printf("[-] [INIT] [Using default containers log configuration because of: %v]", err)
-		cl = &container.LogConfig{}
-	} else {
+	cl := &container.LogConfig{}
+	if containerLogs != "" {
+		err = loadJSON(containerLogs, cl)
+		if err != nil {
+			return fmt.Errorf("log config: %v", err)
+		}
 		log.Printf("[-] [INIT] [Loaded log configuration from %s]", containerLogs)
 	}
 	config.lock.Lock()

config_test.go

@@ -12,6 +12,8 @@ import (
 	"github.com/aerokube/selenoid/session"
 )
 
+const testLogConf = "config/container-logs.json"
+
 func configfile(s string) string {
 	tmp, err := ioutil.TempFile("", "config")
 	if err != nil {
@@ -32,31 +34,39 @@ func TestConfig(t *testing.T) {
 	confFile := configfile(`{}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	err := conf.Load(confFile, logConfPath)
+	err := conf.Load(confFile, testLogConf)
 	AssertThat(t, err, Is{nil})
 }
 
 func TestConfigError(t *testing.T) {
 	confFile := configfile(`{}`)
 	os.Remove(confFile)
 	conf := config.NewConfig()
-	err := conf.Load(confFile, logConfPath)
+	err := conf.Load(confFile, testLogConf)
 	AssertThat(t, err.Error(), EqualTo{fmt.Sprintf("browsers config: read error: open %s: no such file or directory", confFile)})
 }
 
+func TestLogConfigError(t *testing.T) {
+	confFile := configfile(`{}`)
+	defer os.Remove(confFile)
+	conf := config.NewConfig()
+	err := conf.Load(confFile, "some-missing-file")
+	AssertThat(t, err, Not{nil})
+}
+
 func TestConfigParseError(t *testing.T) {
 	confFile := configfile(`{`)
 	defer os.Remove(confFile)
 	var conf config.Config
-	err := conf.Load(confFile, logConfPath)
+	err := conf.Load(confFile, testLogConf)
 	AssertThat(t, err.Error(), EqualTo{"browsers config: parse error: unexpected end of JSON input"})
 }
 
 func TestConfigEmptyState(t *testing.T) {
 	confFile := configfile(`{}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	state := conf.State(session.NewMap(), 0, 0, 0)
 	AssertThat(t, state.Total, EqualTo{0})
@@ -69,7 +79,7 @@ func TestConfigNonEmptyState(t *testing.T) {
 	confFile := configfile(`{}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	sessions := session.NewMap()
 	sessions.Put("0", &session.Session{Caps: session.Caps{Name: "firefox", Version: "49.0"}, Quota: "unknown"})
@@ -85,7 +95,7 @@ func TestConfigEmptyVersions(t *testing.T) {
 	confFile := configfile(`{"firefox":{}}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	sessions := session.NewMap()
 	sessions.Put("0", &session.Session{Caps: session.Caps{Name: "firefox", Version: "49.0"}, Quota: "unknown"})
@@ -101,7 +111,7 @@ func TestConfigNonEmptyVersions(t *testing.T) {
 	confFile := configfile(`{"firefox":{"default":"49.0","versions":{"49.0":{}}}}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	sessions := session.NewMap()
 	sessions.Put("0", &session.Session{Caps: session.Caps{Name: "firefox", Version: "49.0"}, Quota: "unknown"})
@@ -117,7 +127,7 @@ func TestConfigFindMissingBrowser(t *testing.T) {
 	confFile := configfile(`{}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	_, _, ok := conf.Find("firefox", "")
 	AssertThat(t, ok, Is{false})
@@ -127,7 +137,7 @@ func TestConfigFindDefaultVersionError(t *testing.T) {
 	confFile := configfile(`{"firefox":{"default":""}}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	_, _, ok := conf.Find("firefox", "")
 	AssertThat(t, ok, Is{false})
@@ -137,7 +147,7 @@ func TestConfigFindDefaultVersion(t *testing.T) {
 	confFile := configfile(`{"firefox":{"default":"49.0"}}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	_, v, ok := conf.Find("firefox", "")
 	AssertThat(t, ok, Is{false})
@@ -148,7 +158,7 @@ func TestConfigFindFoundByEmptyPrefix(t *testing.T) {
 	confFile := configfile(`{"firefox":{"default":"49.0","versions":{"49.0":{}}}}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	_, v, ok := conf.Find("firefox", "")
 	AssertThat(t, ok, Is{true})
@@ -159,7 +169,7 @@ func TestConfigFindFoundByPrefix(t *testing.T) {
 	confFile := configfile(`{"firefox":{"default":"49.0","versions":{"49.0":{}}}}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	_, v, ok := conf.Find("firefox", "49")
 	AssertThat(t, ok, Is{true})
@@ -170,7 +180,7 @@ func TestConfigFindFoundByMatch(t *testing.T) {
 	confFile := configfile(`{"firefox":{"default":"49.0","versions":{"49.0":{}}}}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	_, v, ok := conf.Find("firefox", "49.0")
 	AssertThat(t, ok, Is{true})
@@ -181,7 +191,7 @@ func TestConfigFindImage(t *testing.T) {
 	confFile := configfile(`{"firefox":{"default":"49.0","versions":{"49.0":{"image":"image","port":"5555", "path":"/"}}}}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 
 	b, v, ok := conf.Find("firefox", "49.0")
 	AssertThat(t, ok, Is{true})
@@ -198,18 +208,18 @@ func TestConfigConcurrentLoad(t *testing.T) {
 
 	done := make(chan struct{})
 	go func() {
-		conf.Load(confFile, logConfPath)
+		conf.Load(confFile, testLogConf)
 		done <- struct{}{}
 	}()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 	<-done
 }
 
 func TestConfigConcurrentLoadAndRead(t *testing.T) {
 	confFile := configfile(`{"firefox":{"default":"49.0","versions":{"49.0":{"image":"image","port":"5555", "path":"/", "tmpfs": {"/tmp":"size=64k"}}}}}`)
 	defer os.Remove(confFile)
 	conf := config.NewConfig()
-	err := conf.Load(confFile, logConfPath)
+	err := conf.Load(confFile, testLogConf)
 	if err != nil {
 		t.Error(err)
 	}
@@ -218,15 +228,15 @@ func TestConfigConcurrentLoadAndRead(t *testing.T) {
 		browser, _, _ := conf.Find("firefox", "")
 		done <- browser.Tmpfs["/tmp"]
 	}()
-	conf.Load(confFile, logConfPath)
+	conf.Load(confFile, testLogConf)
 	<-done
 }
 
 func TestConfigConcurrentRead(t *testing.T) {
 	confFile := configfile(`{"firefox":{"default":"49.0","versions":{"49.0":{"image":"image","port":"5555", "path":"/", "tmpfs": {"/tmp":"size=64k"}}}}}`)
 	defer os.Remove(confFile)
 	var conf config.Config
-	err := conf.Load(confFile, logConfPath)
+	err := conf.Load(confFile, testLogConf)
 	if err != nil {
 		t.Error(err)
 	}

docs/docker-compose.adoc

@@ -5,17 +5,21 @@ To avoid network problems between browser drivers inside containers and Selenoid
 [source,yaml]
 ----
 version: '3'
+networks:
+  selenoid:
 services:
   selenoid:
-    network_mode: bridge
+    networks:
+      elk: null
     image: aerokube/selenoid:latest-release
     volumes:
       - "$PWD:/etc/selenoid"
       - "/var/run/docker.sock:/var/run/docker.sock"
       - "$PWD:/opt/selenoid/video"
+      - "$PWD:/opt/selenoid/logs"
     environment:
       - OVERRIDE_VIDEO_OUTPUT_DIR=$PWD
-    command: ["-conf", "/etc/selenoid/browsers.json", "-video-output-dir", "/opt/selenoid/video"]        
+    command: ["-conf", "/etc/selenoid/browsers.json", "-video-output-dir", "/opt/selenoid/video", "-video-output-dir", "/opt/selenoid/logs", "-container-network", "selenoid"]        
     ports:
       - "4444:4444"     
 ----

docs/faq.adoc

@@ -78,10 +78,6 @@ When running Selenoid as Docker container video feature can be not working (beca
 . You are passing an `OVERRIDE_VIDEO_OUTPUT_DIR` environment variable pointing to a directory on the `host machine` where video files are actually stored
 . When passing custom arguments to Selenoid container (such as `-limit` or `-timeout`) you also have to pass `-video-output-dir /opt/selenoid/video` and mount host machine video dir to `/opt/selenoid/video`
 
-**Can't open Selenoid video with Firefox**
-
-This is because we are using H264 codec which is not supported in Firefox for licensing reasons. Should work like a charm in Google Chrome or VLC player.
-
 **Can't get VNC feature to work: Disconnected**
 
 Please check the following:
@@ -93,10 +89,6 @@ Please check the following:
 
 You are using `driver.close()` instead of `driver.quit()` and just closed the last browser tab instead of removing the session.
 
-**Can't maximize browser window**
-
-This is because of missing window manager in browser images. Should be fixed soon.
-
 **Can Selenoid pull browser images automatically?**
 
 No, we did not implement this feature intentionally. We consider that all such cluster maintenance tasks can influence performance and stability when done automatically.

docs/index.adoc

@@ -30,8 +30,6 @@ include::updating-browsers.adoc[leveloffset=+1]
 include::timezone.adoc[leveloffset=+1]
 include::video.adoc[leveloffset=+1]
 include::logs.adoc[leveloffset=+1]
-include::s3.adoc[leveloffset=+1]
-include::metadata.adoc[leveloffset=+1]
 include::docker-compose.adoc[leveloffset=+1]
 include::log-files.adoc[leveloffset=+1]
 include::cli-flags.adoc[leveloffset=+1]
@@ -44,6 +42,8 @@ include::file-upload.adoc[leveloffset=+1]
 include::file-download.adoc[leveloffset=+1]
 include::clipboard.adoc[leveloffset=+1]
 include::devtools.adoc[leveloffset=+1]
+include::s3.adoc[leveloffset=+1]
+include::metadata.adoc[leveloffset=+1]
 
 include::contributing.adoc[]
 

main.go

@@ -106,7 +106,7 @@ func init() {
 	flag.BoolVar(&enableFileUpload, "enable-file-upload", false, "File upload support")
 	flag.StringVar(&listen, "listen", ":4444", "Network address to accept connections")
 	flag.StringVar(&confPath, "conf", "config/browsers.json", "Browsers configuration file")
-	flag.StringVar(&logConfPath, "log-conf", "config/container-logs.json", "Container logging configuration file")
+	flag.StringVar(&logConfPath, "log-conf", "", "Container logging configuration file")
 	flag.IntVar(&limit, "limit", 5, "Simultaneous container runs")
 	flag.IntVar(&retryCount, "retry-count", 1, "New session attempts retry count")
 	flag.DurationVar(&timeout, "timeout", 60*time.Second, "Session idle timeout in time.Duration format")

service/docker.go

@@ -26,12 +26,17 @@ import (
 const (
 	sysAdmin               = "SYS_ADMIN"
 	overrideVideoOutputDir = "OVERRIDE_VIDEO_OUTPUT_DIR"
-	vncPort                = "5900"
-	devtoolsPort           = "7070"
-	fileserverPort         = "8080"
-	clipboardPort          = "9090"
 )
 
+var ports = struct {
+	VNC, Devtools, Fileserver, Clipboard string
+}{
+	VNC:        "5900",
+	Devtools:   "7070",
+	Fileserver: "8080",
+	Clipboard:  "9090",
+}
+
 // Docker - docker container manager
 type Docker struct {
 	ServiceBase
@@ -141,11 +146,11 @@ func (d *Docker) StartWithCancel() (*StartedService, error) {
 	}
 	servicePort := d.Service.Port
 	pc := map[string]nat.Port{
-		servicePort:    selenium,
-		vncPort:        vnc,
-		devtoolsPort:   devtools,
-		fileserverPort: fileserver,
-		clipboardPort:  clipboard,
+		servicePort:      selenium,
+		ports.VNC:        vnc,
+		ports.Devtools:   devtools,
+		ports.Fileserver: fileserver,
+		ports.Clipboard:  clipboard,
 	}
 	hostPort := getHostPort(d.Environment, servicePort, d.Caps, stat, pc)
 	u := &url.URL{Scheme: "http", Host: hostPort.Selenium, Path: d.Service.Path}
@@ -213,24 +218,24 @@ func getPortConfig(service *config.Browser, caps session.Caps, env Environment)
 	if err != nil {
 		return nil, fmt.Errorf("new selenium port: %v", err)
 	}
-	fileserver, err := nat.NewPort("tcp", fileserverPort)
+	fileserver, err := nat.NewPort("tcp", ports.Fileserver)
 	if err != nil {
 		return nil, fmt.Errorf("new fileserver port: %v", err)
 	}
-	clipboard, err := nat.NewPort("tcp", clipboardPort)
+	clipboard, err := nat.NewPort("tcp", ports.Clipboard)
 	if err != nil {
 		return nil, fmt.Errorf("new clipboard port: %v", err)
 	}
 	exposedPorts := map[nat.Port]struct{}{selenium: {}, fileserver: {}, clipboard: {}}
 	var vnc nat.Port
 	if caps.VNC {
-		vnc, err = nat.NewPort("tcp", vncPort)
+		vnc, err = nat.NewPort("tcp", ports.VNC)
 		if err != nil {
 			return nil, fmt.Errorf("new vnc port: %v", err)
 		}
 		exposedPorts[vnc] = struct{}{}
 	}
-	devtools, err := nat.NewPort("tcp", devtoolsPort)
+	devtools, err := nat.NewPort("tcp", ports.Devtools)
 	if err != nil {
 		return nil, fmt.Errorf("new devtools port: %v", err)
 	}
@@ -369,13 +374,13 @@ func getHostPort(env Environment, servicePort string, caps session.Caps, stat ty
 	}
 	hp := session.HostPort{
 		Selenium:   fn(servicePort, pc[servicePort]),
-		Fileserver: fn(fileserverPort, pc[fileserverPort]),
-		Clipboard:  fn(clipboardPort, pc[clipboardPort]),
-		Devtools:   fn(devtoolsPort, pc[devtoolsPort]),
+		Fileserver: fn(ports.Fileserver, pc[ports.Fileserver]),
+		Clipboard:  fn(ports.Clipboard, pc[ports.Clipboard]),
+		Devtools:   fn(ports.Devtools, pc[ports.Devtools]),
 	}
 
 	if caps.VNC {
-		hp.VNC = fn(vncPort, pc[vncPort])
+		hp.VNC = fn(ports.VNC, pc[ports.VNC])
 	}
 
 	return hp