-
Notifications
You must be signed in to change notification settings - Fork 30
Fixes BZ #1176423: create reservation for VIP ip addresses in DHCP (Do not merge) #402
base: master
Are you sure you want to change the base?
Conversation
https://bugzilla.redhat.com/show_bug.cgi?id=1176423 For VIPs on the provisioning network (with DHCP active), calling unused_ip is insufficient. We also need to create the actual DHCP reservation so the allocated IP address won't be reused elsewhere. This change handles both reserving and freeing IP addresses. For a given VIP nic, we reserve the IP address in DHCP when initially creating this VIP on a DHCP network, or moving its network traffic type to such a network. We delete the reservation when moving the VIP *away from* a DHCP network, or on deleting it. One side effect of this is VIP creation takes still longer than before, due to the extra DHCP proxy action, but we already have a "waiting..." dialog with spinner for this step.
Visual ACK. Really needs to be tested on a system that can repro the issue reliably. |
This patch works as expected, creating the reservations for vips in dhcpd.leases. However, it does not resolve the issue of conflicting dhcp entries. New hosts still get conflicting ip addresses despite existing dhcp host entries. |
@@ -3,15 +3,46 @@ class VipNic < Nic::Managed | |||
has_one :deployment_vip_nic, :dependent => :destroy, :class_name => 'Staypuft::DeploymentVipNic' | |||
has_one :deployment, :class_name => 'Staypuft::Deployment', :through => :deployment_vip_nic | |||
|
|||
before_save :reserve_ip | |||
before_save :reserve_ip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick... either make them line up, or don't change the before_save.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ew :) I agree
I haven't realized the patch does not actually work. Can you give an example of such a conflict? |
Example leases file: http://fpaste.org/169078/21159698/ Look at, for example, ip address 192.168.223.7. It's reserved using host vip_ceilometer_private_000000000101 with mac 00:00:00:00:01:01 but gets leased out to mac 52:54:00:75:3e:57 |
Taking the IP address you refer to here is the reservation:
But this is not a lease, it's a deleted lease and that is a difference.
Note the We had a bug in foreman-proxy. Proxy was seeing those "free" leases as "active", but this is already fixed. Can you check you backported it already? |
@mburns72h confirms the patch 239 is in. We have one additional patch, can you check you have it? It solves conflicts caused by Discovery, it was still not merged upstream but review is finished and it will be soon merged into master: |
Yes, that is in as well |
For the record: it looks like ISC DHCP issues leases for reserved hosts which are offline. |
https://bugzilla.redhat.com/show_bug.cgi?id=1176423
For VIPs on the provisioning network (with DHCP active), calling unused_ip
is insufficient. We also need to create the actual DHCP reservation
so the allocated IP address won't be reused elsewhere.
This change handles both reserving and freeing IP addresses. For a given
VIP nic, we reserve the IP address in DHCP when initially creating this
VIP on a DHCP network, or moving its network traffic type to such a network.
We delete the reservation when moving the VIP away from a DHCP network,
or on deleting it.
One side effect of this is VIP creation takes still longer than before, due
to the extra DHCP proxy action, but we already have a "waiting..." dialog with
spinner for this step.