Project

General

Profile

Actions

Bug #10020

closed

Smart Proxy DHCP does not set option 66 and 67 (nextServer, filename) on new DHCP Record creation

Added by Benjamin Papillon almost 9 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
DHCP
Target version:
Difficulty:
Triaged:
Fixed in Releases:
Found in Releases:

Description

Here is the setup :
On CentOS 6 host named foreman, installed with foreman-installer
- Foreman 1.8-RC2 installed
- Smart proxy providing TFTP,DNS,Puppet,PuppetCA.

On Windowns 2008-R2 (AD server)
- Smart proxy 1.8-RC2 from git
- Ruby 1.9.3 has been installed as dependency

Windows Smart Proxy was registered successfully into foreman infrastructure, and DHCP functionality worked well for importing networks.

Networks are configured to use Smart Proxy on foreman host for DNS and TFTP
Networks are configured to use Smart Proxy on Windows host for DHCP

Issue :
- When provisioning a new host, DHCP records are not created with option 66 nor option 67. Option 60 is set as it should.

Setting up the 2 missing DHCP options manually results in a working provisioned installation.

From the smart proxy code source (1.8-stable branch), you can see line 44 of modules/dhcp/providers/server/native_ms.rb, a for/loop that is iterating all options passed to addRecord function.
When I add some logger.debug here, I can see that only option 60 is present in the structure.

Any help welcome :)

Regards,

Benjamin


Related issues 1 (0 open1 closed)

Related to Foreman - Bug #12245: Nic::Bootable should be removedClosedDaniel Lobato Garcia10/21/2015Actions
Actions #1

Updated by Benjamin Papillon almost 9 years ago

Digging deeper, I found that options are not transmitted from Foreman to Smart proxy. (I ran tcpdump on the clear stream between foreman and smart proxy)

Here is the POST that foreman is doing to Smart Proxy, you can clearly see that nextServer and filename are missing :

POST /dhcp/10.10.45.0 HTTP/1.1
Accept: application/json
Accept-Encoding: gzip, deflate
Content-Length: 106
Content-Type: application/x-www-form-urlencoded
User-Agent: Ruby
Host: v-ad-01.virtu-bench.local:8080

hostname=test8centos6.virtu-bench.local&mac=00%3A50%3A56%3A9f%3A3b%3A76&ip=10.10.45.133&network=10.10.45.0
Actions #2

Updated by Lukas Zapletal almost 9 years ago

  • Project changed from Foreman to Smart Proxy
  • Category changed from DHCP to DHCP

Related changes which might be interesting to investigate: #6743 #7660 #8788

Actions #3

Updated by Benjamin Papillon almost 9 years ago

  • Category deleted (DHCP)

I did another test, creating a new CentOS6 host dedicated to DHCP function.

After running some foreman-installer commands to deploy only the DHCP Smart Proxy, I registered it onto my running Foreman instance (all 1.8 versions)
Here are some observations :
- The current dhcpd.conf template "hardcode" the pxeserver option to the current host the smart proxy is installed in :
In /usr/share/foreman-installer/modules/foreman_proxy/manifests/proxydhcp.pp:

    pxeserver   => $ip,

In the template /usr/share/foreman-installer/modules/dhcp/templates/dhcpd.conf.erb that use pxeserver :
<% if has_variable?( 'pxeserver' ) &&
  has_variable?( 'pxefilename' ) &&
  @pxeserver &&
  @pxefilename -%>
# PXE Handoff.
next-server <%= @pxeserver %>;
filename "<%= @pxefilename %>";
<% end -%>

This should not be necessary as the smart proxy should create a reserved IP record with those values

- The nextServer and filename options are not passed to Smart Proxy
Here is the POST command from foreman to Smart Proxy :

POST /dhcp/10.10.46.0 HTTP/1.1
Accept: application/json
Accept-Encoding: gzip, deflate
Content-Length: 106
Content-Type: application/x-www-form-urlencoded
User-Agent: Ruby
Host: 10.10.46.120:8080

hostname=test9centos6.virtu-bench.local&mac=00%3A50%3A56%3A9f%3A58%3Aaa&ip=10.10.46.121&network=10.10.46.0

To conclude, I'd say nobody ever tried to separate TFTP from DHCP with the foreman-installer

Actions #4

Updated by Lukas Zapletal almost 9 years ago

  • translation missing: en.field_release set to 50

Setting the release to ensure triage. Thanks.

Actions #5

Updated by Dominic Cleal almost 9 years ago

  • Project changed from Smart Proxy to Foreman
  • Category set to DHCP
  • translation missing: en.field_release changed from 50 to 28

Seems to be a 1.8 regression as part of the NIC reworking in #7456.

The primary boot NIC is being created as an instance of Nic::Managed through the UI, but the nextServer/filename attributes are only implemented in Nic::Bootable (a derivative of Nic::Managed), so the NIC is getting its DHCP record created with only basic attributes.

Incidentally, the dhcp_attrs method is also in Orchestration::DHCP which confused me.. the active dhcp_attrs methods seems to be in Nic::* classes.

Actions #6

Updated by Dominic Cleal almost 9 years ago

Dominic Cleal wrote:

Seems to be a 1.8 regression as part of the NIC reworking in #7456.
[...]
Incidentally, the dhcp_attrs method is also in Orchestration::DHCP which confused me.. the active dhcp_attrs methods seems to be in Nic::* classes.

I think this was an accident of history. In 1.7, the full dhcp_attrs implementation (with filename/nextServer) in Orchestration::DHCP would have been used for the host's primary interface so it all worked properly. dhcp_attrs has been in the Nic::* classes forever and was only used for additional interfaces in 1.7.

Since the primary interface is now a NIC in 1.8, it no longer uses the full dhcp_attrs impl and instead uses a Nic::* one, but since we create a managed NIC rather than bootable NIC, it doesn't work.

Perhaps dhcp_attrs in Nic::Managed should be aware of the provision? flag and add filename/nextServer, or we should be creating Nic::Bootable (which doesn't appear to be the direction the new code is taking).

Actions #7

Updated by Dominic Cleal almost 9 years ago

  • Status changed from New to Assigned
  • Assignee set to Dominic Cleal
Actions #8

Updated by The Foreman Bot almost 9 years ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/2293 added
  • Pull request deleted ()
Actions #9

Updated by Dominic Cleal almost 9 years ago

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

Updated by Dominic Cleal over 8 years ago

  • Related to Bug #12245: Nic::Bootable should be removed added
Actions

Also available in: Atom PDF