18794: Restart RailsAPI workers when config file changes.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/lib/controller/rails_restart_test.go b/lib/controller/rails_restart_test.go
new file mode 100644 (file)
index 0000000..75dd22b
--- /dev/null
+++ b/lib/controller/rails_restart_test.go
@@ -0,0 +1,85 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package controller
+
+import (
+       "bytes"
+       "crypto/sha256"
+       "crypto/tls"
+       "fmt"
+       "io/ioutil"
+       "net/http"
+       "net/url"
+       "os"
+       "time"
+
+       "git.arvados.org/arvados.git/lib/config"
+       "git.arvados.org/arvados.git/sdk/go/arvadostest"
+       "git.arvados.org/arvados.git/sdk/go/ctxlog"
+       check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&railsRestartSuite{})
+
+type railsRestartSuite struct{}
+
+// This tests RailsAPI, not controller -- but tests RailsAPI's
+// integration with passenger, so it needs to run against the
+// run-tests.sh environment where RailsAPI runs under passenger, not
+// in the Rails test environment.
+func (s *railsRestartSuite) TestConfigReload(c *check.C) {
+       hc := http.Client{Transport: &http.Transport{
+               TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+       }}
+
+       confdata, err := os.ReadFile(os.Getenv("ARVADOS_CONFIG"))
+       c.Assert(err, check.IsNil)
+       oldhash := fmt.Sprintf("%x", sha256.Sum256(confdata))
+       c.Logf("oldhash %s", oldhash)
+
+       ldr := config.NewLoader(&bytes.Buffer{}, ctxlog.TestLogger(c))
+       cfg, err := ldr.Load()
+       c.Assert(err, check.IsNil)
+       cc, err := cfg.GetCluster("")
+       c.Assert(err, check.IsNil)
+       var metricsURL string
+       for u := range cc.Services.RailsAPI.InternalURLs {
+               u := url.URL(u)
+               mu, err := u.Parse("/metrics")
+               c.Assert(err, check.IsNil)
+               metricsURL = mu.String()
+       }
+
+       req, err := http.NewRequest(http.MethodGet, metricsURL, nil)
+       c.Assert(err, check.IsNil)
+       req.Header.Set("Authorization", "Bearer "+arvadostest.ManagementToken)
+
+       resp, err := hc.Do(req)
+       c.Assert(err, check.IsNil)
+       c.Check(resp.StatusCode, check.Equals, http.StatusOK)
+       body, err := ioutil.ReadAll(resp.Body)
+       c.Assert(err, check.IsNil)
+       c.Check(string(body), check.Matches, `(?ms).*`+oldhash+`.*`)
+
+       f, err := os.OpenFile(os.Getenv("ARVADOS_CONFIG"), os.O_WRONLY|os.O_APPEND, 0)
+       c.Assert(err, check.IsNil)
+       _, err = f.Write([]byte{'\n'})
+       c.Assert(err, check.IsNil)
+       err = f.Close()
+       c.Assert(err, check.IsNil)
+       newhash := fmt.Sprintf("%x", sha256.Sum256(append(confdata, '\n')))
+       c.Logf("newhash %s", newhash)
+
+       // Wait 2s to give RailsAPI's 1 Hz reload_config thread time
+       // to poll and hit restart.txt
+       time.Sleep(2 * time.Second)
+
+       resp, err = hc.Do(req)
+       c.Assert(err, check.IsNil)
+       c.Check(resp.StatusCode, check.Equals, http.StatusOK)
+       body, err = ioutil.ReadAll(resp.Body)
+       c.Assert(err, check.IsNil)
+       c.Check(string(body), check.Matches, `(?ms).*`+newhash+`.*`)
+}

diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go
index 9281f51d0cf0ee2b46ca97c2e59fde9f68051d4d..ec55725412c381714014a09931441855f9393466 100644 (file)
--- a/sdk/go/arvadostest/fixtures.go
+++ b/sdk/go/arvadostest/fixtures.go
@@ -16,7 +16,7 @@ const (
        AnonymousToken          = "4kg6k6lzmp9kj4cpkcoxie964cmvjahbt4fod9zru44k4jqdmi"
        DataManagerToken        = "320mkve8qkswstz7ff61glpk3mhgghmg67wmic7elw4z41pke1"
        SystemRootToken         = "systemusertesttoken1234567890aoeuidhtnsqjkxbmwvzpy"
-       ManagementToken         = "jg3ajndnq63sywcd50gbs5dskdc9ckkysb0nsqmfz08nwf17nl"
+       ManagementToken         = "e687950a23c3a9bceec28c6223a06c79"
        ActiveUserUUID          = "zzzzz-tpzed-xurymjxw79nv3jz"
        FederatedActiveUserUUID = "zbbbb-tpzed-xurymjxw79nv3jz"
        SpectatorUserUUID       = "zzzzz-tpzed-l1s2piq4t4mps8r"

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 6514c2af4597f145863b8059096e61845ac72607..1a146e26d50ad5d12cda9728414e1dac25050b00 100644 (file)
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -331,6 +331,19 @@ def run(leave_running_atexit=False):
         os.makedirs(gitdir)
     subprocess.check_output(['tar', '-xC', gitdir, '-f', gittarball])
 
+    # Customizing the passenger config template is the only documented
+    # way to override the default passenger_stat_throttle_rate (10 s).
+    # In the testing environment, we want restart.txt to take effect
+    # immediately.
+    resdir = subprocess.check_output(['bundle', 'exec', 'passenger-config', 'about', 'resourcesdir']).decode().rstrip()
+    with open(resdir + '/templates/standalone/config.erb') as f:
+        template = f.read()
+    newtemplate = re.sub('http {', 'http {\n        passenger_stat_throttle_rate 0;', template)
+    if newtemplate == template:
+        raise "template edit failed"
+    with open('tmp/passenger-nginx.conf.erb', 'w') as f:
+        f.write(newtemplate)
+
     port = internal_port_from_config("RailsAPI")
     env = os.environ.copy()
     env['RAILS_ENV'] = 'test'
@@ -344,6 +357,12 @@ def run(leave_running_atexit=False):
     railsapi = subprocess.Popen(
         ['bundle', 'exec',
          'passenger', 'start', '-p{}'.format(port),
+         '--nginx-config-template', 'tmp/passenger-nginx.conf.erb',
+        '--no-friendly-error-pages',
+        '--disable-anonymous-telemetry',
+        '--disable-security-update-check',
+        '--no-compile-runtime',
+        '--no-install-runtime',
          '--pid-file', pid_file,
          '--log-file', '/dev/stdout',
          '--ssl',

diff --git a/services/api/config/initializers/reload_config.rb b/services/api/config/initializers/reload_config.rb
new file mode 100644 (file)
index 0000000..af266ca
--- /dev/null
+++ b/services/api/config/initializers/reload_config.rb
@@ -0,0 +1,35 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+if !File.owned?(Rails.root.join('tmp'))
+  Rails.logger.debug("reload_config: not owner of #{Rails.root}/tmp, skipping")
+else
+  Thread.new do
+    lockfile = Rails.root.join('tmp', 'reload_config.lock')
+    File.open(lockfile, File::WRONLY|File::CREAT, 0600) do |f|
+      # Note we don't use LOCK_NB here. If we did, each time passenger
+      # kills the lock-holder process, we would be left with nobody
+      # checking for updates until passenger starts a new worker,
+      # which could be a long time.
+      Rails.logger.debug("reload_config: waiting for lock on #{lockfile}")
+      f.flock(File::LOCK_EX)
+      conffile = ENV['ARVADOS_CONFIG'] || "/etc/arvados/config.yml"
+      Rails.logger.info("reload_config: polling for updated mtime on #{conffile} with threshold #{Rails.configuration.SourceTimestamp}")
+      while true
+        sleep 1
+        t = File.mtime(conffile)
+        if t.to_f > Rails.configuration.SourceTimestamp.to_f
+          restartfile = Rails.root.join('tmp', 'restart.txt')
+          touchtime = Time.now
+          Rails.logger.info("reload_config: mtime on #{conffile} changed to #{t}, touching #{restartfile} to #{touchtime}")
+          File.utime(touchtime, touchtime, restartfile)
+          # Even if passenger doesn't notice that we hit restart.txt
+          # and kill our process, there's no point waiting around to
+          # hit it again.
+          break
+        end
+      end
+    end
+  end
+end