From 85f83ef204608ae5b1bc5f83f05a3123ca7871da Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Wed, 3 Apr 2024 00:09:24 +0200 Subject: [PATCH 1/7] removed .sh' from scalelite_prune_recordings such that it actually gets executed by cron + added short README section (#1055) --- bigbluebutton/README.md | 4 ++++ ...alelite_prune_recordings.sh => scalelite_prune_recordings} | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) rename bigbluebutton/{scalelite_prune_recordings.sh => scalelite_prune_recordings} (93%) diff --git a/bigbluebutton/README.md b/bigbluebutton/README.md index e4ca597f..22afdcd9 100644 --- a/bigbluebutton/README.md +++ b/bigbluebutton/README.md @@ -61,3 +61,7 @@ Finally (after switching back to root), set the `spool_dir` setting in `scalelit ### Other configurations If you need to customize the rsync command (for example, to pass the `--rsh` option to set up a tunnel), you can add extra rsync command line arguments via the `extra_rsync_opts` array in `scalelite.yml`. + +### Recording cleanup cronjob + +Copy the script `scalelite_prune_recordings` to `/etc/cron.daily` on the BBB server, to periodically clean up the local files of recordings that have been transferred to Scalelite. You may adjust the variables MAXAGE and EVENTS_MAXAGE to the number of days you would like to keep the local files on the BBB server. diff --git a/bigbluebutton/scalelite_prune_recordings.sh b/bigbluebutton/scalelite_prune_recordings similarity index 93% rename from bigbluebutton/scalelite_prune_recordings.sh rename to bigbluebutton/scalelite_prune_recordings index e4788f95..0dae538d 100755 --- a/bigbluebutton/scalelite_prune_recordings.sh +++ b/bigbluebutton/scalelite_prune_recordings @@ -1,6 +1,6 @@ -#!/bin/bash +#!/bin/bash -# Place this file in /etc/cron.daily +# Place this file in /etc/cron.daily and make sure it is executable # # Delete sent recording after 4 days From e5ecdaefe41548061355a886b8861d77863d1f65 Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Wed, 3 Apr 2024 00:09:39 +0200 Subject: [PATCH 2/7] prevent column line wrapping in output of rake status task (#1054) --- lib/tasks/status.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/status.rake b/lib/tasks/status.rake index 7fb3e21c..42110ebc 100644 --- a/lib/tasks/status.rake +++ b/lib/tasks/status.rake @@ -35,7 +35,7 @@ server.largest_meeting, server.videos, server.load) t.add_column('LOAD', &:load) end - puts table.pack + puts table.pack(max_table_width: nil) end def status_with_state(state) From e93f3f71204713d9256caac79e49797a39975205 Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Fri, 5 Apr 2024 18:15:54 +0200 Subject: [PATCH 3/7] Fixes & improvements for servers:yaml rake task (#1056) * use string keys over symbols in hash produced by servers:yaml task, such that YAML.safe_load consumes it correctly (with default arguments), and for better human write-/readability * fix 'enabled' value in servers:yaml output * fix server.enabled? in servers:yaml[verbose] * fix logic mistake which prevented state 'cordoned' to be outputted in output of servers:yaml[verbose] --- lib/server_sync.rb | 14 +++++++------- lib/tasks/servers.rake | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/server_sync.rb b/lib/server_sync.rb index 39367b00..1b644fc6 100644 --- a/lib/server_sync.rb +++ b/lib/server_sync.rb @@ -171,15 +171,15 @@ def self.sync(servers, mode = 'cordon', dryrun = false) def self.dump(verbose) Server.all.to_h do |server| info = { - url: server.url, - secret: server.secret, - load_multiplier: server.load_multiplier.to_f || 1.0, - enabled: server.enabled, + "url" => server.url, + "secret" => server.secret, + "load_multiplier" => server.load_multiplier.to_f || 1.0, + "enabled" => server.enabled?, } if verbose - info[:state] = server.state.presence || server.enabled ? 'enabled' : 'disabled' - info[:load] = server.load.presence || -1.0 - info[:online] = server.online + info["state"] = server.state.presence || (server.enabled? ? 'enabled' : 'disabled') + info["load"] = server.load.presence || -1.0 + info["online"] = server.online end [server.id, info] end diff --git a/lib/tasks/servers.rake b/lib/tasks/servers.rake index 1e396dc4..e0c0108b 100644 --- a/lib/tasks/servers.rake +++ b/lib/tasks/servers.rake @@ -200,7 +200,7 @@ namespace :servers do desc 'Return a yaml compatible with servers:sync' task :yaml, [:verbose] => :environment do |_t, args| - puts({ servers: ServerSync.dump(!!args.verbose) }.to_yaml) + puts({ "servers" => ServerSync.dump(!!args.verbose) }.to_yaml) end desc('List all meetings running in specific BigBlueButton servers') From a659cb5932eb609e524f79755747061ea1b4bd57 Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Fri, 5 Apr 2024 19:15:53 +0200 Subject: [PATCH 4/7] in server find_available, get server_load set only once and iterate through it until server with valid hash is found (#1052) --- app/models/server.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/models/server.rb b/app/models/server.rb index 8e360b81..1c1b9bcb 100644 --- a/app/models/server.rb +++ b/app/models/server.rb @@ -278,11 +278,9 @@ def self.find(id) # Find the server with the lowest load (for creating a new meeting) def self.find_available with_connection do |redis| - id, load, hash = 5.times do - ids_loads = redis.zrange('server_load', 0, 0, with_scores: true) - raise RecordNotFound.new("Couldn't find available Server", name, nil) if ids_loads.blank? - - id, load = ids_loads.first + ids_loads = redis.zrange('server_load', 0, -1, with_scores: true) + raise RecordNotFound.new("Couldn't find available Server", name, nil) if ids_loads.blank? + id, load, hash = ids_loads.each do |id, load| hash = redis.hgetall(key(id)) break id, load, hash if hash.present? end From 4fb8a7f1ff8144a19931e22b91e7315abea04427 Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Fri, 5 Apr 2024 21:16:26 +0200 Subject: [PATCH 5/7] Support for tagged servers (ready for review) (#1049) * initial minimal implementation of server tags in app/models/server.rb * change defaulting mechanism in find_available to fully preserve old behavior if argument tag not passed * now use meta_server-tag parameter from create API call as tag argument for find_available * update basic servers.rake tasks for new server tags * add support for optional tags and fall back only to untagged servers * improved comments in server.rb * add tests for tagged server.find_available * now raise specific error message when no server with required tag could be found * add tests for tagged create API * add support for tags in server API + corresponding RSpec * now consequently handle requests/tasks with empty or ! tags as nil and prevent tagging servers with empty string (turn it into nil) * finalize tag support in all rake tasks (server sync / status) + add task to set only the tag * store the actual server tag in meeting metadata (not the requested one) * refactor tag-related Rails tests and and migrate to RSpec * now always override/delete tag in create metadata + refactor tagged create API tests to implicitly check for correctly adjusted meta_server-tag * fix typo .nil -> .nil? * update api-README for tags * update rake-README for tags * add README for server tags --- app/controllers/api/servers_controller.rb | 12 +- .../bigbluebutton_api_controller.rb | 11 +- app/models/server.rb | 28 +++- app/services/server_update_service.rb | 6 + docs/api-README.md | 10 +- docs/rake-README.md | 26 ++- docs/tags-README.md | 22 +++ lib/server_sync.rb | 15 +- lib/tasks/servers.rake | 19 ++- lib/tasks/status.rake | 5 +- spec/models/server_spec.rb | 154 ++++++++++++++++++ .../bigbluebutton_api_controller_spec.rb | 62 +++++++ spec/requests/servers_controller_spec.rb | 30 +++- .../bigbluebutton_api_controller_test.rb | 92 +++++++++++ test/models/server_test.rb | 127 +++++++++++++++ 15 files changed, 592 insertions(+), 27 deletions(-) create mode 100644 docs/tags-README.md diff --git a/app/controllers/api/servers_controller.rb b/app/controllers/api/servers_controller.rb index 6cace03d..3e2d29da 100644 --- a/app/controllers/api/servers_controller.rb +++ b/app/controllers/api/servers_controller.rb @@ -18,6 +18,7 @@ class ServersController < ApplicationController # "id": String, # "url": String, # "secret": String, + # "tag": String, # "state": String, # "load": String, # "load_multiplier": String, @@ -49,6 +50,7 @@ def get_servers # "id": String, # "url": String, # "secret": String, + # "tag": String, # "state": String, # "load": String, # "load_multiplier": String, @@ -71,6 +73,7 @@ def get_server_info # "url": String, # Required: URL of the BigBlueButton server # "secret": String, # Required: Secret key of the BigBlueButton server # "load_multiplier": Float # Optional: A non-zero number, defaults to 1.0 if not provided or zero + # "tag": String # Optional: A special-purpose tag for the server (empty String to not set it) # } # } def add_server @@ -80,7 +83,8 @@ def add_server tmp_load_multiplier = server_create_params[:load_multiplier].presence&.to_d || 1.0 tmp_load_multiplier = 1.0 if tmp_load_multiplier.zero? - server = Server.create!(url: server_create_params[:url], secret: server_create_params[:secret], load_multiplier: tmp_load_multiplier) + server = Server.create!(url: server_create_params[:url], secret: server_create_params[:secret], + load_multiplier: tmp_load_multiplier, tag: server_create_params[:tag].presence) render json: server_to_json(server), status: :created end end @@ -95,6 +99,7 @@ def add_server # "state": String, # Optional: 'enable', 'cordon', or 'disable' # "load_multiplier": Float # Optional: A non-zero number # "secret": String # Optional: Secret key of the BigBlueButton server + # "tag": String # Optional: A special-purpose tag for the server, empty string to remove the tag # } # } def update_server @@ -166,6 +171,7 @@ def server_to_json(server) id: server.id, url: server.url, secret: server.secret, + tag: server.tag.nil? ? '' : server.tag, state: server.state.presence || (server.enabled ? 'enabled' : 'disabled'), load: server.load.presence || 'unavailable', load_multiplier: server.load_multiplier.nil? ? 1.0 : server.load_multiplier.to_d, @@ -174,11 +180,11 @@ def server_to_json(server) end def server_create_params - params.require(:server).permit(:url, :secret, :load_multiplier) + params.require(:server).permit(:url, :secret, :load_multiplier, :tag) end def server_update_params - params.require(:server).permit(:state, :load_multiplier, :secret) + params.require(:server).permit(:state, :load_multiplier, :secret, :tag) end def server_panic_params diff --git a/app/controllers/bigbluebutton_api_controller.rb b/app/controllers/bigbluebutton_api_controller.rb index 5e2d0f13..d140739f 100644 --- a/app/controllers/bigbluebutton_api_controller.rb +++ b/app/controllers/bigbluebutton_api_controller.rb @@ -160,9 +160,9 @@ def create params.require(:meetingID) begin - server = Server.find_available - rescue ApplicationRedisRecord::RecordNotFound - raise InternalError, 'Could not find any available servers.' + server = Server.find_available(params[:'meta_server-tag']) + rescue ApplicationRedisRecord::RecordNotFound => e + raise InternalError, e.message end # Create meeting in database @@ -188,6 +188,11 @@ def create duration = params[:duration].to_i params[:'meta_tenant-id'] = @tenant.id if @tenant.present? + if server.tag.present? + params[:'meta_server-tag'] = server.tag + else + params.delete(:'meta_server-tag') + end # Set/Overite duration if MAX_MEETING_DURATION is set and it's greater than params[:duration] (if passed) if !Rails.configuration.x.max_meeting_duration.zero? && diff --git a/app/models/server.rb b/app/models/server.rb index 1c1b9bcb..2e80d503 100644 --- a/app/models/server.rb +++ b/app/models/server.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Server < ApplicationRedisRecord - define_attribute_methods :id, :url, :secret, :enabled, :load, :online, :load_multiplier, :healthy_counter, + define_attribute_methods :id, :url, :secret, :tag, :enabled, :load, :online, :load_multiplier, :healthy_counter, :unhealthy_counter, :state, :meetings, :users, :largest_meeting, :videos # Unique ID for this server @@ -13,6 +13,9 @@ class Server < ApplicationRedisRecord # Shared secret for making API requests to this server application_redis_attr :secret + # Special purpose tag for this server + application_redis_attr :tag + # Whether the server is administratively enabled (allowed to create new meetings) application_redis_attr :enabled @@ -91,6 +94,9 @@ def save! result = redis.multi do |pipeline| pipeline.hset(server_key, 'url', url) if url_changed? pipeline.hset(server_key, 'secret', secret) if secret_changed? + if tag_changed? + tag.present? ? pipeline.hset(server_key, 'tag', tag) : pipeline.hdel(server_key, 'tag') + end pipeline.hset(server_key, 'online', online ? 'true' : 'false') if online_changed? pipeline.hset(server_key, 'load_multiplier', load_multiplier) if load_multiplier_changed? pipeline.hset(server_key, 'state', state) if state_changed? @@ -276,15 +282,29 @@ def self.find(id) end # Find the server with the lowest load (for creating a new meeting) - def self.find_available + 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 + end + + # 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("Couldn't find available Server", name, nil) if ids_loads.blank? + 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 RecordNotFound.new("Could not find any available servers with tag=#{tag}.", name, nil) if tag_required + tag = nil # fall back to servers without tag + end + ids_loads = ids_loads.select { |myid, _| redis.hget(key(myid), 'tag') == tag } id, load, hash = ids_loads.each do |id, load| hash = redis.hgetall(key(id)) break id, load, hash if hash.present? end - raise RecordNotFound.new("Couldn't find available Server", name, id) if hash.blank? + raise RecordNotFound.new("Could not find any available servers.", name, id) if hash.blank? hash['id'] = id if hash['state'].present? diff --git a/app/services/server_update_service.rb b/app/services/server_update_service.rb index 3981cd42..bf07be93 100644 --- a/app/services/server_update_service.rb +++ b/app/services/server_update_service.rb @@ -13,6 +13,8 @@ def call update_secret if @params[:secret].present? + update_tag unless @params[:tag].nil? + @server.save! end @@ -43,4 +45,8 @@ def update_load_multiplier def update_secret @server.secret = @params[:secret] end + + def update_tag + @server.tag = @params[:tag].presence + end end diff --git a/docs/api-README.md b/docs/api-README.md index dc0f5b02..fa4705af 100644 --- a/docs/api-README.md +++ b/docs/api-README.md @@ -22,6 +22,7 @@ n/a "id": String, "url": String, "secret": String, + "tag": String, "state": String, "load": String, "load_multiplier": String, @@ -56,6 +57,7 @@ Returns the data associated with a single server "id": String, "url": String, "secret": String, + "tag": String, "state": String, "load": String, "load_multiplier": String, @@ -83,7 +85,8 @@ Adds a new server "server": { "url": String, # Required: URL of the BigBlueButton server "secret": String, # Required: Secret key of the BigBlueButton server - "load_multiplier": Float # Optional: A non-zero number, defaults to 1.0 if not provided or zero + "load_multiplier": Float, # Optional: A non-zero number, defaults to 1.0 if not provided or zero + "tag": String # Optional: A special-purpose tag for the server (empty string to not set it) } } ``` @@ -94,6 +97,7 @@ Adds a new server "id": String, "url": String, "secret": String, + "tag": String, "state": String, "load": String, "load_multiplier": String, @@ -122,7 +126,8 @@ Updates a server "server": { "state": String, # Optional: 'enable', 'cordon', or 'disable' "load_multiplier": Float # Optional: A non-zero number - "secret": String # Optional: Secret key of the BigBlueButton server + "secret": String, # Optional: Secret key of the BigBlueButton server + "tag": String # Optional: A special-purpose tag for the server (empty string to remove the tag) } } ``` @@ -133,6 +138,7 @@ Updates a server "id": String, "url": String, "secret": String, + "tag": String, "state": String, "load": String, "load_multiplier": String, diff --git a/docs/rake-README.md b/docs/rake-README.md index 344c6b9e..d80d4949 100644 --- a/docs/rake-README.md +++ b/docs/rake-README.md @@ -131,7 +131,7 @@ Particular information to note: ### Add a server ```sh -./bin/rake servers:add[url,secret,loadMultiplier] +./bin/rake servers:add[url,secret,loadMultiplier,tag] ``` The `url` value is the complete URL to the BigBlueButton API endpoint of the server. The `/api` on the end is required. @@ -139,6 +139,8 @@ You can find the BigBlueButton server's URL and Secret by running `bbb-conf --se The `loadMultiplier` can be used to give individual servers a higher or lower priority over other servers. A higher loadMultiplier should be placed on the weaker servers. If not passed, it defaults to a value of `1`. +The `tag` can be optionally passed to assign a special-purpose tag to the server. The tag can then be referenced via a meta-parameter in the create call, to place meetings only on servers with the corresponding tag. + This command will print out the ID of the newly created server, and `OK` if it was successful. Note that servers are added in the disabled state; see "Enable a server" below to enable it. @@ -156,13 +158,16 @@ You should either wait for all meetings to end, or run the "Panic" function firs ### Update a server ```sh -./bin/rake servers:update[id,secret,loadMultiplier] +./bin/rake servers:update[id,secret,loadMultiplier,tag] ``` Updates the secret and load_multiplier for a BigBlueButton server. The `loadMultiplier` can be used to give individual servers a higher or lower priority over other servers. A higher loadMultiplier should be placed on the weaker servers. +The `tag` can be optionally passed to assign a special-purpose tag to the server. The tag can then be referenced via a meta-parameter in the create call, to place meetings only on servers with the corresponding tag. +Call the task with empty tag argument as in `servers:update[id,secret,loadMultiplier,]` to untag the server. + After changing the server needs to be polled at least once to see the new load. ### Disable a server @@ -223,6 +228,16 @@ The `loadMultiplier` can be used to give individual servers a higher or lower pr After changing the server needs to be polled at least once to see the new load. +### Tag a server + +```sh +./bin/rake servers:tag[id,tag] +``` + +Adds or removes the tag for a BigBlueButton server. + +The `tag` argument is used to assign a special-purpose tag to the server. The tag can then be referenced via a meta-parameter in the create call, to place meetings only on servers with the corresponding tag. Call the task with empty tag argument as in `servers:tag[id,]` to untag the server. + ### Poll all servers ```sh @@ -300,6 +315,7 @@ servers: url: # default: "https:///bigbluebutton/api" enabled: # default: true load_multiplier: # default: 1.0, must be greater than 0 + tag: # default: '' # Example for a simple server with default values bbb1.example.com: @@ -349,9 +365,9 @@ by `servers:sync`. This will print a table displaying a list of all servers and some basic statistics that can be used for monitoring the overall status of the deployment ``` - HOSTNAME STATE STATUS MEETINGS USERS LARGEST MEETING VIDEOS - bbb1.example.com enabled online 12 25 7 15 - bbb2.example.com enabled online 4 14 4 5 + HOSTNAME STATE STATUS MEETINGS USERS LARGEST MEETING VIDEOS LOAD TAG + bbb1.example.com enabled online 12 25 7 15 25.0 + bbb2.example.com enabled online 4 14 4 5 14.0 test ``` ### Manage Meetings diff --git a/docs/tags-README.md b/docs/tags-README.md new file mode 100644 index 00000000..9b3fe176 --- /dev/null +++ b/docs/tags-README.md @@ -0,0 +1,22 @@ +# Tagged Servers + +## Description + +This feature allows to provide users the choice of specially configured servers (e.g. optimized for very large conferences or running newer BBB versions), without requiring a dedicated Loadbalancer + Frontend infrastructures. It works by first assigning so-called tags to certain servers and then referencing the tag via a meta-parameter in the create API call, to request placing the meeting on a server with corresponding tag. Because the create call is made by the BBB frontend, the feature needs to be supported by the frontend. + +## How to tag servers in Scalelite + +When adding a server in Scalelite, you can optionally specify a non-empty string as "tag" for the server. Per default, it is nil. + +This works for all supported ways of adding servers: Per `rake servers:add` task, per `addServer` API call or per `rake servers:sync` from YAML file. The same is true for the `rake servers:update` task and `updateServer` call. For more details, see `api-README.md` and `rake-README.md`. + +## Create call with server-tag metaparameter + +A create API call with a meta 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. + +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. + diff --git a/lib/server_sync.rb b/lib/server_sync.rb index 1b644fc6..3adf1684 100644 --- a/lib/server_sync.rb +++ b/lib/server_sync.rb @@ -3,7 +3,7 @@ # Tools to synchronize cluster state with a list of servers. Used by the # servers:sync an servers:yaml tasks. class ServerSync - SERVER_PARAMS = %w[url secret enabled load_multiplier].freeze + SERVER_PARAMS = %w[url secret enabled load_multiplier tag].freeze PARAMS_IGNORE = %w[load state online].freeze SYNC_MODES = %w[keep cordon force].freeze @@ -29,8 +29,8 @@ def self.sync_file(path, mode = 'cordon', dryrun = false) # Synchronizes cluster state with a server list. The servers parameter should # be a hash mapping server IDs to parameters. The +secret+ parameter is # required, the API +url+ is auto-derived from the server id if it looks like - # a hostname, +enabled+ is true by defalt and +load_multiplier+ is +1.0+ by - # default. + # a hostname, +enabled+ is true by defalt, +load_multiplier+ is +1.0+ by + # default and +tag+ is +nil+ by default. # The +mode+ decides what happens to undesired servers. Valid values are # +keep+, +cordon+ (default) and +force+. The last option will end all # meetings before the server is removed. @@ -51,6 +51,7 @@ def self.sync(servers, mode = 'cordon', dryrun = false) params['url'] = "https://#{id}/bigbluebutton/api" if params['url'].nil? params['enabled'] = params['enabled'].nil? || !!params['enabled'] params['load_multiplier'] = 1.0 if params['load_multiplier'].nil? + params['tag'] = params['tag'].presence bad_params = params.keys - SERVER_PARAMS - PARAMS_IGNORE raise(SyncError, "Server id=#{id} contains invalid characters") unless /^[a-zA-Z0-9_.-]+$/.match?(id) @@ -77,7 +78,8 @@ def self.sync(servers, mode = 'cordon', dryrun = false) id: id, url: params['url'], secret: params['secret'], - load_multiplier: params['load_multiplier'] + load_multiplier: params['load_multiplier'], + tag: params['tag'] ) end logger.info("[#{id}] Server created") @@ -96,6 +98,10 @@ def self.sync(servers, mode = 'cordon', dryrun = false) server.load_multiplier = params['load_multiplier'] logger.info("[#{id}] Server updated: load_multiplier=#{params['load_multiplier']}") end + unless server.tag == params['tag'] + server.tag = params['tag'] + logger.info("[#{id}] Server updated: tag=#{params['tag']}") + end if params['enabled'] && !server.enabled? server.state = 'enabled' @@ -174,6 +180,7 @@ def self.dump(verbose) "url" => server.url, "secret" => server.secret, "load_multiplier" => server.load_multiplier.to_f || 1.0, + "tag" => server.tag.nil? ? '' : server.tag, "enabled" => server.enabled?, } if verbose diff --git a/lib/tasks/servers.rake b/lib/tasks/servers.rake index e0c0108b..949de4ca 100644 --- a/lib/tasks/servers.rake +++ b/lib/tasks/servers.rake @@ -15,13 +15,14 @@ task servers: :environment do end puts("\tload: #{server.load.presence || 'unavailable'}") puts("\tload multiplier: #{server.load_multiplier.nil? ? 1.0 : server.load_multiplier.to_d}") + puts("\ttag: #{server.tag.nil? ? '' : server.tag}") puts("\t#{server.online ? 'online' : 'offline'}") end end namespace :servers do desc 'Add a new BigBlueButton server (it will be added disabled)' - task :add, [:url, :secret, :load_multiplier] => :environment do |_t, args| + task :add, [:url, :secret, :load_multiplier, :tag] => :environment do |_t, args| if args.url.nil? || args.secret.nil? puts('Error: Please input at least a URL and a secret!') exit(1) @@ -40,13 +41,13 @@ namespace :servers do tmp_load_multiplier = 1.0 end end - server = Server.create!(url: args.url, secret: args.secret, load_multiplier: tmp_load_multiplier) + server = Server.create!(url: args.url, secret: args.secret, load_multiplier: tmp_load_multiplier, tag: args.tag.presence) puts('OK') puts("id: #{server.id}") end desc 'Update a BigBlueButton server' - task :update, [:id, :secret, :load_multiplier] => :environment do |_t, args| + task :update, [:id, :secret, :load_multiplier, :tag] => :environment do |_t, args| server = Server.find(args.id) server.secret = args.secret unless args.secret.nil? tmp_load_multiplier = server.load_multiplier @@ -58,6 +59,7 @@ namespace :servers do end end server.load_multiplier = tmp_load_multiplier + server.tag = args.tag.presence unless args.tag.nil? server.save! puts('OK') rescue ApplicationRedisRecord::RecordNotFound @@ -175,6 +177,17 @@ namespace :servers do exit(1) end + desc 'Set the tag of a BigBlueButton server' + task :tag, [:id, :tag] => :environment do |_t, args| + server = Server.find(args.id) + server.tag = args.tag.presence + server.save! + puts('OK') + rescue ApplicationRedisRecord::RecordNotFound + puts("ERROR: No server found with id: #{args.id}") + exit(1) + end + desc 'Adds multiple BigBlueButton servers defined in a YAML file passed as an argument' task :addAll, [:path] => :environment do |_t, args| servers = YAML.load_file(args.path)['servers'] diff --git a/lib/tasks/status.rake b/lib/tasks/status.rake index 42110ebc..046ebdd1 100644 --- a/lib/tasks/status.rake +++ b/lib/tasks/status.rake @@ -7,7 +7,7 @@ task status: :environment do include ApiHelper servers_info = [] - ServerInfo = Struct.new(:hostname, :state, :status, :meetings, :users, :largest, :videos, :load) + ServerInfo = Struct.new(:hostname, :state, :status, :meetings, :users, :largest, :videos, :load, :tag) Server.all.each do |server| state = server.state @@ -15,7 +15,7 @@ task status: :environment do state = server.state.present? ? status_with_state(state) : status_without_state(enabled) info = ServerInfo.new(URI.parse(server.url).host, state, server.online ? 'online' : 'offline', server.meetings, server.users, -server.largest_meeting, server.videos, server.load) +server.largest_meeting, server.videos, server.load, server.tag) # Convert to openstruct to allow dot syntax usage servers_info.push(info) @@ -33,6 +33,7 @@ server.largest_meeting, server.videos, server.load) t.add_column('LARGEST MEETING', &:largest) t.add_column('VIDEOS', &:videos) t.add_column('LOAD', &:load) + t.add_column('TAG', &:tag) end puts table.pack(max_table_width: nil) diff --git a/spec/models/server_spec.rb b/spec/models/server_spec.rb index f8b963c8..1e3f3d23 100644 --- a/spec/models/server_spec.rb +++ b/spec/models/server_spec.rb @@ -152,6 +152,14 @@ end end + context 'with any tag and no available servers' do + it 'throws an error' do + expect { + described_class.find_available('test-tag') + }.to raise_error(ApplicationRedisRecord::RecordNotFound) + end + end + context 'with missing server hash' do before do # This is mostly a failsafe check @@ -280,6 +288,152 @@ }.to raise_error(ApplicationRedisRecord::RecordNotFound) end end + + context 'with tagged servers' do + before 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') + 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', + 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', + enabled: 'true') + redis.sadd?('servers', 'test-3') + redis.sadd?('server_enabled', 'test-3') + redis.zadd('server_load', 2, 'test-3') + end + end + + context 'and without argument' do + let(:server) { described_class.find_available } + + it 'returns untagged server with lowest load' do + expect(server.id).to eq 'test-3' + expect(server.url).to eq 'https://test-3.example.com/bigbluebutton/api' + expect(server.secret).to eq 'test-3-secret' + expect(server.tag).to be_nil + expect(server.enabled).to be true + expect(server.state).to be_nil + expect(server.load).to eq 2 + end + end + + context 'and with empty tag argument' do + let(:server) { described_class.find_available('') } + + it 'returns untagged server with lowest load' do + expect(server.id).to eq 'test-3' + end + end + + context 'and with ! tag argument' do + let(:server) { described_class.find_available('!') } + + it 'returns untagged server with lowest load' do + expect(server.id).to eq 'test-3' + end + end + end + + context 'with differently tagged servers' do + let(:server) { described_class.find_available('test-tag') } + + before 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-tag', 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 + end + + context 'and optional tag argument' do + let(:server) { described_class.find_available('test-tag') } + + it 'returns matching tagged server with lowest load' do + expect(server.id).to eq 'test-3' + expect(server.url).to eq 'https://test-3.example.com/bigbluebutton/api' + expect(server.secret).to eq 'test-3-secret' + expect(server.tag).to eq 'test-tag' + expect(server.enabled).to be true + expect(server.state).to be_nil + expect(server.load).to eq 2 + end + end + + context 'and required tag argument' do + let(:server) { described_class.find_available('test-tag!') } + + it 'returns matching tagged server with lowest load' do + expect(server.id).to eq 'test-3' + expect(server.tag).to eq 'test-tag' + end + end + end + + context 'with no matching tagged servers' do + before 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', 3, 'test-1') + redis.mapped_hmset('server:test-2', url: 'https://test-2.example.com/bigbluebutton/api', secret: 'test-2-secret', + tag: 'wrong-tag', enabled: 'true') + redis.sadd?('servers', 'test-2') + redis.sadd?('server_enabled', 'test-2') + redis.zadd('server_load', 1, 'test-2') + redis.mapped_hmset('server:test-3', url: 'https://test-3.example.com/bigbluebutton/api', secret: 'test-3-secret', + enabled: 'true') + redis.sadd?('servers', 'test-3') + redis.sadd?('server_enabled', 'test-3') + redis.zadd('server_load', 2, 'test-3') + end + end + + context 'and optional tag argument' do + let(:server) { described_class.find_available('test-tag') } + + it 'returns untagged server with lowest load' do + expect(server.id).to eq 'test-3' + expect(server.url).to eq 'https://test-3.example.com/bigbluebutton/api' + expect(server.secret).to eq 'test-3-secret' + expect(server.tag).to be_nil + expect(server.enabled).to be true + expect(server.state).to be_nil + expect(server.load).to eq 2 + end + end + + it 'raises error with specific message' do + expect { + described_class.find_available('test-tag!') + }.to raise_error(ApplicationRedisRecord::RecordNotFound, "Could not find any available servers with tag=test-tag.") + end + end end describe 'Servers load' do diff --git a/spec/requests/bigbluebutton_api_controller_spec.rb b/spec/requests/bigbluebutton_api_controller_spec.rb index 7f769938..bca9f8c7 100644 --- a/spec/requests/bigbluebutton_api_controller_spec.rb +++ b/spec/requests/bigbluebutton_api_controller_spec.rb @@ -504,6 +504,16 @@ expect(response_xml.at_xpath("/response/message").text).to(eq(expected_error.message)) end + it "responds with specific InternalError if no servers with required tag are available in create" do + get bigbluebutton_api_create_url, params: { meetingID: "test-meeting-1", "meta_server-tag" => "test-tag!" } + + response_xml = Nokogiri.XML(response.body) + expected_error = BBBErrors::InternalError.new("Could not find any available servers with tag=test-tag.") + expect(response_xml.at_xpath("/response/returncode").text).to(eq("FAILED")) + expect(response_xml.at_xpath("/response/messageKey").text).to(eq(expected_error.message_key)) + expect(response_xml.at_xpath("/response/message").text).to(eq(expected_error.message)) + end + it "creates the meeting successfully for a get request" do params = { meetingID: "test-meeting-1", moderatorPW: "mp", voiceBridge: "1234567" } @@ -559,6 +569,58 @@ expect(new_server.load).to eq(7) end + it "with optional tag places the meeting on untagged server if no matching tagged server available" do + server.tag = "wrong-tag" + server.save! + server2 = Server.create(url: "https://test-2.example.com/bigbluebutton/api/", + secret: "test-2-secret", enabled: true, load: 1) + + create_params = { meetingID: "test-meeting-1", moderatorPW: "mp", voiceBridge: "1234567", "meta_server-tag" => "test-tag" } + stub_params = { meetingID: "test-meeting-1", moderatorPW: "mp", voiceBridge: "1234567" } + + stub_create = stub_request(:get, encode_bbb_uri("create", server2.url, server2.secret, stub_params)) + .to_return(body: "SUCCESStest-meeting-1 + apmp") + + get bigbluebutton_api_create_url, params: create_params + + # Reload + meeting = Meeting.find(create_params[:meetingID]) + + response_xml = Nokogiri.XML(response.body) + expect(stub_create).to have_been_requested + expect(response_xml.at_xpath("/response/returncode").text).to eq("SUCCESS") + expect(meeting.id).to eq(create_params[:meetingID]) + expect(meeting.server.id).to eq(server2.id) + expect(meeting.server.load).to eq(2) + expect(meeting.server.tag).to be_nil + end + + it "with (required) tag places the meeting on matching tagged server" do + server2 = Server.create(url: "https://test-2.example.com/bigbluebutton/api/", + secret: "test-2-secret", enabled: true, load: 1, tag: "test-tag") + + create_params = { meetingID: "test-meeting-1", moderatorPW: "mp", voiceBridge: "1234567", "meta_server-tag" => "test-tag!" } + stub_params = { meetingID: "test-meeting-1", moderatorPW: "mp", voiceBridge: "1234567", "meta_server-tag" => "test-tag" } + + stub_create = stub_request(:get, encode_bbb_uri("create", server2.url, server2.secret, stub_params)) + .to_return(body: "SUCCESStest-meeting-1 + apmp") + + get bigbluebutton_api_create_url, params: create_params + + # Reload + meeting = Meeting.find(create_params[:meetingID]) + + response_xml = Nokogiri.XML(response.body) + expect(stub_create).to have_been_requested + expect(response_xml.at_xpath("/response/returncode").text).to eq("SUCCESS") + expect(meeting.id).to eq(create_params[:meetingID]) + expect(meeting.server.id).to eq(server2.id) + expect(meeting.server.load).to eq(2) + expect(meeting.server.tag).to eq("test-tag") + end + it "sets the duration param to MAX_MEETING_DURATION if set" do create_params = { meetingID: "test-meeting-1", moderatorPW: "test-password", voiceBridge: "1234567" } stub_params = { meetingID: "test-meeting-1", moderatorPW: "test-password", voiceBridge: "1234567", duration: 3600 } diff --git a/spec/requests/servers_controller_spec.rb b/spec/requests/servers_controller_spec.rb index 6311d1ae..b3014a8d 100644 --- a/spec/requests/servers_controller_spec.rb +++ b/spec/requests/servers_controller_spec.rb @@ -25,6 +25,7 @@ expect(server).not_to be_nil expect(server_data['url']).to eq(server.url) expect(server_data['secret']).to eq(server.secret) + expect(server_data['tag']).to eq(server.tag.nil? ? '' : server.tag) expect(server_data['state']).to eq(if server.state.present? server.state else @@ -48,7 +49,8 @@ let(:valid_params) { { url: 'https://example.com/bigbluebutton', secret: 'supersecret', - load_multiplier: 1.5 } + load_multiplier: 1.5, + tag: 'test-tag' } } it 'creates a new BigBlueButton server' do @@ -59,6 +61,7 @@ expect(server.url).to eq(valid_params[:url]) expect(server.secret).to eq(valid_params[:secret]) expect(server.load_multiplier).to eq(valid_params[:load_multiplier].to_s) + expect(server.tag).to eq(valid_params[:tag]) end it 'defaults load_multiplier to 1.0 if not provided' do @@ -142,6 +145,31 @@ expect(response.parsed_body['error']).to eq("Load-multiplier must be a non-zero number") end end + + context 'when updating tag' do + it 'updates the server tag' do + server = create(:server) + post scalelite_api_update_server_url, params: { id: server.id, server: { tag: 'test-tag' } } + updated_server = Server.find(server.id) # Reload + expect(updated_server.tag).to eq("test-tag") + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(updated_server.id) + expect(response.parsed_body['tag']).to eq(updated_server.tag) + end + + it 'updates the server tag back to nil' do + server = create(:server) + post scalelite_api_update_server_url, params: { id: server.id, server: { tag: 'test-tag' } } + updated_server = Server.find(server.id) # Reload + expect(updated_server.tag).to eq("test-tag") + post scalelite_api_update_server_url, params: { id: server.id, server: { tag: '' } } + updated_server = Server.find(server.id) # Reload + expect(updated_server.tag).to be_nil + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(updated_server.id) + expect(response.parsed_body['tag']).to eq('') + end + end end describe 'DELETE #deleteServer' do diff --git a/test/controllers/bigbluebutton_api_controller_test.rb b/test/controllers/bigbluebutton_api_controller_test.rb index 82caa601..e6fdc6a0 100644 --- a/test/controllers/bigbluebutton_api_controller_test.rb +++ b/test/controllers/bigbluebutton_api_controller_test.rb @@ -436,6 +436,24 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest assert_equal expected_error.message, response_xml.at_xpath('/response/message').text end + test 'create responds with specific InternalError if no servers with required tag are available in create' do + Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', + secret: 'test-1-secret', enabled: true, load: 0) + + BigBlueButtonApiController.stub_any_instance(:verify_checksum, nil) do + get bigbluebutton_api_create_url, params: { meetingID: 'test-meeting-1', + 'meta_server-tag' => 'test-tag!' } + end + + response_xml = Nokogiri::XML(@response.body) + + expected_error = InternalError.new('Could not find any available servers with tag=test-tag.') + + assert_equal 'FAILED', response_xml.at_xpath('/response/returncode').text + assert_equal expected_error.message_key, response_xml.at_xpath('/response/messageKey').text + assert_equal expected_error.message, response_xml.at_xpath('/response/message').text + end + test 'create creates the room successfully for a get request' do server1 = Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', secret: 'test-1-secret', enabled: true, load: 0) @@ -560,6 +578,80 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest assert_equal 7, server1.load end + test 'create with optional tag places the meeting on untagged server if no matching tagged server available' do + Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', + secret: 'test-1-secret', enabled: true, load: 0, tag: 'wrong-tag') + server2 = Server.create(url: 'https://test-2.example.com/bigbluebutton/api/', + secret: 'test-2-secret', enabled: true, load: 1) + + params = { + meetingID: 'test-meeting-1', moderatorPW: 'mp', 'meta_server-tag' => 'test-tag' + } + + bbb_create = \ + stub_request(:get, "#{server2.url}create") + .with(query: hash_including({})) + .to_return do |request| + request_params = URI.decode_www_form(request.uri.query) + assert_equal params[:meetingID], request_params.assoc('meetingID').last + assert_equal params[:moderatorPW], request_params.assoc('moderatorPW').last + assert_nil request_params.assoc('meta_server-tag') + + { body: meeting_create_response(params[:meetingID], params[:moderatorPW]) } + end + + BigBlueButtonApiController.stub_any_instance(:verify_checksum, nil) do + get bigbluebutton_api_create_url, params: params + end + + assert_requested bbb_create + + # Reload + meeting = Meeting.find(params[:meetingID]) + + response_xml = Nokogiri::XML(@response.body) + assert_equal 'SUCCESS', response_xml.at_xpath('/response/returncode').text + assert_equal params[:meetingID], meeting.id + assert_equal server2.id, meeting.server.id + assert_equal 2, meeting.server.load + assert_nil meeting.server.tag + end + + test 'create with (required) tag places the meeting on matching tagged server' do + Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', + secret: 'test-1-secret', enabled: true, load: 0) + server2 = Server.create(url: 'https://test-2.example.com/bigbluebutton/api/', + secret: 'test-2-secret', enabled: true, load: 1, tag: 'test-tag') + + create_params = { + meetingID: 'test-meeting-1', moderatorPW: 'mp', 'meta_server-tag' => 'test-tag!' + } + stub_params = { + meetingID: 'test-meeting-1', moderatorPW: 'mp', 'meta_server-tag' => 'test-tag' + } + + bbb_create = \ + stub_request(:get, "#{server2.url}create") + .with(query: hash_including(stub_params)) + .to_return(body: meeting_create_response(stub_params[:meetingID], stub_params[:moderatorPW])) + + BigBlueButtonApiController.stub_any_instance(:verify_checksum, nil) do + get bigbluebutton_api_create_url, params: create_params + end + + assert_requested bbb_create + + # Reload + meeting = Meeting.find(create_params[:meetingID]) + + response_xml = Nokogiri::XML(@response.body) + assert_equal 'SUCCESS', response_xml.at_xpath('/response/returncode').text + assert_equal create_params[:meetingID], meeting.id + assert_equal server2.id, meeting.server.id + assert_equal 2, meeting.server.load + assert_equal 'test-tag', meeting.server.tag + end + test 'create sets the duration param to MAX_MEETING_DURATION if set' do server1 = Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', secret: 'test-1-secret', enabled: true, load: 0) diff --git a/test/models/server_test.rb b/test/models/server_test.rb index 0c653e8a..c20be75a 100644 --- a/test/models/server_test.rb +++ b/test/models/server_test.rb @@ -130,6 +130,12 @@ class ServerTest < ActiveSupport::TestCase end end + test 'Server find_available with any tag and no available servers' do + assert_raises(ApplicationRedisRecord::RecordNotFound) do + Server.find_available('test-tag') + end + end + test 'Server find_available with missing server hash' do # This is mostly a failsafe check RedisStore.with_connection do |redis| @@ -237,6 +243,127 @@ class ServerTest < ActiveSupport::TestCase end end + test 'Server find_available without or with empty 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') + 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', + 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', + enabled: 'true') + redis.sadd?('servers', 'test-3') + redis.sadd?('server_enabled', 'test-3') + redis.zadd('server_load', 2, 'test-3') + end + + server = Server.find_available + 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_nil(server.tag) + assert(server.enabled) + assert_nil(server.state) + assert_equal(2, server.load) + + server = Server.find_available('') + assert_equal('test-3', server.id) + + server = Server.find_available('!') + assert_equal('test-3', server.id) + end + + test 'Server find_available with tag argument 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-tag', 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') + 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-tag', server.tag) + assert(server.enabled) + assert_nil(server.state) + assert_equal(2, server.load) + + server = Server.find_available('test-tag!') + assert_equal('test-3', server.id) + assert_equal('test-tag', 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', + enabled: 'true') + redis.sadd?('servers', 'test-1') + redis.sadd?('server_enabled', 'test-1') + redis.zadd('server_load', 3, 'test-1') + redis.mapped_hmset('server:test-2', url: 'https://test-2.example.com/bigbluebutton/api', secret: 'test-2-secret', + tag: 'wrong-tag', enabled: 'true') + redis.sadd?('servers', 'test-2') + redis.sadd?('server_enabled', 'test-2') + redis.zadd('server_load', 1, 'test-2') + redis.mapped_hmset('server:test-3', url: 'https://test-3.example.com/bigbluebutton/api', secret: 'test-3-secret', + enabled: 'true') + redis.sadd?('servers', 'test-3') + redis.sadd?('server_enabled', 'test-3') + redis.zadd('server_load', 2, 'test-3') + end + + server = Server.find_available('test-tag') + 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_nil(server.tag) + assert(server.enabled) + assert_nil(server.state) + assert_equal(2, server.load) + end + + test 'Server find_available with required tag raises error with specific message 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', + enabled: 'true') + redis.sadd?('servers', 'test-1') + redis.sadd?('server_enabled', 'test-1') + redis.zadd('server_load', 2, 'test-1') + redis.mapped_hmset('server:test-2', url: 'https://test-2.example.com/bigbluebutton/api', secret: 'test-2-secret', + tag: 'wrong-tag', enabled: 'true') + redis.sadd?('servers', 'test-2') + redis.sadd?('server_enabled', 'test-2') + redis.zadd('server_load', 1, 'test-2') + end + + assert_raises(ApplicationRedisRecord::RecordNotFound, "Could not find any available servers with tag=test-tag.") do + Server.find_available('test-tag!') + end + end + test 'Servers load are retained after being cordoned' do RedisStore.with_connection do |redis| redis.mapped_hmset('server:test-1', url: 'https://test-1.example.com/bigbluebutton/api', secret: 'test-1-secret', From 6d5202578b8a94181488ab58a3c1e2c765123dc8 Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Mon, 22 Apr 2024 22:07:51 +0200 Subject: [PATCH 6/7] Improved handling of create API call (#1050) * in create call, now check whether meeting is already running before finding available servers * even more streamlining of the create call handling * now preliminarily increment server load in join call (too) * add test for load increment on join call * add RSpec version of test for load increment on join --- .../bigbluebutton_api_controller.rb | 62 +++++++++++-------- .../bigbluebutton_api_controller_spec.rb | 19 ++++++ .../bigbluebutton_api_controller_test.rb | 20 ++++++ 3 files changed, 74 insertions(+), 27 deletions(-) diff --git a/app/controllers/bigbluebutton_api_controller.rb b/app/controllers/bigbluebutton_api_controller.rb index d140739f..46837c6a 100644 --- a/app/controllers/bigbluebutton_api_controller.rb +++ b/app/controllers/bigbluebutton_api_controller.rb @@ -160,33 +160,38 @@ def create params.require(:meetingID) begin - server = Server.find_available(params[:'meta_server-tag']) - rescue ApplicationRedisRecord::RecordNotFound => e - raise InternalError, e.message + # Check if meeting is already running + meeting = Meeting.find(params[:meetingID], @tenant&.id) + server = meeting.server + logger.debug("Found existing meeting #{params[:meetingID]} on BigBlueButton server #{server.id}.") + rescue ApplicationRedisRecord::RecordNotFound + begin + # Find available server and create meeting on it + server = Server.find_available(params[:'meta_server-tag']) + + # Create meeting in database + logger.debug("Creating meeting #{params[:meetingID]} in database.") + moderator_pwd = params[:moderatorPW].presence || SecureRandom.alphanumeric(8) + meeting = Meeting.find_or_create_with_server!( + params[:meetingID], + server, + moderator_pwd, + params[:voiceBridge], + @tenant&.id + ) + + # Update server if meeting (unexpectedly) already existed on a different server + server = meeting.server + + logger.debug("Incrementing server #{server.id} load by 1") + server.increment_load(1) + rescue ApplicationRedisRecord::RecordNotFound => e + raise InternalError, e.message + end end - # Create meeting in database - logger.debug("Creating meeting #{params[:meetingID]} in database.") - - moderator_pwd = params[:moderatorPW].presence || SecureRandom.alphanumeric(8) - params[:moderatorPW] = moderator_pwd - - meeting = Meeting.find_or_create_with_server!( - params[:meetingID], - server, - moderator_pwd, - params[:voiceBridge], - @tenant&.id - ) - - # Update with old server if meeting already existed in database - server = meeting.server - - logger.debug("Incrementing server #{server.id} load by 1") - server.increment_load(1) - - duration = params[:duration].to_i - + params[:moderatorPW] = meeting.moderator_pw + params[:voiceBridge] = meeting.voice_bridge params[:'meta_tenant-id'] = @tenant.id if @tenant.present? if server.tag.present? params[:'meta_server-tag'] = server.tag @@ -194,6 +199,8 @@ def create params.delete(:'meta_server-tag') end + duration = params[:duration].to_i + # Set/Overite duration if MAX_MEETING_DURATION is set and it's greater than params[:duration] (if passed) if !Rails.configuration.x.max_meeting_duration.zero? && (duration.zero? || duration > Rails.configuration.x.max_meeting_duration) @@ -201,8 +208,6 @@ def create params[:duration] = Rails.configuration.x.max_meeting_duration end - params[:voiceBridge] = meeting.voice_bridge - if @tenant&.lrs_endpoint.present? lrs_payload = LrsPayloadService.new(tenant: @tenant, secret: server.secret).call params[:'meta_secret-lrs-payload'] = lrs_payload if lrs_payload.present? @@ -294,6 +299,9 @@ def join logger.info("The requested meeting #{params[:meetingID]} does not exist") raise MeetingNotFoundError end + logger.debug("Incrementing server #{server.id} load by 1") + server.increment_load(1) + # Get list of params that should not be modified by join API call excluded_params = Rails.configuration.x.join_exclude_params diff --git a/spec/requests/bigbluebutton_api_controller_spec.rb b/spec/requests/bigbluebutton_api_controller_spec.rb index bca9f8c7..bc10214e 100644 --- a/spec/requests/bigbluebutton_api_controller_spec.rb +++ b/spec/requests/bigbluebutton_api_controller_spec.rb @@ -1224,6 +1224,25 @@ expect(response).to redirect_to(encode_bbb_uri("join", server.url, server.secret, params).to_s) end + it "increments the server load by the value of load_multiplier" do + server.load_multiplier = 7.0 + server.save! + meeting = create(:meeting, server: server) + + # Reload 1 + new_server = Server.find(server.id) + load_before_join = new_server.load + + # Join + params = { meetingID: meeting.id, password: "test-password", fullName: "test-name" } + get bigbluebutton_api_join_url, params: params + + # Reload 2 + new_server = Server.find(server.id) + expected_load = load_before_join + 7.0 + expect(new_server.load).to eq(expected_load) + end + it "redirects user to the current join url with only permitted params for join" do meeting = create(:meeting, server: server) params = { meetingID: meeting.id, password: "test-password", fullName: "test-name", test1: "", test2: "" } diff --git a/test/controllers/bigbluebutton_api_controller_test.rb b/test/controllers/bigbluebutton_api_controller_test.rb index e6fdc6a0..599b0311 100644 --- a/test/controllers/bigbluebutton_api_controller_test.rb +++ b/test/controllers/bigbluebutton_api_controller_test.rb @@ -1191,6 +1191,26 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest assert_redirected_to encode_bbb_uri('join', server1.url, server1.secret, params).to_s end + test 'join increments the server load by the value of load_multiplier' do + server1 = Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', + secret: 'test-1-secret', enabled: true, online: true, load: 0, load_multiplier: 7.0) + meeting = Meeting.find_or_create_with_server('test-meeting-1', server1, 'mp') + + # Reload 1 + server1 = Server.find(server1.id) + load_before_join = server1.load + + params = { meetingID: meeting.id, moderatorPW: 'mp', fullName: 'test-name' } + BigBlueButtonApiController.stub_any_instance(:verify_checksum, nil) do + get bigbluebutton_api_join_url, params: params + end + + # Reload 2 + server1 = Server.find(server1.id) + expected_load = load_before_join + 7.0 + assert_equal expected_load, server1.load + end + test 'join redirects user to the current join url with only permitted params for join' do server1 = Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', secret: 'test-1-secret', enabled: true, load: 0, online: true) From dd6fb56ca5a8c1e03715c49f75e9995cf1419cad Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Mon, 22 Apr 2024 22:09:23 +0200 Subject: [PATCH 7/7] Poll & list BBB server version (if revealed) (#1058) * initial implementation of version poll task * add server bbb_version to Redis model and adapt poll/servers/status rake tasks * add bbb_version to verbose output of rake server:yaml * add bbb_version to output of servers / getServerInfo API * mention BBB configuration to reveal version in README --- README.md | 5 ++++ app/controllers/api/servers_controller.rb | 3 +++ app/models/server.rb | 6 ++++- lib/server_sync.rb | 3 ++- lib/tasks/poll.rake | 32 ++++++++++++++++++++++- lib/tasks/servers.rake | 1 + lib/tasks/status.rake | 5 ++-- 7 files changed, 50 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 559dfb1e..b156b7ca 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,11 @@ To help users who are behind restrictive firewalls to send/receive media (audio, Again, [bbb-install.sh](https://github.com/bigbluebutton/bbb-install#install-a-turn-server) can automate this process for you. +If you want Scalelite to infer and display the BBB version of your servers, you need to add the following line to `/etc/bigbluebutton/bbb-web.properties` on your BBB servers: +``` +allowRevealOfBBBVersion=true +``` + ### Setup a shared volume for recordings See [Setting up a shared volume for recordings](docs/sharedvolume-README.md) diff --git a/app/controllers/api/servers_controller.rb b/app/controllers/api/servers_controller.rb index 3e2d29da..45bac1b3 100644 --- a/app/controllers/api/servers_controller.rb +++ b/app/controllers/api/servers_controller.rb @@ -19,6 +19,7 @@ class ServersController < ApplicationController # "url": String, # "secret": String, # "tag": String, + # "bbb_version": String, # "state": String, # "load": String, # "load_multiplier": String, @@ -51,6 +52,7 @@ def get_servers # "url": String, # "secret": String, # "tag": String, + # "bbb_version": String, # "state": String, # "load": String, # "load_multiplier": String, @@ -172,6 +174,7 @@ def server_to_json(server) url: server.url, secret: server.secret, tag: server.tag.nil? ? '' : server.tag, + bbb_version: server.bbb_version.nil? ? '' : server.bbb_version, state: server.state.presence || (server.enabled ? 'enabled' : 'disabled'), load: server.load.presence || 'unavailable', load_multiplier: server.load_multiplier.nil? ? 1.0 : server.load_multiplier.to_d, diff --git a/app/models/server.rb b/app/models/server.rb index 2e80d503..75ba0ab2 100644 --- a/app/models/server.rb +++ b/app/models/server.rb @@ -2,7 +2,7 @@ class Server < ApplicationRedisRecord define_attribute_methods :id, :url, :secret, :tag, :enabled, :load, :online, :load_multiplier, :healthy_counter, - :unhealthy_counter, :state, :meetings, :users, :largest_meeting, :videos + :unhealthy_counter, :state, :meetings, :users, :largest_meeting, :videos, :bbb_version # Unique ID for this server application_redis_attr :id @@ -49,6 +49,9 @@ class Server < ApplicationRedisRecord # Indicator of total video streams in this server application_redis_attr :videos + # Indicator of the BBB version of this server + application_redis_attr :bbb_version + def online=(value) value = !!value online_will_change! unless @online == value @@ -104,6 +107,7 @@ def save! pipeline.hset(server_key, 'users', users) if users_changed? pipeline.hset(server_key, 'largest_meeting', largest_meeting) if largest_meeting_changed? pipeline.hset(server_key, 'videos', videos) if videos_changed? + pipeline.hset(server_key, 'bbb_version', bbb_version) if bbb_version_changed? pipeline.sadd?('servers', id) if id_changed? state.present? ? save_with_state(pipeline) : save_without_state(pipeline) end diff --git a/lib/server_sync.rb b/lib/server_sync.rb index 3adf1684..3648603b 100644 --- a/lib/server_sync.rb +++ b/lib/server_sync.rb @@ -4,7 +4,7 @@ # servers:sync an servers:yaml tasks. class ServerSync SERVER_PARAMS = %w[url secret enabled load_multiplier tag].freeze - PARAMS_IGNORE = %w[load state online].freeze + PARAMS_IGNORE = %w[load state bbb_version online].freeze SYNC_MODES = %w[keep cordon force].freeze class SyncError < StandardError @@ -186,6 +186,7 @@ def self.dump(verbose) if verbose info["state"] = server.state.presence || (server.enabled? ? 'enabled' : 'disabled') info["load"] = server.load.presence || -1.0 + info["bbb_version"] = server.bbb_version.nil? ? '' : server.bbb_version info["online"] = server.online end [server.id, info] diff --git a/lib/tasks/poll.rake b/lib/tasks/poll.rake index 1a0ad16a..41e4d74a 100644 --- a/lib/tasks/poll.rake +++ b/lib/tasks/poll.rake @@ -174,6 +174,36 @@ namespace :poll do pool.wait_for_termination(5) || pool.kill end + desc 'Check the BBB version of all servers' + task versions: :environment do + include ApiHelper + + Rails.logger.debug('Checking versions') + pool = Concurrent::FixedThreadPool.new(Rails.configuration.x.poller_threads.to_i - 1, name: 'sync-version-data') + tasks = Server.all.map do |server| + Concurrent::Promises.future_on(pool) do + Rails.logger.debug { "Checking Version of id=#{server.id}" } + bbb_version = get_post_req(encode_bbb_uri('', server.url, server.secret)).xpath('/response/bbbVersion').text + server.bbb_version = bbb_version + rescue StandardError => e + Rails.logger.warn("Unexpected error when checking version of server id=#{server.id}: #{e}") + ensure + begin + server.save! + rescue ApplicationRedisRecord::RecordNotSaved => e + Rails.logger.warn("Unable to update Server id=#{server.id}: #{e}") + end + end + end + begin + Concurrent::Promises.zip_futures_on(pool, *tasks).wait!(Rails.configuration.x.poller_wait_timeout) + rescue StandardError => e + Rails.logger.warn("Error #{e}") + end + pool.shutdown + pool.wait_for_termination(5) || pool.kill + end + desc 'Run all pollers once' - multitask all: [:servers, :meetings] + multitask all: [:servers, :meetings, :versions] end diff --git a/lib/tasks/servers.rake b/lib/tasks/servers.rake index 949de4ca..f5f34098 100644 --- a/lib/tasks/servers.rake +++ b/lib/tasks/servers.rake @@ -15,6 +15,7 @@ task servers: :environment do end puts("\tload: #{server.load.presence || 'unavailable'}") puts("\tload multiplier: #{server.load_multiplier.nil? ? 1.0 : server.load_multiplier.to_d}") + puts("\tbbb version: #{server.bbb_version.nil? ? '' : server.bbb_version}") puts("\ttag: #{server.tag.nil? ? '' : server.tag}") puts("\t#{server.online ? 'online' : 'offline'}") end diff --git a/lib/tasks/status.rake b/lib/tasks/status.rake index 046ebdd1..98328dea 100644 --- a/lib/tasks/status.rake +++ b/lib/tasks/status.rake @@ -7,7 +7,7 @@ task status: :environment do include ApiHelper servers_info = [] - ServerInfo = Struct.new(:hostname, :state, :status, :meetings, :users, :largest, :videos, :load, :tag) + ServerInfo = Struct.new(:hostname, :state, :status, :meetings, :users, :largest, :videos, :load, :bbb_version, :tag) Server.all.each do |server| state = server.state @@ -15,7 +15,7 @@ task status: :environment do state = server.state.present? ? status_with_state(state) : status_without_state(enabled) info = ServerInfo.new(URI.parse(server.url).host, state, server.online ? 'online' : 'offline', server.meetings, server.users, -server.largest_meeting, server.videos, server.load, server.tag) +server.largest_meeting, server.videos, server.load, server.bbb_version, server.tag) # Convert to openstruct to allow dot syntax usage servers_info.push(info) @@ -33,6 +33,7 @@ server.largest_meeting, server.videos, server.load, server.tag) t.add_column('LARGEST MEETING', &:largest) t.add_column('VIDEOS', &:videos) t.add_column('LOAD', &:load) + t.add_column('BBB VERSION', &:bbb_version) t.add_column('TAG', &:tag) end