From 847b813a5c026b8ba9bef80281ea27417bf165da Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Fri, 20 Dec 2024 17:01:45 +0100 Subject: [PATCH] Support for semicolon-separated list of tags on create calls (#1127) * initial implementation of support for comma-separated tag list * re-sort the concatenated id_loads_tagged array by load * fix the untagged server selection * allow 'none' as a special tag to explicitly reference untagged servers * document extended tag feature * add tests for extended tag feature * doc fix * switch tag list delimiter from ',' to ';' (for better compatibility with config in Greenlight) --------- Co-authored-by: Ahmad Farhat --- app/models/server.rb | 35 ++++++++++++++++++++--------- docs/tags-README.md | 27 ++++++++++++++++++---- spec/models/server_spec.rb | 44 ++++++++++++++++++++++++++++++++++++ test/models/server_test.rb | 46 +++++++++++++++++++++++++++++++++++++- 4 files changed, 136 insertions(+), 16 deletions(-) diff --git a/app/models/server.rb b/app/models/server.rb index d92a0b5c..2d129cdf 100644 --- a/app/models/server.rb +++ b/app/models/server.rb @@ -286,24 +286,37 @@ def self.find(id) end # Find the server with the lowest load (for creating a new meeting) - def self.find_available(tag_arg = nil) - # Check if tag is required - tag = tag_arg.presence - tag_required = false - if !tag.nil? && tag[-1] == '!' - tag = tag[0..-2].presence # always returns String, if tag is String - tag_required = true + def self.find_available(tags_arg = nil) + # Check if passed tags are required + tags_arg = tags_arg.presence + tags_required = false + if !tags_arg.nil? && tags_arg[-1] == '!' + tags_arg = tags_arg[0..-2].presence # always returns String, if tag is String + tags_required = true unless tags_arg.nil? end + tags = tags_arg&.split(';') # Find available&matching server with the lowest load with_connection do |redis| ids_loads = redis.zrange('server_load', 0, -1, with_scores: true) raise RecordNotFound.new("Could not find any available servers.", name, nil) if ids_loads.blank? - if !tag.nil? && ids_loads.none? { |myid, _| redis.hget(key(myid), 'tag') == tag } - raise BBBErrors::ServerTagUnavailableError, tag if tag_required - tag = nil # fall back to servers without tag + + # build a list of matching tagged servers, otherwise fall back to untagged + unless tags.nil? + ids_loads_tagged = [] + tags.each do |tag| + ids_loads_tagged.concat ids_loads.select { |myid, _| (redis.hget(key(myid), 'tag') || "none") == tag } + end + ids_loads_tagged.sort_by! { |id_load| id_load[1] } end - ids_loads = ids_loads.select { |myid, _| redis.hget(key(myid), 'tag') == tag } + if ids_loads_tagged.blank? + raise BBBErrors::ServerTagUnavailableError, tags_arg if tags_required + tags = nil # fall back to servers without tag + ids_loads_tagged = ids_loads.select { |myid, _| redis.hget(key(myid), 'tag').nil? } + end + ids_loads = ids_loads_tagged + + # try to select the server with lowest load id, load, hash = ids_loads.each do |id, load| hash = redis.hgetall(key(id)) break id, load, hash if hash.present? diff --git a/docs/tags-README.md b/docs/tags-README.md index 9b3fe176..2f9b5939 100644 --- a/docs/tags-README.md +++ b/docs/tags-README.md @@ -12,11 +12,30 @@ This works for all supported ways of adding servers: Per `rake servers:add` task ## Create call with server-tag metaparameter -A create API call with a meta feature is supposed to work as follows: +A create API call using this feature is supposed to work as follows: -1) When making a "create" API call towards Scalelite, you can optionally pass a meta_server-tag string as parameter. If passed, it will be handled as follows: -2) If the last character of meta_server-tag is not a '!', the tag will will be intepreted as *optional*. The meeting will be created on the lowest load server with the corresponding tag, if any is available, or on the lowest load untagged (i.e. tag == nil) server otherwise. -3) If the last character of meta_server-tag is a '!', this character will be stripped and the remaining tag will be interpreted as *required*. The meeting will be created on the lowest load server with the corresponding tag or fail to be created (with specific error message), if no matching server is available. +1) When making a "create" API call towards Scalelite, you can optionally pass a meta_server-tag parameter with a string value. The string can be a single tag or a semicolon-separated list of tags and may additionally contain a '!' as last character. It will be handled as follows: +2) If the last character of meta_server-tag is not a '!', the tags will will be intepreted as *optional*. The meeting will be created on the least loaded server with a tag matching one of the passed tags (the special tag 'none' will match untagged servers), if any is available, or on the least loaded untagged server otherwise. +3) If the last character of meta_server-tag is a '!', this character will be stripped and the remaining tags will be interpreted as *required*. The meeting will be created on the least loaded server with a tag matching one of the passed tags (the special tag 'none' will match untagged servers) or *fail* to be created (with a specific error message), if no matching server is available. NOTE: Create calls without or with ''/'!' as meta_server-tag will only match untagged servers. So, for a frontend unaware of the feature, SL will behave as previously if a pool of untagged ("default") servers is maintained. It is recommended to always add your default servers as untagged servers. +### Examples + +Consider the following setup: +`$ bundle exec rake status` +``` +HOSTNAME STATE STATUS MEETINGS USERS LARGEST MEETING VIDEOS LOAD BBB VERSION TAG + bbb-1 enabled online 1 2 2 0 3.0 3.0.0 test + bbb-2 enabled online 1 1 1 0 2.0 3.0.0 + bbb-3 enabled online 0 0 0 0 0.0 3.0.0 + bbb-4 enabled online 1 1 1 0 2.0 3.0.0 test2 + ``` + +Now, consider the following examples of `meta_server-tag` parameters: +- Passing `meta_server-tag=` or `meta_server-tag=!` or omitting the parameter altogether are all equivalent and will place the meeting on `bbb-3` (least loaded untagged). +- Passing `meta_server-tag=test` or `meta_server-tag=test!` will place the meeting on `bbb-1` (the only match). +- Passing `meta_server-tag=test;test2` or `meta_server-tag=test;test2!` will place the meeting on `bbb-4` (least loaded match). +- Passing `meta_server-tag=none` or `meta_server-tag=none!` will place the meeting on `bbb-3` ) (least loaded match). +- Passing `meta_server-tag=test3` will place the meeting on `bbb-3` (fallback to least loaded untagged). +- Passing `meta_server-tag=test3!` will place the meeting on `bbb-3` (fallback to least loaded untagged). \ No newline at end of file diff --git a/spec/models/server_spec.rb b/spec/models/server_spec.rb index 54ca5b95..1826770d 100644 --- a/spec/models/server_spec.rb +++ b/spec/models/server_spec.rb @@ -339,6 +339,22 @@ expect(server.id).to eq 'test-3' end end + + context 'and with none tag argument' do + let(:server) { described_class.find_available('none') } + + it 'returns untagged server with lowest load' do + expect(server.id).to eq 'test-3' + end + end + + context 'and with none! tag argument' do + let(:server) { described_class.find_available('none!') } + + it 'returns untagged server with lowest load' do + expect(server.id).to eq 'test-3' + end + end end context 'with differently tagged servers' do @@ -366,6 +382,11 @@ redis.sadd?('servers', 'test-4') redis.sadd?('server_enabled', 'test-4') redis.zadd('server_load', 1, 'test-4') + redis.mapped_hmset('server:test-5', url: 'https://test-5.example.com/bigbluebutton/api', secret: 'test-5-secret', + tag: 'test-tag2', enabled: 'true') + redis.sadd?('servers', 'test-5') + redis.sadd?('server_enabled', 'test-5') + redis.zadd('server_load', 1, 'test-5') end end @@ -391,6 +412,29 @@ expect(server.tag).to eq 'test-tag' end end + + context 'and optional tag list argument' do + let(:server) { described_class.find_available('test-tag;test-tag2') } + + it 'returns matching tagged server with lowest load' do + expect(server.id).to eq 'test-5' + expect(server.url).to eq 'https://test-5.example.com/bigbluebutton/api' + expect(server.secret).to eq 'test-5-secret' + expect(server.tag).to eq 'test-tag2' + expect(server.enabled).to be true + expect(server.state).to be_nil + expect(server.load).to eq 1 + end + end + + context 'and required tag list argument' do + let(:server) { described_class.find_available('test-tag;test-tag2!') } + + it 'returns matching tagged server with lowest load' do + expect(server.id).to eq 'test-5' + expect(server.tag).to eq 'test-tag2' + end + end end context 'with no matching tagged servers' do diff --git a/test/models/server_test.rb b/test/models/server_test.rb index 57ef0070..2761bfcd 100644 --- a/test/models/server_test.rb +++ b/test/models/server_test.rb @@ -243,7 +243,7 @@ class ServerTest < ActiveSupport::TestCase end end - test 'Server find_available without or with empty tag returns untagged server with lowest load' do + test 'Server find_available without or with empty tag or with none tag returns untagged server with lowest load' do RedisStore.with_connection do |redis| redis.mapped_hmset('server:test-1', url: 'https://test-1.example.com/bigbluebutton/api', secret: 'test-1-secret', tag: 'test-tag', enabled: 'true') @@ -276,6 +276,12 @@ class ServerTest < ActiveSupport::TestCase server = Server.find_available('!') assert_equal('test-3', server.id) + + server = Server.find_available('none') + assert_equal('test-3', server.id) + + server = Server.find_available('none!') + assert_equal('test-3', server.id) end test 'Server find_available with tag argument returns matching tagged server with lowest load' do @@ -316,6 +322,44 @@ class ServerTest < ActiveSupport::TestCase assert_equal('test-tag', server.tag) end + test 'Server find_available with multiple tag arguments returns matching tagged server with lowest load' do + RedisStore.with_connection do |redis| + redis.mapped_hmset('server:test-1', url: 'https://test-1.example.com/bigbluebutton/api', secret: 'test-1-secret', + enabled: 'true') + redis.sadd?('servers', 'test-1') + redis.sadd?('server_enabled', 'test-1') + redis.zadd('server_load', 1, 'test-1') + redis.mapped_hmset('server:test-2', url: 'https://test-2.example.com/bigbluebutton/api', secret: 'test-2-secret', + tag: 'test-tag', enabled: 'true') + redis.sadd?('servers', 'test-2') + redis.sadd?('server_enabled', 'test-2') + redis.zadd('server_load', 3, 'test-2') + redis.mapped_hmset('server:test-3', url: 'https://test-3.example.com/bigbluebutton/api', secret: 'test-3-secret', + tag: 'test-tag2', enabled: 'true') + redis.sadd?('servers', 'test-3') + redis.sadd?('server_enabled', 'test-3') + redis.zadd('server_load', 2, 'test-3') + redis.mapped_hmset('server:test-4', url: 'https://test-4.example.com/bigbluebutton/api', secret: 'test-4-secret', + tag: 'wrong-tag', enabled: 'true') + redis.sadd?('servers', 'test-4') + redis.sadd?('server_enabled', 'test-4') + redis.zadd('server_load', 1, 'test-4') + end + + server = Server.find_available('test-tag;test-tag2') + assert_equal('test-3', server.id) + assert_equal('https://test-3.example.com/bigbluebutton/api', server.url) + assert_equal('test-3-secret', server.secret) + assert_equal('test-tag2', server.tag) + assert(server.enabled) + assert_nil(server.state) + assert_equal(2, server.load) + + server = Server.find_available('test-tag;test-tag2!') + assert_equal('test-3', server.id) + assert_equal('test-tag2', server.tag) + end + test 'Server find_available with optional tag returns untagged server with lowest load if no matching tagged server available' do RedisStore.with_connection do |redis| redis.mapped_hmset('server:test-1', url: 'https://test-1.example.com/bigbluebutton/api', secret: 'test-1-secret',