From 40febe3304dcdc4be7e0e242ad866c2a4bca60e3 Mon Sep 17 00:00:00 2001 From: Ivan Krutov Date: Thu, 26 Oct 2017 17:57:24 +0300 Subject: [PATCH 1/3] Fixed NPE in log driver tagging logic (fixes #248) --- .travis.yml | 2 +- service/docker.go | 10 +- service_test.go | 283 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 290 insertions(+), 5 deletions(-) create mode 100644 service_test.go diff --git a/.travis.yml b/.travis.yml index eb33010e..c3e6b3a2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,7 @@ services: - docker script: - - go test -race -coverprofile=coverage.txt -covermode=atomic -coverpkg .,github.com/aerokube/selenoid/session,github.com/aerokube/selenoid/config,github.com/aerokube/selenoid/protect + - go test -v -race -coverprofile=coverage.txt -covermode=atomic -coverpkg .,github.com/aerokube/selenoid/session,github.com/aerokube/selenoid/config,github.com/aerokube/selenoid/protect,github.com/aerokube/selenoid/service - GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -ldflags "-X main.buildStamp=`date -u '+%Y-%m-%d_%I:%M:%S%p'` -X main.gitRevision=`git describe --tags || git rev-parse HEAD`" - gox -os "linux darwin windows" -arch "amd64" -osarch="windows/386" -output "dist/{{.Dir}}_{{.OS}}_{{.Arch}}" -ldflags "-X main.buildStamp=`date -u '+%Y-%m-%d_%I:%M:%S%p'` -X main.gitRevision=`git describe --tags || git rev-parse HEAD`" diff --git a/service/docker.go b/service/docker.go index 8a867f9f..8472c900 100644 --- a/service/docker.go +++ b/service/docker.go @@ -143,10 +143,12 @@ func getPortConfig(service *config.Browser, caps session.Caps, env Environment) } func getLogConfig(logConfig container.LogConfig, caps session.Caps) container.LogConfig { - const tag = "tag" - _, ok := logConfig.Config[tag] - if caps.Name != "" && !ok { - logConfig.Config[tag] = caps.Name + if logConfig.Config != nil { + const tag = "tag" + _, ok := logConfig.Config[tag] + if caps.Name != "" && !ok { + logConfig.Config[tag] = caps.Name + } } return logConfig } diff --git a/service_test.go b/service_test.go new file mode 100644 index 00000000..472c3dd7 --- /dev/null +++ b/service_test.go @@ -0,0 +1,283 @@ +package main + +import ( + "fmt" + . "github.com/aandryashin/matchers" + "github.com/aerokube/selenoid/config" + "github.com/aerokube/selenoid/service" + "github.com/aerokube/selenoid/session" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/client" + "net/http" + "net/http/httptest" + "net/url" + "os" + "sync" + "testing" + "time" +) + +var ( + mockServer *httptest.Server + lock sync.Mutex +) + +func init() { + updateMux(testMux()) + timeout = 2 * time.Second + serviceStartupTimeout = 1 * time.Second + newSessionAttemptTimeout = 1 * time.Second + sessionDeleteTimeout = 1 * time.Second +} + +func updateMux(mux http.Handler) { + lock.Lock() + defer lock.Unlock() + mockServer = httptest.NewServer(mux) + os.Setenv("DOCKER_HOST", "tcp://"+hostPort(mockServer.URL)) + os.Setenv("DOCKER_API_VERSION", "1.29") +} + +func testMux() http.Handler { + mux := http.NewServeMux() + + //Selenium Hub mock + mux.HandleFunc("/wd/hub", http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }, + )) + + //Docker API mock + mux.HandleFunc("/v1.29/containers/create", http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + output := `{"id": "e90e34656806", "warnings": []}` + w.Write([]byte(output)) + }, + )) + mux.HandleFunc("/v1.29/containers/e90e34656806/start", http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + }, + )) + mux.HandleFunc("/v1.29/containers/e90e34656806", http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + }, + )) + mux.HandleFunc("/v1.29/containers/e90e34656806/json", http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + p := port(mockServer.URL) + output := fmt.Sprintf(` + { + "Id": "e90e34656806", + "Created": "2015-01-06T15:47:31.485331387Z", + "Driver": "aufs", + "HostConfig": {}, + "NetworkSettings": { + "Ports": { + "4444/tcp": [ + { + "HostIp": "0.0.0.0", + "HostPort": "%s" + } + ], + "5900/tcp": [ + { + "HostIp": "0.0.0.0", + "HostPort": "5900" + } + ], + "%s/tcp": [ + { + "HostIp": "0.0.0.0", + "HostPort": "%s" + } + ] + }, + "Networks": { + "bridge": { + "IPAMConfig": null, + "Links": null, + "Aliases": null, + "NetworkID": "0152391a00ed79360bcf69401f7e2659acfab9553615726dbbcfc08b4f367b25", + "EndpointID": "6a36b6f58b37490666329fd0fd74b21aa4eba939dd1ce466bdb6e0f826d56f98", + "Gateway": "127.0.0.1", + "IPAddress": "127.0.0.1", + "IPPrefixLen": 16, + "IPv6Gateway": "", + "GlobalIPv6Address": "", + "GlobalIPv6PrefixLen": 0, + "MacAddress": "02:42:ac:11:00:02" + } + } + }, + "State": {}, + "Mounts": [] + } + `, p, p, p) + w.Write([]byte(output)) + }, + )) + return mux +} + +func parseUrl(input string) *url.URL { + u, err := url.Parse(input) + if err != nil { + panic(err) + } + return u +} + +func hostPort(input string) string { + return parseUrl(input).Host +} + +func port(input string) string { + return parseUrl(input).Port() +} + +func testConfig(env *service.Environment) *config.Config { + conf := config.NewConfig() + p := "4444" + if env.InDocker { + p = port(mockServer.URL) + } + conf.Browsers["firefox"] = config.Versions{ + Default: "33.0", + Versions: map[string]*config.Browser{ + "33.0": { + Image: "selenoid/firefox:33.0", + Tmpfs: map[string]string{"/tmp": "size=128m"}, + Port: p, + Volumes: []string{"/test:/test"}, + }, + }, + } + conf.Browsers["internet explorer"] = config.Versions{ + Default: "11", + Versions: map[string]*config.Browser{ + "11": { + Image: []interface{}{ + "/usr/bin/test-command", "-arg", + }, + }, + }, + } + return conf +} + +func testEnvironment() *service.Environment { + return &service.Environment{ + CPU: int64(0), + Memory: int64(0), + Network: containerNetwork, + StartupTimeout: serviceStartupTimeout, + CaptureDriverLogs: captureDriverLogs, + } +} + +func TestFindOutsideOfDocker(t *testing.T) { + env := testEnvironment() + env.InDocker = false + testDocker(t, env, testConfig(env)) +} + +func TestFindInsideOfDocker(t *testing.T) { + env := testEnvironment() + env.InDocker = true + cfg := testConfig(env) + logConfig := make(map[string]string) + cfg.ContainerLogs = &container.LogConfig{ + Type: "rsyslog", + Config: logConfig, + } + testDocker(t, env, cfg) +} + +func TestFindDockerIPSpecified(t *testing.T) { + env := testEnvironment() + env.IP = "127.0.0.1" + testDocker(t, env, testConfig(env)) +} + +func testDocker(t *testing.T, env *service.Environment, cfg *config.Config) { + starter := createDockerStarter(t, env, cfg) + startedService, err := starter.StartWithCancel() + AssertThat(t, err, Is{nil}) + AssertThat(t, startedService.Url, Not{nil}) + AssertThat(t, startedService.ID, EqualTo{"e90e34656806"}) + AssertThat(t, startedService.VNCHostPort, EqualTo{"127.0.0.1:5900"}) + AssertThat(t, startedService.Cancel, Not{nil}) +} + +func createDockerStarter(t *testing.T, env *service.Environment, cfg *config.Config) service.Starter { + cli, err := client.NewEnvClient() + AssertThat(t, err, Is{nil}) + manager := service.DefaultManager{Environment: env, Client: cli, Config: cfg} + caps := session.Caps{ + Name: "firefox", + Version: "33.0", + ScreenResolution: "1024x768", + VNC: true, + HostsEntries: "example.com:192.168.0.1,test.com:192.168.0.2", + ApplicationContainers: "one,two", + TimeZone: "Europe/Moscow", + ContainerHostname: "some-hostname", + TestName: "my-cool-test", + } + starter, success := manager.Find(caps, 42) + AssertThat(t, success, Is{true}) + AssertThat(t, starter, Not{nil}) + return starter +} + +func failingMux(numDeleteRequests *int) http.Handler { + mux := http.NewServeMux() + mux.HandleFunc("/v1.29/containers/create", http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + output := `{"id": "e90e34656806", "warnings": []}` + w.Write([]byte(output)) + }, + )) + mux.HandleFunc("/v1.29/containers/e90e34656806/start", http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }, + )) + mux.HandleFunc("/v1.29/containers/e90e34656806", http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + *numDeleteRequests++ + w.WriteHeader(http.StatusNoContent) + }, + )) + return mux +} + +func TestDeleteContainerOnStartupError(t *testing.T) { + numDeleteRequests := 0 + updateMux(failingMux(&numDeleteRequests)) + defer updateMux(testMux()) + env := testEnvironment() + starter := createDockerStarter(t, env, testConfig(env)) + _, err := starter.StartWithCancel() + AssertThat(t, err, Not{nil}) + AssertThat(t, numDeleteRequests, EqualTo{1}) +} + +func TestFindDriver(t *testing.T) { + env := testEnvironment() + manager := service.DefaultManager{Environment: env, Config: testConfig(env)} + caps := session.Caps{ + Name: "internet explorer", //Using default version + ScreenResolution: "1024x768", + VNC: true, + } + starter, success := manager.Find(caps, 42) + AssertThat(t, success, Is{true}) + AssertThat(t, starter, Not{nil}) +} From 3b537ce94c478d5b1bf080bcacdd584631471a7e Mon Sep 17 00:00:00 2001 From: Ivan Krutov Date: Fri, 27 Oct 2017 06:49:01 +0300 Subject: [PATCH 2/3] Added documentation about updating browsers (fixes #250) --- docs/browsers-configuration-file.adoc | 13 ++++++-- docs/index.adoc | 1 + docs/logging-configuration-file.adoc | 9 +++++- docs/updating-browsers.adoc | 46 +++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 docs/updating-browsers.adoc diff --git a/docs/browsers-configuration-file.adoc b/docs/browsers-configuration-file.adoc index cc3c7ee0..8d3885e8 100644 --- a/docs/browsers-configuration-file.adoc +++ b/docs/browsers-configuration-file.adoc @@ -1,6 +1,6 @@ == Browsers Configuration File -Selenoid uses simple JSON configuration files of the following format: +Selenoid requires a simple JSON configuration file of the following format: .browsers.json [source,javascript] @@ -44,6 +44,13 @@ Selenoid uses simple JSON configuration files of the following format: This file represents a mapping between a list of supported browser versions and Docker container images or driver binaries. +[NOTE] +==== +To use custom browsers configuration file - use `-conf` flag: + + $ ./selenoid -conf /path/to/browsers.json +==== + === Browser Name and Version Browser name and version are just strings that are matched against Selenium desired capabilities: @@ -147,5 +154,5 @@ In some usage scenarios you may want to store browsers configuration file under # cat /path/to/browsers.json | jq -r '..|.image?|strings' | xargs -I{} docker pull {} NOTE: Why this is not the part of Selenoid? Well, this is easy to implement, but under heavy load the result can be unpredictable. -For instance, you've updated file, reloaded Selenoid and it should pull new images. How long you should wait new sessions then? -What if Docker Registry is inaccessible? So for maintenance reasons it is easier to delegate such simple logic to external script. +For example after updating the file and reloading Selenoid it should pull new images. How long will you wait for new sessions then? +What to do if Docker Registry is inaccessible? So for maintenance reasons it is easier to delegate such simple logic to external script. diff --git a/docs/index.adoc b/docs/index.adoc index e76e8ca6..62e16bc4 100644 --- a/docs/index.adoc +++ b/docs/index.adoc @@ -23,6 +23,7 @@ include::browser-images.adoc[leveloffset=+1] include::docker-settings.adoc[leveloffset=+1] include::browsers-configuration-file.adoc[leveloffset=+1] include::logging-configuration-file.adoc[leveloffset=+1] +include::updating-browsers.adoc[leveloffset=+1] include::timezone.adoc[leveloffset=+1] include::reloading-configuration.adoc[leveloffset=+1] include::docker-compose.adoc[leveloffset=+1] diff --git a/docs/logging-configuration-file.adoc b/docs/logging-configuration-file.adoc index 195890ec..82477574 100644 --- a/docs/logging-configuration-file.adoc +++ b/docs/logging-configuration-file.adoc @@ -4,7 +4,7 @@ By default Docker container logs are saved to host machine hard drive. When usin But in big Selenium cluster you may want to send logs to some centralized storage like https://www.elastic.co/products/logstash[Logstash] or https://www.graylog.org/[Graylog]. Docker provides such functionality by so-called https://docs.docker.com/engine/admin/logging/overview/[logging drivers]. -Selenoid logging configuration file allows to specify which logging driver to use globally for all started Docker +An optional Selenoid logging configuration file allows to specify which logging driver to use globally for all started Docker containers with browsers. Configuration file has the following format: .config/container-logs.json @@ -40,3 +40,10 @@ For example these Docker logging parameters: } } ---- + +[NOTE] +==== +To specify custom logging configuration file - use `-log-conf` flag: + + $ ./selenoid -log-conf /path/to/logging.json ... +==== diff --git a/docs/updating-browsers.adoc b/docs/updating-browsers.adoc new file mode 100644 index 00000000..2411166b --- /dev/null +++ b/docs/updating-browsers.adoc @@ -0,0 +1,46 @@ +== Updating Browsers + +One of the most frequent Selenoid maintenance tasks is updating browser versions. To modify the list of available browsers you need to: + +. Update browsers configuration file (aka `browsers.json`) by adding or deleting desired browser versions +. When adding new browsers in containers - pull respective container images, e.g.: + + $ docker pull selenoid/chrome:62.0 + +. Restart Selenoid or hot reload its configuration. + + +=== The Short Way + +This approach is mainly convenient for **personal usage** because Selenoid is restarted i.e. any running browser sessions are interrupted. Just type one http://aerokube.com/cm/latest/[Configuration Manager] command: + + $ ./cm selenoid update + +This command will download latest Selenoid release, browser images, generate new `browsers.json` and restart Selenoid. You may want to add `--vnc` flag to download images with VNC support: + + $ ./cm selenoid update --vnc + +Another useful flag is total number of last browser versions to download (default is `2`): + + $ ./cm selenoid update --last-versions 5 + +If you wish to pull new images and regenerate configuration without restarting Selenoid then type: + + $ ./cm selenoid configure --vnc --last-versions 5 + +=== A Bit Longer Way + +We recommend to use this approach on **production clusters** because Selenoid configuration is reloaded without restart. + +. Edit `browsers.json` file and pull new browsers containers manually or using <>. Alternatively configure Selenoid without restarting it like shown in previous section. +. Reload Selenoid configuration (see <> for more details): + + $ docker kill -s HUP selenoid + +. To verify that configuration was reloaded you can check Selenoid health: + + $ curl -s http://example.com:4444/ping + {"uptime":"","lastReloadTime":"2017-05-12 12:33:06.322038542 +0300 MSK","numRequests":} + ++ +Here `lastReloadTime` field shows when browsers configuration was reloaded for the last time. From 6913fb5cb5d4648ab5ff0159ecf9bf9cb6611633 Mon Sep 17 00:00:00 2001 From: Ivan Krutov Date: Fri, 27 Oct 2017 07:06:09 +0300 Subject: [PATCH 3/3] Added ability to disable privileged mode for containers (fixes #251) --- main.go | 3 +++ service/docker.go | 11 +++++++++-- service/service.go | 1 + service_test.go | 1 + 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index b93216a2..edad14c9 100644 --- a/main.go +++ b/main.go @@ -73,6 +73,7 @@ var ( confPath string logConfPath string captureDriverLogs bool + disablePrivileged bool conf *config.Config queue *protect.Queue manager service.Manager @@ -105,6 +106,7 @@ func init() { flag.Var(&cpu, "cpu", "Containers cpu limit as float e.g. 0.2 or 1.0") flag.StringVar(&containerNetwork, "container-network", "default", "Network to be used for containers") flag.BoolVar(&captureDriverLogs, "capture-driver-logs", false, "Whether to add driver process logs to Selenoid output") + flag.BoolVar(&disablePrivileged, "disable-privileged", false, "Whether to disable privileged container mode") flag.Parse() if version { @@ -142,6 +144,7 @@ func init() { Network: containerNetwork, StartupTimeout: serviceStartupTimeout, CaptureDriverLogs: captureDriverLogs, + Privileged: !disablePrivileged, } if disableDocker { manager = &service.DefaultManager{Environment: &environment, Config: conf} diff --git a/service/docker.go b/service/docker.go index 8472c900..6c4f0a9b 100644 --- a/service/docker.go +++ b/service/docker.go @@ -13,12 +13,16 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/api/types/strslice" "github.com/docker/docker/client" "github.com/docker/go-connections/nat" "strings" ) -const comma = "," +const ( + comma = "," + sysAdmin = "SYS_ADMIN" +) // Docker - docker container manager type Docker struct { @@ -54,13 +58,16 @@ func (d *Docker) StartWithCancel() (*StartedService, error) { NetworkMode: container.NetworkMode(d.Network), Tmpfs: d.Service.Tmpfs, ShmSize: getShmSize(d.Service), - Privileged: true, + Privileged: d.Privileged, Resources: container.Resources{ Memory: d.Memory, NanoCPUs: d.CPU, }, ExtraHosts: getExtraHosts(d.Service, d.Caps), } + if !d.Privileged { + hostConfig.CapAdd = strslice.StrSlice{sysAdmin} + } if d.ApplicationContainers != "" { links := strings.Split(d.ApplicationContainers, comma) hostConfig.Links = links diff --git a/service/service.go b/service/service.go index bbbfabc6..f56a91dc 100644 --- a/service/service.go +++ b/service/service.go @@ -22,6 +22,7 @@ type Environment struct { Hostname string StartupTimeout time.Duration CaptureDriverLogs bool + Privileged bool } // ServiceBase - stores fields required by all services diff --git a/service_test.go b/service_test.go index 472c3dd7..3c448df5 100644 --- a/service_test.go +++ b/service_test.go @@ -177,6 +177,7 @@ func testEnvironment() *service.Environment { Network: containerNetwork, StartupTimeout: serviceStartupTimeout, CaptureDriverLogs: captureDriverLogs, + Privileged: false, } }