Skip to content

Commit

Permalink
Ensure default values exist for worker limits/requests
Browse files Browse the repository at this point in the history
Skip validation of limits/request unless one is specified: mostly for testing.
If you specify one of them for a worker, all four need to be specified or
inherited from ancestor worker classes.
  • Loading branch information
jrafanie committed Apr 27, 2021
1 parent 8607799 commit 4775c0b
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 13 deletions.
26 changes: 20 additions & 6 deletions lib/vmdb/settings/validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,27 @@ def workers(data)
def validate_worker_request_limit(worker_class, data)
valid = true
errors = []
worker_settings = worker_class.worker_settings(data)
worker_settings = worker_class.fetch_worker_settings_from_options_hash(data[:config])

# Only validate request/limits if you specify any of them. Specifying at least one of these four, requires
# all of them to be enumerated. This gets around tests that want to stub some of the worker_settings.
worker_settings_keys_with_dependencies = %i[cpu_request_percent cpu_threshold_percent memory_request memory_threshold]
return valid, errors if (worker_settings.keys & worker_settings_keys_with_dependencies).empty?

worker_settings_keys_with_dependencies.each do |key|
if !worker_settings.key?(key)
errors << [key, "#{worker_class.settings_name} #{key} is missing!"]
return false, errors
elsif !worker_settings[key].kind_of?(Numeric)
errors << [key, "#{worker_class.settings_name} #{key} has non-numeric value: #{worker_settings[key].inspect}"]
return false, errors
end
end

# add assertions that these keys exist - remove defaults
cpu_request = worker_settings[:cpu_request_percent] || 0
cpu_limit = worker_settings[:cpu_threshold_percent] || Float::INFINITY
memory_request = worker_settings[:memory_request] || 0.bytes
memory_limit = worker_settings[:memory_threshold] || Float::INFINITY.megabytes
cpu_request = worker_settings[:cpu_request_percent]
cpu_limit = worker_settings[:cpu_threshold_percent]
memory_request = worker_settings[:memory_request]
memory_limit = worker_settings[:memory_threshold]

if cpu_request > cpu_limit
valid = false
Expand Down
30 changes: 23 additions & 7 deletions spec/lib/vmdb/settings/validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,29 @@
expect(errors.first.last).to include("cannot exceed cpu_threshold_percent")
end

[:memory_request, 500.megabytes, :memory_threshold, 500.megabytes, :cpu_request, 50, :cpu_threshold_percent, 50].each_slice(2) do |key, value|
it "is valid with nil values replaced with default values, when specifying only: #{key}" do
stub_settings_merge(:workers => {:worker_base => {:defaults => {:cpu_request_percent => nil, :cpu_threshold_percent => nil}, :schedule_worker => {key => value}}})
result, errors = subject.validate
expect(result).to eql(true)
expect(errors.empty?).to eql(true)
end
it "is invalid if one of the request/limit values is nil" do
stub_settings_merge(:workers => {:worker_base => {:defaults => {:cpu_request_percent => nil}}})
result, errors = subject.validate
expect(result).to eql(false)
expect(errors.first.first).to eql("workers-cpu_request_percent")
expect(errors.first.last).to include("has non-numeric value")
end

it "is invalid if one of the request/limit values is provided but one is missing" do
hash = Settings.to_hash
hash[:workers][:worker_base][:defaults].delete(:cpu_threshold_percent)
stub_settings(hash)
result, errors = subject.validate
expect(result).to eql(false)
expect(errors.first.first).to eql("workers-cpu_threshold_percent")
expect(errors.first.last).to include("is missing")
end

it "is valid if none of the request/limit values are provided" do
stub_settings({})
result, errors = subject.validate
expect(result).to eql(true)
expect(errors.empty?).to eql(true)
end
end
end

0 comments on commit 4775c0b

Please sign in to comment.