Merge pull request #2983 from zendesk/grosser/move

do not check github during our requests

.env.bootstrap

@@ -5,7 +5,7 @@ AUTH_GITHUB=true
 RAILS_MIN_THREADS=2
 RAILS_MAX_THREADS=10
 CACHE_STORE=memory
-PERIODICAL=stop_expired_deploys:60,remove_expired_locks:60,report_system_stats:60,report_process_stats:60,periodical_deploy:86400,cancel_stalled_builds:3600
+PERIODICAL=stop_expired_deploys:60,remove_expired_locks:60,report_system_stats:60,report_process_stats:60,periodical_deploy:86400,cancel_stalled_builds:3600,repo_provider_status:60
 
 # random string, do not use in live server
 bin/decode_dot_env-U0VDUkVUX1RPS0VOPWUzNzExYzc3N2I0OWE5YmE2ZjRjZDQ0NTM5ZjRjNjc3ZTc2ZjdjMDhjMzQ2ODc1ZTUwYjExMTE5YzYxODM5ZDM4NWIyNzA5ZjFmZDhhYzJkODk5YjMyZGM4MThkMTQ1OWIyNjVjZmY5MWY2ZGNjNjM1NDA2YTQ3M2NkOTAzZjRh

.env.example

@@ -86,8 +86,9 @@ GITHUB_TOKEN=
 # report_process_stats:60
 # periodical_deploy:86400
 # cancel_stalled_builds:3600
+# repo_provider_status:60 (for plugins that use repo_provider_status hook, like samson_github)
 # Recommended setup:
-PERIODICAL=stop_expired_deploys:60,remove_expired_locks:60,report_system_stats:60,report_process_stats:60,periodical_deploy:86400,cancel_stalled_builds:3600
+PERIODICAL=stop_expired_deploys:60,remove_expired_locks:60,report_system_stats:60,report_process_stats:60,periodical_deploy:86400,cancel_stalled_builds:3600,repo_provider_status:60
 
 ## Buddy Check feature: deploys to production require a buddy
 # BUDDY_CHECK_FEATURE=1 # enable

app.json

@@ -59,11 +59,12 @@
     },
     "CACHE_STORE": {
       "description": "Caching backend",
-      "value": "memcached"
+      "value": "memcached",
+      "required": true
     },
     "PERIODICAL": {
       "description": "Tasks to run periodically inside the server process",
-      "value": "stop_expired_deploys:60,remove_expired_locks:60,report_system_stats:60,report_process_stats:60,periodical_deploy:86400,cancel_stalled_builds:3600"
+      "value": "stop_expired_deploys:60,remove_expired_locks:60,report_system_stats:60,report_process_stats:60,periodical_deploy:86400,cancel_stalled_builds:3600,repo_provider_status:60"
     }
   },
   "scripts": {

app/helpers/application_helper.rb

@@ -10,8 +10,6 @@ module ApplicationHelper
   include DateTimeHelper
   include Pagy::Frontend
 
-  cattr_reader(:github_status_cache_key) { 'github-status-ok' }
-
   def render_log(str)
     escaped = ERB::Util.html_escape(str)
     autolink(ansi_escaped(escaped)).html_safe
@@ -55,28 +53,6 @@ def sortable(column, title = nil)
     link_to title, sort: column, direction: direction
   end
 
-  def github_ok?
-    key = github_status_cache_key
-
-    old = Rails.cache.read(key)
-    return old unless old.nil?
-
-    status =
-      begin
-        status_url = Rails.application.config.samson.github.status_url
-        response = Faraday.get("#{status_url}/api/status.json") do |req|
-          req.options.timeout = req.options.open_timeout = 1
-        end
-
-        response.status == 200 && JSON.parse(response.body)['status'] == 'good'
-      rescue Faraday::ClientError
-        false
-      end
-
-    Rails.cache.write(key, status, expires_in: (status ? 5.minutes : 30.seconds))
-    !!status
-  end
-
   def breadcrumb(*items)
     items = items.map do |item|
       if item.is_a?(ActiveRecord::Base)

app/views/layouts/application.html.erb

@@ -19,11 +19,10 @@
       <div class="container">
         <%= render "layouts/alerts" %>
 
-        <% unless github_ok? %>
+        <% Samson::RepoProviderStatus.errors.each do |error| %>
           <div class="alert alert-warning alert-dismissable">
-            <%= image_tag image_url('github.png'), style: 'height: 20px; background: #8a6d3b' %>
             <button type="button" class="close" data-dismiss="alert" aria-hidden="true">&times;</button>
-            GitHub may be having problems. Please monitor their <a href="<%= Rails.application.config.samson.github.status_url %>">status page</a> for more details.
+            <%= autolink(error).html_safe %></li><br/>
           </div>
         <% end %>
 

app/views/releases/_release.html.erb

@@ -3,11 +3,7 @@
     <%= release_label(@project, release) %>
   </td>
   <td>
-    <% if github_ok? %>
-      <%= github_commit_status_icon CommitStatus.new(release.project, release.commit).state %>
-    <% else %>
-      <%= github_commit_status_icon "missing" %>
-    <% end %>
+    <%= github_commit_status_icon CommitStatus.new(release.project, release.commit).state %>
   </td>
   <td>
     <span style="white-space: nowrap">

config/application.rb

@@ -104,7 +104,6 @@ class Application < Rails::Application
     config.samson.github.deploy_team = ENV["GITHUB_DEPLOY_TEAM"].presence
     config.samson.github.web_url = deprecated_url.call("GITHUB_WEB_URL") || 'https://github.com'
     config.samson.github.api_url = deprecated_url.call("GITHUB_API_URL") || 'https://api.github.com'
-    config.samson.github.status_url = deprecated_url.call("GITHUB_STATUS_URL") || 'https://status.github.com'
 
     # Configuration for LDAP
     config.samson.ldap = ActiveSupport::OrderedOptions.new

config/initializers/periodical.rb

@@ -37,13 +37,17 @@
   Samson::PeriodicalDeploy.run
 end
 
+Samson::Periodical.register :report_process_stats, "Report process stats" do
+  Samson::ProcessUtils.report_to_statsd
+end
+
+Samson::Periodical.register :repo_provider_status, "Refresh repo provider status" do
+  Samson::RepoProviderStatus.refresh
+end
+
 if ENV['SERVER_MODE']
   Rails.application.config.after_initialize do
     Samson::Periodical.enabled = true
     Samson::Periodical.run
   end
 end
-
-Samson::Periodical.register :report_process_stats, "Report process stats" do
-  Samson::ProcessUtils.report_to_statsd
-end

lib/samson/hooks.rb

@@ -60,7 +60,8 @@ class UserError < StandardError
       :stage_permitted_params,
       :trace_method,
       :trace_scope,
-      :asynchronous_performance_tracer
+      :asynchronous_performance_tracer,
+      :repo_provider_status
     ].freeze
 
     # Hooks that are slow and we want performance info on

lib/samson/repo_provider_status.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+module Samson
+  class RepoProviderStatus
+    CACHE_KEY = name
+
+    class << self
+      def errors
+        (
+          Rails.cache.read(CACHE_KEY) ||
+          ["To see repo provider status information, add repo_provider_status:60 to PERIODICAL environment variable."]
+        )
+      end
+
+      def refresh
+        Rails.cache.write(CACHE_KEY, Samson::Hooks.fire(:repo_provider_status).compact)
+      end
+    end
+  end
+end

plugins/github/lib/samson_github/samson_plugin.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 module SamsonGithub
+  STATUS_URL = ENV["GITHUB_STATUS_URL"] || 'https://status.github.com'
+
   class Engine < Rails::Engine
   end
 end
@@ -28,3 +30,15 @@ class Engine < Rails::Engine
     GithubDeployment.new(deploy).update(deployment)
   end
 end
+
+Samson::Hooks.callback :repo_provider_status do
+  error = "GitHub may be having problems. Please check their status page #{SamsonGithub::STATUS_URL} for details."
+  begin
+    response = Faraday.get("#{SamsonGithub::STATUS_URL}/api/status.json") do |req|
+      req.options.timeout = req.options.open_timeout = 1
+    end
+    error unless response.status == 200 && JSON.parse(response.body)['status'] == 'good'
+  rescue StandardError
+    error
+  end
+end

plugins/github/test/samson_github/samson_plugin_test.rb

@@ -58,4 +58,38 @@
       Samson::Hooks.fire(:before_deploy, deploy, nil)
     end
   end
+
+  describe :repo_provider_status do
+    def fire
+      Samson::Hooks.fire(:repo_provider_status)
+    end
+
+    let(:status_url) { "#{SamsonGithub::STATUS_URL}/api/status.json" }
+
+    around { |t| Samson::Hooks.only_callbacks_for_plugin('github', :repo_provider_status, &t) }
+
+    it "reports good response" do
+      assert_request(:get, status_url, to_return: {body: {status: 'good'}.to_json}) do
+        fire.must_equal [nil]
+      end
+    end
+
+    it "reports bad response" do
+      assert_request(:get, status_url, to_return: {body: {status: 'bad'}.to_json}) do
+        fire.to_s.must_include "GitHub may be having problems"
+      end
+    end
+
+    it "reports invalid response" do
+      assert_request(:get, status_url, to_return: {status: 400}) do
+        fire.to_s.must_include "GitHub may be having problems"
+      end
+    end
+
+    it "reports errors" do
+      assert_request(:get, status_url, to_timeout: []) do
+        fire.to_s.must_include "GitHub may be having problems"
+      end
+    end
+  end
 end

test/helpers/application_helper_test.rb

@@ -169,48 +169,6 @@
     end
   end
 
-  describe "#github_ok?" do
-    let(:status_url) { "#{Rails.application.config.samson.github.status_url}/api/status.json" }
-
-    it "returns cached true" do
-      Rails.cache.write(github_status_cache_key, true)
-      assert github_ok?
-    end
-
-    it "returns cached false" do
-      Rails.cache.write(github_status_cache_key, false)
-      refute github_ok?
-    end
-
-    it "caches good response" do
-      assert_request(:get, status_url, to_return: {body: {status: 'good'}.to_json}) do
-        assert github_ok?
-        Rails.cache.read(github_status_cache_key).must_equal true
-      end
-    end
-
-    it "caches bad response" do
-      assert_request(:get, status_url, to_return: {body: {status: 'bad'}.to_json}) do
-        refute github_ok?
-        Rails.cache.read(github_status_cache_key).must_equal false
-      end
-    end
-
-    it "caches invalid response" do
-      assert_request(:get, status_url, to_return: {status: 400}) do
-        refute github_ok?
-        Rails.cache.read(github_status_cache_key).must_equal false
-      end
-    end
-
-    it "caches timeout" do
-      assert_request(:get, status_url, to_timeout: []) do
-        refute github_ok?
-        Rails.cache.read(github_status_cache_key).must_equal false
-      end
-    end
-  end
-
   describe "#breadcrumb" do
     let(:stage) { stages(:test_staging) }
     let(:project) { projects(:test) }

test/integration/cleanliness_test.rb

@@ -274,4 +274,13 @@ def assert_content(files)
       end
     end
   end
+
+  it "uses/recommends consistent PERIODICAL" do
+    values = [
+      File.read('.env.bootstrap')[/PERIODICAL=(.*)/, 1],
+      JSON.parse(File.read('app.json')).dig("env", "PERIODICAL", "value"),
+      File.read('.env.example')[/PERIODICAL=(.*)/, 1],
+    ]
+    values.uniq.size.must_equal 1, "Expected all places to use consistent PERIODICAL value, but found #{values.inspect}"
+  end
 end

test/lib/samson/periodical_test.rb

@@ -196,12 +196,13 @@ def call(*args)
   end
 
   it "lists all example periodical tasks in the .env.example" do
-    configureable = File.read('config/initializers/periodical.rb').scan(/\.register.*?:([a-z\d_]+)/)
-    mentioned = File.read('.env.example')[/## Periodical tasks .*^PERIODICAL=/m].scan(/# ([a-z\d_]+):\d+/)
+    configureable = File.read('config/initializers/periodical.rb').scan(/\.register.*?:([a-z\d_]+)/).flatten
+    mentioned = File.read('.env.example')[/## Periodical tasks .*^PERIODICAL=/m].scan(/# ([a-z\d_]+):\d+/).flatten
     configureable.sort.must_equal mentioned.sort
   end
 
   it "runs everything" do
+    stub_request(:get, "https://status.github.com/api/status.json")
     Samson::Periodical.send(:registered).each_key do |task|
       Samson::Periodical.run_once task
     end

test/lib/samson/repo_provider_status_test.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+require_relative "../../test_helper"
+
+SingleCov.covered!
+
+describe Samson::RepoProviderStatus do
+  describe ".errors" do
+    it "shows error when periodical is not running" do
+      Samson::RepoProviderStatus.errors.first.must_include "PERIODICAL"
+    end
+
+    it "is empty when everything is fine" do
+      Samson::Hooks.with_callback(:repo_provider_status) do
+        Samson::RepoProviderStatus.refresh
+      end
+      Samson::RepoProviderStatus.errors.must_equal []
+    end
+
+    it "can show multiple errors" do
+      Samson::Hooks.with_callback(:repo_provider_status, -> { "Foo" }, -> { nil }, -> { "Bar" }) do
+        Samson::RepoProviderStatus.refresh
+      end
+      Samson::RepoProviderStatus.errors.must_equal ["Foo", "Bar"]
+    end
+  end
+end

test/test_helper.rb

@@ -198,10 +198,6 @@ def with_registries(registries)
     ENV['DOCKER_REGISTRIES'] = old
   end
 
-  def stub_github_status_check
-    stub_request(:get, "#{Rails.application.config.samson.github.status_url}/api/status.json").to_return(body: "{}")
-  end
-
   def silence_thread_exceptions
     old = Thread.report_on_exception
     Thread.report_on_exception = false
@@ -297,7 +293,6 @@ def use_test_routes(controller)
     middleware = Rails.application.config.middleware.detect { |m| m.name == 'Warden::Manager' }
     manager = Warden::Manager.new(nil, &middleware.block)
     request.env['warden'] = Warden::Proxy.new(request.env, manager)
-    stub_github_status_check
   end
 
   after do
@@ -333,9 +328,3 @@ def process(*args)
     end
   end)
 end
-
-ActionDispatch::IntegrationTest.class_eval do
-  before do
-    stub_github_status_check
-  end
-end