Bug #7766

ms_native dhcp smart proxy code scales poorly

Added by Dan Berger over 2 years ago. Updated 3 months ago.

Status:Closed
Priority:Normal
Assigned To:Dmitri Dolguikh
Category:DHCP
Target version:Foreman - Team Ivan Iteration 5
Difficulty:hard Bugzilla link:
Found in release:1.5.2 Pull request:https://github.com/theforeman/smart-proxy/pull/445, https://github.com/theforeman/smart-proxy/pull/426
Story points-
Velocity based estimate-
Release1.14.0Release relationshipAuto

Description

the ms native dhcp server code (native_ms.rb) runs netsh commands per reservation - so in a scope with hundreds of reservations, selecting an unused address can take on the order of a minute.

I've reworked loadSubnetData and loadSubetOptions to parse the output of a dhcp server config dump, which takes runtime from ~1 minute to ~2 seconds - the patch isn't suitable for upstreaming, but I'd be happy to share more detail with someone interested in doing it "for real."

I've looked at the 1.6 and development versions of the file and they seem substantively similar in this regard.


Related issues

Related to Smart Proxy - Bug #6916: MS DHCP Timeout Resolved 08/04/2014
Related to Smart Proxy - Feature #4171: Smart proxy DHCP plugin does not honor exclude ranges Duplicate 01/23/2014
Related to Smart Proxy - Bug #1280: MS DHCP ignores address exclusions Resolved 10/31/2011
Related to Smart Proxy - Bug #16056: Non-english locale casuses "Vendor Class not found" error... Resolved 08/11/2016
Related to Smart Proxy - Bug #15789: In MS DHCP provider general/Vendor options must exist bef... Resolved 07/22/2016
Related to Smart Proxy - Feature #15554: Add lease deletion facility to dhcp providers New 06/30/2016
Related to Smart Proxy - Bug #13420: [Smart proxy] provider dhcp_native_ms DELETE method broken Resolved 01/27/2016

Associated revisions

Revision 14d8e523
Added by Dmitri Dolguikh 3 months ago

Fixes #7766: ms dhcp provider uses native dhcps api now.

History

#1 Updated by Oliver Weinmann over 2 years ago

Hi Dan,

I have very similar issues where the smart proxy takes too long to register a new reservation and a timeout is reached and a host creation fails. I guess your fix could also fix this very very annoying issue.

#2 Updated by Dominic Cleal over 2 years ago

  • Related to Bug #6916: MS DHCP Timeout added

#3 Updated by The Foreman Bot almost 2 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/smart-proxy/pull/277 added

#4 Updated by Dmitri Dolguikh over 1 year ago

  • Pull request https://github.com/theforeman/smart-proxy/pull/294 added

#5 Updated by Oliver Weinmann over 1 year ago

Hi,

I'm happy to test, but the code contains static ip ranges?

#6 Updated by Oliver Weinmann over 1 year ago

Ok, i tested the 3 patched files with 1.82 and the dev branch. foreman is not able to find a free ip:

ERF12-4395 [ProxyAPI::ProxyException]: Unable to retrieve DHCP entry for 172.28.19.103 ([RestClient::BadRequest]: 400 Bad Request) for proxy https://gedasvw16.a.space.corp:8443/dhcp

from the proxy.log:

E, [2015-06-23T17:26:48.213031 #1748] ERROR -- : undefined method `each' for #<String:0x3a11c90>
D, [2015-06-23T17:26:48.213031 #1748] DEBUG -- : c:/foreman-proxy/modules/dhcp/providers/server/native_ms.rb:318:in `parse_options'
c:/foreman-proxy/modules/dhcp/providers/server/native_ms.rb:124:in `block in subnet_entries_to_records'
c:/foreman-proxy/modules/dhcp/providers/server/native_ms.rb:122:in `each'
c:/foreman-proxy/modules/dhcp/providers/server/native_ms.rb:122:in `subnet_entries_to_records'
c:/foreman-proxy/modules/dhcp/providers/server/native_ms.rb:150:in `loadSubnetData'
c:/foreman-proxy/modules/dhcp/subnet.rb:65:in `load'
c:/foreman-proxy/modules/dhcp/subnet.rb:75:in `records'
c:/foreman-proxy/modules/dhcp/server.rb:79:in `block in find_record'
c:/foreman-proxy/modules/dhcp/server.rb:78:in `each'
c:/foreman-proxy/modules/dhcp/server.rb:78:in `find_record'
c:/foreman-proxy/modules/dhcp/dhcp_api.rb:84:in `block in <class:DhcpApi>'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1610:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1610:in `block in compile!'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:974:in `[]'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:974:in `block (3 levels) in route!'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:993:in `route_eval'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:974:in `block (2 levels) in route!'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1014:in `block in process_route'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1012:in `catch'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1012:in `process_route'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:972:in `block in route!'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:971:in `each'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:971:in `route!'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1084:in `block in dispatch!'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1066:in `block in invoke'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1066:in `catch'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1066:in `invoke'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1081:in `dispatch!'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:906:in `block in call!'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1066:in `block in invoke'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1066:in `catch'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1066:in `invoke'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:906:in `call!'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:894:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/methodoverride.rb:21:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/commonlogger.rb:33:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:218:in `call'
c:/foreman-proxy/lib/proxy/log.rb:35:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/xss_header.rb:18:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/path_traversal.rb:16:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/json_csrf.rb:18:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/base.rb:49:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/base.rb:49:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/frame_options.rb:31:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/nulllogger.rb:9:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/head.rb:11:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/show_exceptions.rb:21:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:181:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:2021:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1486:in `block in call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1795:in `synchronize'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/sinatra-1.4.6/lib/sinatra/base.rb:1486:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/builder.rb:138:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/urlmap.rb:65:in `block in call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/urlmap.rb:50:in `each'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/urlmap.rb:50:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/builder.rb:138:in `call'
C:/Ruby193/lib/ruby/gems/1.9.1/gems/rack-1.5.5/lib/rack/handler/webrick.rb:60:in `service'
C:/Ruby193/lib/ruby/1.9.1/webrick/httpserver.rb:138:in `service'
C:/Ruby193/lib/ruby/1.9.1/webrick/httpserver.rb:94:in `run'
C:/Ruby193/lib/ruby/1.9.1/webrick/server.rb:191:in `block in start_thread'

#7 Updated by Oliver Weinmann over 1 year ago

Ok, got it.

I'm using ruby 1.9 and .each is no longer supported on a string.

in native_ms.rb

response.each do |line|

should be

response.each_line do |line|

BTW this code works a lot better. Please improve it and release it in 1.9! :)

#8 Updated by Dominic Cleal 9 months ago

  • Status changed from Ready For Testing to New
  • Pull request deleted (https://github.com/theforeman/smart-proxy/pull/294)

PR closed.

#9 Updated by The Foreman Bot 9 months ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/smart-proxy/pull/426 added

#10 Updated by The Foreman Bot 7 months ago

  • Assigned To set to Dmitri Dolguikh
  • Pull request https://github.com/theforeman/smart-proxy/pull/445 added

#11 Updated by Ivan Necas 6 months ago

  • Target version set to Team Ivan Iteration 1

#12 Updated by Ivan Necas 6 months ago

  • Target version changed from Team Ivan Iteration 1 to Team Ivan Iteration 2

#13 Updated by Dmitri Dolguikh 6 months ago

  • Related to Feature #4171: Smart proxy DHCP plugin does not honor exclude ranges added

#14 Updated by Dominic Cleal 6 months ago

  • Related to Bug #1280: MS DHCP ignores address exclusions added

#15 Updated by Dmitri Dolguikh 6 months ago

  • Related to Bug #16056: Non-english locale casuses "Vendor Class not found" errors when using windows dhcp provider added

#16 Updated by Dmitri Dolguikh 6 months ago

  • Related to Bug #15789: In MS DHCP provider general/Vendor options must exist before assigning values to them added

#17 Updated by Dmitri Dolguikh 6 months ago

  • Related to Feature #15554: Add lease deletion facility to dhcp providers added

#18 Updated by Dmitri Dolguikh 6 months ago

  • Related to Bug #13420: [Smart proxy] provider dhcp_native_ms DELETE method broken added

#19 Updated by Ivan Necas 5 months ago

  • Target version changed from Team Ivan Iteration 2 to Team Ivan Iteration 3

#20 Updated by Ivan Necas 5 months ago

  • Target version changed from Team Ivan Iteration 3 to Team Ivan Iteration 4

#21 Updated by Dmitri Dolguikh 4 months ago

  • Project changed from Foreman to Smart Proxy
  • Category changed from Performance to DHCP
  • Difficulty changed from easy to hard

#22 Updated by Ivan Necas 4 months ago

  • Target version changed from Team Ivan Iteration 4 to Team Ivan Iteration 5

#23 Updated by Anonymous 3 months ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

#24 Updated by Dominic Cleal 3 months ago

  • Release set to 1.14.0

Also available in: Atom PDF