From 1a3a8b0a7cc31e9bfb63c6001bc049a831e13dd1 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Tue, 26 Apr 2022 11:16:26 -0400 Subject: [PATCH] 18794: Restart RailsAPI workers when config file changes. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- lib/controller/rails_restart_test.go | 85 +++++++++++++++++++ sdk/go/arvadostest/fixtures.go | 2 +- sdk/python/tests/run_test_server.py | 19 +++++ .../api/config/initializers/reload_config.rb | 35 ++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 lib/controller/rails_restart_test.go create mode 100644 services/api/config/initializers/reload_config.rb diff --git a/lib/controller/rails_restart_test.go b/lib/controller/rails_restart_test.go new file mode 100644 index 0000000000..75dd22b4a9 --- /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 9281f51d0c..ec55725412 100644 --- 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 6514c2af45..1a146e26d5 100644 --- 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 index 0000000000..af266cae36 --- /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 -- 2.30.2