diff --git a/.travis.yml b/.travis.yml index a06a2d78..366fe0ce 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,6 @@ cache: bundler services: - redis - - rabbitmq addons: postgresql: 9.3 diff --git a/Gemfile b/Gemfile index 3abb910a..0cad3c43 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,8 @@ gem 'travis-rollout', git: 'https://github.com/travis-ci/travis-rollout', re gem 'travis-exceptions', git: 'https://github.com/travis-ci/travis-exceptions' gem 'travis-logger', git: 'https://github.com/travis-ci/travis-logger' gem 'travis-settings', git: 'https://github.com/travis-ci/travis-settings' +gem 'travis-encrypt', git: 'https://github.com/travis-ci/travis-encrypt' +gem 'settings', git: 'https://github.com/travis-ci/settings' gem 'gh', git: 'https://github.com/travis-ci/gh' gem 'coder', git: 'https://github.com/rkh/coder' diff --git a/Gemfile.lock b/Gemfile.lock index b624b391..2f1e4bba 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -16,6 +16,18 @@ GIT net-http-persistent (~> 2.9) net-http-pipeline +GIT + remote: https://github.com/travis-ci/settings + revision: b590df5c1f6d2e9f75e8b84cd8540e8b046318ce + specs: + settings (0.0.1) + +GIT + remote: https://github.com/travis-ci/travis-encrypt + revision: b91fac2529202dcc4f638146efa88ce08c9f2432 + specs: + travis-encrypt (0.0.5) + GIT remote: https://github.com/travis-ci/travis-exceptions revision: ab236981f810b820bc3ebfaf33f317bbaa9b3465 @@ -187,8 +199,10 @@ DEPENDENCIES rollout rspec sentry-raven + settings! sidekiq-pro! travis-config (~> 1.1.3) + travis-encrypt! travis-exceptions! travis-lock travis-logger! diff --git a/Rakefile b/Rakefile index 035c3e92..f09d3fe4 100644 --- a/Rakefile +++ b/Rakefile @@ -4,7 +4,7 @@ namespace :db do env = ENV["ENV"] || 'test' abort "Cannot run rake db:create in production." if env == 'production' - url = "https://raw.githubusercontent.com/travis-ci/travis-migrations/master/db/main/structure.sql" + url = "https://raw.githubusercontent.com/travis-ci/travis-migrations/sf-settings-tables/db/main/structure.sql" file = 'db/structure.sql' system "curl -fs #{url} -o #{file} --create-dirs" abort "failed to download #{url}" unless File.exist?(file) diff --git a/lib/travis/owners/db.rb b/lib/travis/owners/db.rb index 38b7664c..5596d9b0 100644 --- a/lib/travis/owners/db.rb +++ b/lib/travis/owners/db.rb @@ -9,6 +9,10 @@ def owners attrs.any? ? attrs.map { |(type, id)| find(type, id) } : [owner] end + def uuid + OwnerGroup.where(owner: owner).pluck(:uuid).first + end + private def find(type, id) diff --git a/lib/travis/owners/group.rb b/lib/travis/owners/group.rb index a29a260d..52668c02 100644 --- a/lib/travis/owners/group.rb +++ b/lib/travis/owners/group.rb @@ -1,3 +1,5 @@ +require 'travis/owners/record' + module Travis module Owners class Group < Struct.new(:all, :config) @@ -11,8 +13,12 @@ def key logins.join(':') end + def id + db.uuid + end + def logins - all.map(&:login) + all.map(&:login).sort end def max_jobs @@ -46,6 +52,10 @@ def subscriptions @subscriptions ||= Subscriptions.new(self, plans) end + def db + Db.new(all.first) + end + def plans config && config[:plans] || {} end diff --git a/lib/travis/scheduler/limit/by_queue.rb b/lib/travis/scheduler/limit/by_queue.rb index 86d08758..00ab5f41 100644 --- a/lib/travis/scheduler/limit/by_queue.rb +++ b/lib/travis/scheduler/limit/by_queue.rb @@ -1,5 +1,6 @@ require 'travis/scheduler/helper/context' require 'travis/scheduler/helper/logging' +require 'travis/scheduler/model/settings' module Travis module Scheduler @@ -8,7 +9,6 @@ class ByQueue < Struct.new(:context, :reports, :owners, :job, :selected, :state, include Helper::Context def enqueue? - return true unless enabled? return true unless queue == ENV['BY_QUEUE_NAME'] result = current < max report(max) if result @@ -17,8 +17,8 @@ def enqueue? private - def enabled? - config[owners.key] || ENV['BY_QUEUE_DEFAULT'] + def queue + job.queue ||= Queue.new(job, context.config, nil).select end def current @@ -26,30 +26,38 @@ def current end def max - config.fetch(owners.key, default).to_i + by_config || by_setting || default end - def queue - job.queue ||= Queue.new(job, context.config, nil).select + def by_config + config[owners.key].to_i if config.key?(owners.key) end - def repo - job.repository + def by_setting + # p settings[:by_queue_enabled].enabled? + settings[:by_queue_enabled].enabled? && settings[:by_queue].value end - def report(value) - reports << MSGS[:max] % [owners.to_s, "queue #{job.queue}", value] - value + def repo + job.repository end def default - ENV['BY_QUEUE_DEFAULT'].to_i + ENV.fetch('BY_QUEUE_DEFAULT', 2).to_i end - # TODO make this a repo setting at some point? def config @config ||= ENV['BY_QUEUE_LIMIT'].to_s.split(',').map { |pair| pair.split('=') }.to_h end + + def settings + @settings ||= Model::Settings.new(owners) + end + + def report(value) + reports << MSGS[:max] % [owners.to_s, "queue #{job.queue}", value] + value + end end end end diff --git a/lib/travis/scheduler/model/settings.rb b/lib/travis/scheduler/model/settings.rb new file mode 100644 index 00000000..6e56e88d --- /dev/null +++ b/lib/travis/scheduler/model/settings.rb @@ -0,0 +1,23 @@ +require 'settings' + +# hmmm. +Settings::Definition::OWNERS[:owners] = 'Travis::Owners::Group' + +module Travis + module Scheduler + module Model + class Settings < ::Settings::Group + int :by_queue, + owner: [:owners], + scope: :repo, + internal: true, + requires: :by_queue_enabled + + bool :by_queue_enabled, + owner: [:owners], + scope: :repo, + internal: true + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 465d06c3..fbe05fbe 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -22,6 +22,8 @@ RSpec.configure do |c| c.mock_with :mocha + + c.include FactoryGirl::Syntax::Methods c.include Support::Env c.include Support::Features c.include Support::Logger diff --git a/spec/support/env.rb b/spec/support/env.rb index 93e76240..ad3ad26d 100644 --- a/spec/support/env.rb +++ b/spec/support/env.rb @@ -12,7 +12,8 @@ def env(vars) end def define_env(vars) - vars.each { |key, value| ENV[key.to_s] = value.to_s } + resolve = ->(v) { v.is_a?(Proc) ? instance_exec(&v) : v } + vars.each { |key, value| ENV[key.to_s] = resolve.(value).to_s } end def undefine_env(vars) diff --git a/spec/support/factories.rb b/spec/support/factories.rb index 3a4875c9..75490a54 100644 --- a/spec/support/factories.rb +++ b/spec/support/factories.rb @@ -1,4 +1,5 @@ require 'factory_girl' +require 'settings' FactoryGirl.define do REPO_KEY = OpenSSL::PKey::RSA.generate(4096) @@ -12,6 +13,8 @@ login 'travis-ci' end + factory :owner_group + factory :subscription do valid_to Time.now + 24 * 3600 end @@ -75,5 +78,7 @@ author_name 'Sven Fuchs' author_email 'me@svenfuchs.com' end + + factory :setting, class: Settings::Record::Setting end diff --git a/spec/travis/scheduler/limit_spec.rb b/spec/travis/scheduler/limit_spec.rb index b7188e7e..abc028e2 100644 --- a/spec/travis/scheduler/limit_spec.rb +++ b/spec/travis/scheduler/limit_spec.rb @@ -1,8 +1,8 @@ describe Travis::Scheduler::Limit::Jobs do - let(:org) { FactoryGirl.create(:org, login: 'travis-ci') } - let(:repo) { FactoryGirl.create(:repo, owner: owner) } - let(:build) { FactoryGirl.create(:build) } - let!(:owner) { FactoryGirl.create(:user, login: 'svenfuchs') } + let(:org) { create(:org, login: 'travis-ci') } + let(:repo) { create(:repo, owner: owner) } + let(:build) { create(:build) } + let!(:owner) { create(:user, login: 'svenfuchs') } let(:owners) { Travis::Owners.group(data, config.to_h) } let(:context) { Travis::Scheduler.context } let(:redis) { context.redis } @@ -27,7 +27,7 @@ def create_jobs(count, attrs = {}) queueable: true, private: false } - 1.upto(count) { FactoryGirl.create(:job, defaults.merge(attrs)) } + 1.upto(count) { create(:job, defaults.merge(attrs)) } end describe 'with a boost limit 2' do @@ -43,7 +43,7 @@ def create_jobs(count, attrs = {}) describe 'with a subscription limit 1' do before { create_jobs(3) } - before { FactoryGirl.create(:subscription, valid_to: Time.now.utc, owner_type: owner.class.name, owner_id: owner.id, selected_plan: :one) } + before { create(:subscription, valid_to: Time.now.utc, owner_type: owner.class.name, owner_id: owner.id, selected_plan: :one) } before { subject } it { expect(subject.size).to eq 1 } @@ -107,7 +107,7 @@ def create_jobs(count, attrs = {}) before { config.limit.default = 1 } before { create_jobs(7, state: :created) } before { create_jobs(3, state: :started) } - before { FactoryGirl.create(:subscription, selected_plan: :seven, valid_to: Time.now.utc, owner_type: owner.class.name, owner_id: owner.id) } + before { create(:subscription, selected_plan: :seven, valid_to: Time.now.utc, owner_type: owner.class.name, owner_id: owner.id) } before { repo.settings.update_attributes!(maximum_number_of_builds: 5) } before { subject } @@ -117,7 +117,7 @@ def create_jobs(count, attrs = {}) it { expect(report).to include('user svenfuchs: total: 7, running: 3, queueable: 2') } end - describe 'with no by_queue config being given (enterprise)' do + describe 'with no by_queue config being given' do before { create_jobs(9, queue: 'builds.osx') } before { create_jobs(1, queue: 'builds.docker') } before { config.limit.default = 99 } @@ -127,96 +127,126 @@ def create_jobs(count, attrs = {}) it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 10') } end - describe 'with a default by_queue limit of 2 (org)' do + describe 'by_queue using env vars' do env BY_QUEUE_NAME: 'builds.osx' - env BY_QUEUE_DEFAULT: 2 - before { create_jobs(9, queue: 'builds.osx') } - before { create_jobs(1, queue: 'builds.docker') } - before { config.limit.default = 99 } - before { subject } + # there needs to be a record in the owner_groups table, so we have an uuid + before { create(:owner_group, owner: owner, uuid: SecureRandom.uuid) } - it { expect(subject.size).to eq 3 } - it { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } - it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 3') } - end + describe 'with a default by_queue limit of 2' do + before { create_jobs(9, queue: 'builds.osx') } + before { create_jobs(1, queue: 'builds.docker') } + before { config.limit.default = 99 } + before { subject } - describe 'with a queue name set, but now default or owner config given (com)' do - env BY_QUEUE_NAME: 'builds.osx' + it { expect(subject.size).to eq 3 } + xit { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } + xit { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 3') } + end - before { create_jobs(9, queue: 'builds.osx') } - before { create_jobs(1, queue: 'builds.docker') } - before { config.limit.default = 99 } - before { ENV['BY_QUEUE_NAME'] = 'builds.osx' } - after { ENV.delete('BY_QUEUE_NAME') } - before { subject } + describe 'with a default by_queue limit of 2' do + env BY_QUEUE_DEFAULT: 2 - it { expect(subject.size).to eq 10 } - it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 10') } - end + before { create_jobs(9, queue: 'builds.osx') } + before { create_jobs(1, queue: 'builds.docker') } + before { config.limit.default = 99 } + before { subject } - describe 'with a by_queue limit of 2 for the owner' do - env BY_QUEUE_NAME: 'builds.osx' - env BY_QUEUE_LIMIT: 'svenfuchs=2' + it { expect(subject.size).to eq 3 } + it { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } + it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 3') } + end - before { create_jobs(9, queue: 'builds.osx') } - before { create_jobs(1, queue: 'builds.docker') } - before { config.limit.default = 99 } - before { subject } + describe 'with a by_queue limit of 2 for the owner' do + env BY_QUEUE_LIMIT: -> { "#{owner.login}=2" } - it { expect(subject.size).to eq 3 } - it { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } - it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 3') } - end + before { create_jobs(9, queue: 'builds.osx') } + before { create_jobs(1, queue: 'builds.docker') } + before { config.limit.default = 99 } + before { subject } - describe 'with a by_queue limit of 2 for the owner, and a default given' do - env BY_QUEUE_NAME: 'builds.osx' - env BY_QUEUE_LIMIT: 'svenfuchs=2' - env BY_QUEUE_DEFAULT: 2 + it { expect(subject.size).to eq 3 } + it { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } + it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 3') } + end - before { create_jobs(9, queue: 'builds.osx') } - before { create_jobs(1, queue: 'builds.docker') } - before { config.limit.default = 99 } - before { subject } + describe 'with a by_queue limit of 2 for the owner and a repo limit of 3 on another repo' do + let(:other) { create(:repo, github_id: 2) } - it { expect(subject.size).to eq 3 } - it { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } - it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 3') } - end + env BY_QUEUE_LIMIT: -> { "#{owner.login}=2" } - describe 'with a by_queue limit of 2 for the owner and a repo limit of 3 on another repo' do - env BY_QUEUE_NAME: 'builds.osx' - env BY_QUEUE_LIMIT: 'svenfuchs=2' - env BY_QUEUE_DEFAULT: 2 + before { create_jobs(9, repository: repo, queue: 'builds.osx') } + before { create_jobs(5, repository: other, queue: 'builds.docker') } + before { config.limit.default = 99 } + before { other.settings.update_attributes!(maximum_number_of_builds: 3) } + before { subject } - let(:other) { FactoryGirl.create(:repo, github_id: 2) } - before { create_jobs(9, repository: repo, queue: 'builds.osx') } - before { create_jobs(5, repository: other, queue: 'builds.docker') } - before { config.limit.default = 99 } - before { other.settings.update_attributes!(maximum_number_of_builds: 3) } - before { subject } + it { expect(subject.size).to eq 5 } + it { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } + it { expect(report).to include('user svenfuchs: total: 14, running: 0, queueable: 5') } + end + + describe 'with a by_queue limit for the owner and jobs created for a different queue' do + env BY_QUEUE_LIMIT: -> { "#{owner.login}=2" } - it { expect(subject.size).to eq 5 } - it { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } - it { expect(report).to include('user svenfuchs: total: 14, running: 0, queueable: 5') } + before { create_jobs(1, queue: 'builds.osx') } + before { create_jobs(9, queue: 'builds.docker') } + before { config.limit.default = 5 } + before { subject } + + it { expect(subject.size).to eq 5 } + it { expect(report).to include('max jobs for user svenfuchs by default: 5') } + it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 5') } + end end - describe 'with a by_queue limit for the owner and jobs created for a different queue' do + describe 'by_queue using settings' do + let(:settings) { Travis::Scheduler::Model::Settings.new(owners) } + before { settings[:by_queue_enabled].enable } env BY_QUEUE_NAME: 'builds.osx' - env BY_QUEUE_LIMIT: 'svenfuchs=2' - before { create_jobs(9, queue: 'builds.docker') } - before { create_jobs(1, queue: 'builds.osx') } - before { config.limit.default = 5 } - before { subject } + describe 'with a by_queue limit of 2 for the owner' do + before { create_jobs(9, queue: 'builds.osx') } + before { create_jobs(1, queue: 'builds.docker') } + before { config.limit.default = 99 } + before { settings[:by_queue].set(2) } + before { subject } + + it { expect(subject.size).to eq 3 } + it { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } + it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 3') } + end + + describe 'with a by_queue limit of 2 for the owner and a repo limit of 3 on another repo' do + let(:other) { create(:repo, github_id: 2) } + + before { create_jobs(9, repository: repo, queue: 'builds.osx') } + before { create_jobs(5, repository: other, queue: 'builds.docker') } + before { config.limit.default = 99 } + before { other.settings.update_attributes!(maximum_number_of_builds: 3) } + before { settings[:by_queue].set(2) } + before { subject } + + it { expect(subject.size).to eq 5 } + it { expect(report).to include('max jobs for user svenfuchs by queue builds.osx: 2') } + it { expect(report).to include('user svenfuchs: total: 14, running: 0, queueable: 5') } + end - it { expect(subject.size).to eq 5 } - it { expect(report).to include('max jobs for user svenfuchs by default: 5') } - it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 5') } + describe 'with a by_queue limit for the owner and jobs created for a different queue' do + before { create_jobs(1, queue: 'builds.osx') } + before { create_jobs(9, queue: 'builds.docker') } + before { config.limit.default = 5 } + before { settings[:by_queue].set(2) } + before { subject } + + it { expect(subject.size).to eq 5 } + it { expect(report).to include('max jobs for user svenfuchs by default: 5') } + it { expect(report).to include('user svenfuchs: total: 10, running: 0, queueable: 5') } + end end describe 'delegated accounts' do - let(:carla) { FactoryGirl.create(:user, login: 'carla') } + let(:carla) { create(:user, login: 'carla') } before { create_jobs(3, owner: owner, state: :created, queueable: true) } before { create_jobs(3, owner: org, state: :created, queueable: true) } @@ -226,7 +256,7 @@ def create_jobs(count, attrs = {}) before { config.limit.delegate = { owner.login => org.login, carla.login => org.login } } describe 'with one subscription' do - before { FactoryGirl.create(:subscription, selected_plan: :seven, valid_to: Time.now.utc, owner_type: org.class.name, owner_id: org.id) } + before { create(:subscription, selected_plan: :seven, valid_to: Time.now.utc, owner_type: org.class.name, owner_id: org.id) } before { subject } it { expect(subject.size).to eq 5 } @@ -236,8 +266,8 @@ def create_jobs(count, attrs = {}) end describe 'with multiple subscriptions' do - before { FactoryGirl.create(:subscription, selected_plan: :one, valid_to: Time.now.utc, owner_type: owner.class.name, owner_id: owner.id) } - before { FactoryGirl.create(:subscription, selected_plan: :seven, valid_to: Time.now.utc, owner_type: org.class.name, owner_id: org.id) } + before { create(:subscription, selected_plan: :one, valid_to: Time.now.utc, owner_type: owner.class.name, owner_id: owner.id) } + before { create(:subscription, selected_plan: :seven, valid_to: Time.now.utc, owner_type: org.class.name, owner_id: org.id) } before { subject } it { expect(subject.size).to eq 6 } @@ -248,9 +278,9 @@ def create_jobs(count, attrs = {}) end describe 'stages' do - let(:one) { FactoryGirl.create(:stage, number: 1) } - let(:two) { FactoryGirl.create(:stage, number: 2) } - let(:three) { FactoryGirl.create(:stage, number: 3) } + let(:one) { create(:stage, number: 1) } + let(:two) { create(:stage, number: 2) } + let(:three) { create(:stage, number: 3) } before { create_jobs(1, owner: owner, state: :created, stage: one, stage_number: '1.1') } before { create_jobs(1, owner: owner, state: :created, stage: one, stage_number: '1.2') } @@ -311,7 +341,7 @@ def create_jobs(count, attrs = {}) end describe 'no running jobs, 4 private and 4 public jobs waiting, two jobs subscription' do - before { FactoryGirl.create(:subscription, selected_plan: :two, valid_to: Time.now.utc, owner_type: owner.class.name, owner_id: owner.id) } + before { create(:subscription, selected_plan: :two, valid_to: Time.now.utc, owner_type: owner.class.name, owner_id: owner.id) } before { create_jobs(4, private: true) } before { create_jobs(4, private: false) }