Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#3567 closed enhancement (complete)

Extend MySQL with the ability to store hosts

Reported by: tomek Owned by: tomek
Priority: medium Milestone: Kea0.9.2-beta
Component: database-backend Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

We need to extend MySQL to be able to store host reservations.

It is not decided yet whether this will be an extension to existing MySQLLeaseMgr or HostMgr?.

Subtickets

Change History (15)

comment:1 Changed 3 years ago by hschempf

  • Priority changed from medium to low

comment:2 Changed 3 years ago by kalmus

  • Owner set to kalmus
  • Status changed from new to assigned

comment:3 Changed 3 years ago by tomek

The host reservation design is described here: http://kea.isc.org/wiki/HostReservationDesign

comment:4 Changed 3 years ago by tomek

The design mentioned in comment 3 features a solid proposal for the MySQL database schema. MySQL schema is updated using kea-admin. See src/bin/admin and src/bin/admin/scripts/mysql for previous updates. We do have upgrade 1.0 to 2.0 that did insert extra columns in lease6 columns.

You may also want to read Section 5 of Kea User's guide that explains how to use kea-admin (it's currently part of #3644 ticket, but it should be merged in the next couple days).

comment:5 Changed 3 years ago by tomek

  • Milestone changed from Kea0.9.1beta to Kea0.9.1

comment:6 Changed 3 years ago by marcin

  • Milestone changed from Kea0.9.1 to Kea1.1

comment:7 Changed 3 years ago by tomek

  • Owner changed from kalmus to UnAssigned
  • Status changed from assigned to reviewing

comment:8 Changed 3 years ago by tomek

  • Owner changed from UnAssigned to tomek

comment:9 Changed 3 years ago by tomek

  • Owner changed from tomek to kalmus

I received a patch over mail from Adam and committed it on a branch trac3567 (commit e9fc3ede80d833c0b186c1ec4cb4a32a8ebb46ed).

The changes look mostly good, but there are a couple of things that has to be changed.

The tables are created in a separate database called kea_host_reservation. They should
be created in the usual database, not in a separate one. Please fix this in the database
initialization script, but also in the HostReservationDesign page. A bit of explanation:
during the design phase, ISC engineers had a discussion and decided that it should be
possible to keep host reservation in a separate database. The design as of today
not only makes it possible, but actually enforces it. That's too much. While we should
keep in mind that some people may want to keep reservation details separated, most
users will simply keep everything DHCP-related in a single database.

There are a couple of directives:

SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0;
SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0;
SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='TRADITIONAL,ALLOW_INVALID_DATES';

I'm not MySQL expect, so I don't understand them. In particular, allow_invalid_dates look suspicious.
Unless there's a good reason for keeping them, please remove them from both the script and the design.

The initialization script is used in new installations. We also need an
upgrade script for people who run old Kea installation and are migrating
to the new one. Note that kea-admin loads scripts from src/bin/admin/scripts/mysql/upgrade-*.sh
in alphabetic order. Please name your script appropriately (upgrade_2.0_to_3.0.sh).

Feel free to take a look at upgrade_1.0_to_2.0.sh as an example.

Once those scripts are in place, make sure you also update test tests.
They are in mysql_tests.sh.in. To run them:

./configure --with-dhcp-mysql
make
make check (or cd src/bin/admin; make check)

Currently the tests fail:

make[3]: Entering directory `/home/thomson/devel/kea2/src/bin/admin/tests'
for shtest in memfile_tests.sh mysql_tests.sh ; do \
	echo Running test: $shtest ; \
	chmod +x /home/thomson/devel/kea2/src/bin/admin/tests/$shtest ; \
	/bin/bash /home/thomson/devel/kea2/src/bin/admin/tests/$shtest || exit ; \
	done
Running test: memfile_tests.sh

START TEST memfile.init
PASSED memfile.init


START TEST memfile.version
PASSED memfile.version


START TEST memfile.upgrade
PASSED memfile.upgrade

Running test: mysql_tests.sh

START TEST mysql.lease-init
Wiping whole database keatest
Checking if there is a database initialized already. Please ignore errors.
Initializing database using script /home/thomson/devel/kea2/src/bin/admin/scripts/mysql/dhcpdb_create.mysql
ERROR 1044 (42000) at line 173: Access denied for user 'keatest'@'localhost' to database 'kea_host_reservation'
mysql returned status code 1
Assertion failure: 0 != 1, for val1=0, val2=1
kea-admin lease-init mysql returned non-zero status code 0, expected 1
FAILED mysql.lease-init

make[3]: *** [check-local] Error 1
make[3]: Leaving directory `/home/thomson/devel/kea2/src/bin/admin/tests'
make[2]: *** [check-am] Error 2
make[2]: Leaving directory `/home/thomson/devel/kea2/src/bin/admin/tests'
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory `/home/thomson/devel/kea2/src/bin/admin/tests'
make: *** [check-recursive] Error 1

Please propose ChangeLog entry. (See ChangeLog? file) Make sure the entry is
clear that this ticket updates the schema only and the tables are not used yet.
Otherwise users will complain that they put some data there and nothing
happened.

Finally, as this change introduces new files, make sure that it will not
break down the release. In particular, the release involves (among many
other steps) making tar.gz file. This is done with:

make dist

To check whether this tar.gz contains all necessary files (including those
you just added), you can run:

make distcheck

It may require updating Makefile.am in the appropriate directory.

Reassigning this ticket back to Adam. Once you deal with those comments,
please reassign it back to me.

Thanks a lot for doing this work.

comment:10 Changed 3 years ago by tomek

  • Priority changed from low to medium

Bumping up priority. It used to be low in 0.9.1, when we were not sure whether it will make it or not. It's definitely not low anymore.

comment:11 Changed 3 years ago by kalmus

  • Owner changed from kalmus to tomek

comment:12 Changed 3 years ago by kalmus

I'm returning ticket after sending new patch which includes changes as follows:

-New tables are now created in the same database as those associated with leases.

-Removed directives mentioned earlier, for what I understand they could speed up the mysql server import process, but at the cost of skipping some validations. Yet, I'm not a MySQL expert either...

-Added upgrade_2.0_to_3.0.sh script

-Added some tests for new host reservation tables and upgrade script.
Test errors quoted earlier are most likely results of missing keatest user, keatest database and/or necessary privileges, which, according to User's Manual chapter "4.3.2.1. First Time Creation of Kea Database", should be created manually before executing tests.

-Finally, Makefile.am and configure.ac has been updated to include new upgrade file.


Thanks for this guidelines and hope this time everything will work fine.

comment:13 Changed 3 years ago by kalmus

Proposed ChangeLog entry:

XXX.	[func]		kalmus
	 MySQL schema has been extended with tables that can store 
         host reservation. This ticket updates database schema only, 
         the tables are not in use yet.

	(Trac #3567, git xxx)

comment:14 Changed 3 years ago by tomek

  • Resolution set to complete
  • Status changed from reviewing to closed

Thanks for submitting this updated patch. I have reviewed it and it looks good.
I check that the code compiles, unit-tests and make distcheck passes on Ubuntu 14.04.

You ChangeLog? proposal looks good.

I checked your changes to trac3567 and merged it to master, updated ChangeLog? and AUTHORS file.

Code is now merged, closing ticket.

Thanks a lot for doing this work.

comment:15 Changed 2 years ago by hschempf

  • Milestone changed from Kea1.1 to Kea0.9.2-beta

Moved from 1.1 to 0.9.2 beta (per Stephen, this ticket was merged and went out during 0.9.2)

Note: See TracTickets for help on using tickets.