From 862aa6ead647ae28a55ec75f0f1d423d4ad3e03b Mon Sep 17 00:00:00 2001 From: Ivan Krutov Date: Sat, 12 Jan 2019 17:20:23 +0300 Subject: [PATCH 1/3] Using ports structure instead of independent constants --- service/docker.go | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/service/docker.go b/service/docker.go index 27b8fccc..e694045d 100644 --- a/service/docker.go +++ b/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 From 53b1812dd35428dc0c1e8e9c5e76553c9eb5b576 Mon Sep 17 00:00:00 2001 From: Ivan Krutov Date: Sat, 12 Jan 2019 17:30:36 +0300 Subject: [PATCH 2/3] Loading log-conf only when file is specified (fixes #645) --- config/config.go | 12 ++++++------ config_test.go | 48 +++++++++++++++++++++++++++++------------------- main.go | 2 +- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/config/config.go b/config/config.go index cb2fb442..667b2979 100644 --- a/config/config.go +++ b/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() diff --git a/config_test.go b/config_test.go index 7907a0ca..f848afb0 100644 --- a/config_test.go +++ b/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,7 +34,7 @@ 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}) } @@ -40,15 +42,23 @@ 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"}) } @@ -56,7 +66,7 @@ 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,10 +208,10 @@ 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 } @@ -209,7 +219,7 @@ 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,7 +228,7 @@ func TestConfigConcurrentLoadAndRead(t *testing.T) { browser, _, _ := conf.Find("firefox", "") done <- browser.Tmpfs["/tmp"] }() - conf.Load(confFile, logConfPath) + conf.Load(confFile, testLogConf) <-done } @@ -226,7 +236,7 @@ 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) } diff --git a/main.go b/main.go index db05bf82..33b2fcda 100644 --- a/main.go +++ b/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") From 1efadb8099663d3e8527ee4e2e07f957c0e33e15 Mon Sep 17 00:00:00 2001 From: Ivan Krutov Date: Sat, 12 Jan 2019 17:41:22 +0300 Subject: [PATCH 3/3] Documentation improvements (fixes #640) --- docs/docker-compose.adoc | 8 ++++++-- docs/faq.adoc | 8 -------- docs/index.adoc | 4 ++-- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/docs/docker-compose.adoc b/docs/docker-compose.adoc index 181b9e7f..b754410d 100644 --- a/docs/docker-compose.adoc +++ b/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" ---- diff --git a/docs/faq.adoc b/docs/faq.adoc index 8d5c59bf..be675b18 100644 --- a/docs/faq.adoc +++ b/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. \ No newline at end of file diff --git a/docs/index.adoc b/docs/index.adoc index a231d0fe..2c025929 100644 --- a/docs/index.adoc +++ b/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[]