Opened 8 years ago

Closed 8 years ago

#467 closed task (fixed)

"reload zone" command for b10-auth

Reported by: jinmei Owned by: jinmei
Priority: medium Milestone: y2 12 month milestone
Component: b10-auth Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: 0.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

This is originally intended to be a subtask of "AXFR in (for in memory data source)". AXFR in may be too big for this sprint per se, but reloading feature is necessary regardless and should be a releatively reasonable size of task. So I'll work on it.

Subtickets

Change History (11)

comment:1 Changed 8 years ago by jinmei

  • Owner changed from jinmei to UnAssigned
  • Status changed from new to reviewing

branches/trac467 is ready for review.

This may be a bit big branch. If anyone willing to review it feels it's too big, please let me know. Then I'll divide it to the changes to datasrc and bin/auth (the latter depends on the former).

Some other notes:

  • I've changed the zonetable and MemoryDataSrc? interface so that the zone pointer returned by findZone() is now mutable. As original documented we actually expected this change, and I think this is the time to do that. If we can keep things constant that would be generally great, but it's not feasible for objects that can be dynamically updated.
  • I introduced a generic framework for command handling. this means the patch includes diffs that is irrelevant to the purpose of this ticket, but the necessary change seemed small so I used this opportunity to do it. One good thing to do so is that we can add tests for other commands (and I indeed did so). But if the reviewer thinks it should be differred to a different ticket, I'm happy to cancel this part.

comment:2 follow-up: Changed 8 years ago by jinmei

Proposed changelog entry:

  143.?	[func]*		jinmei
	b10-auth: added a new command 'loadzone' for (re)loading a
	specific zone.  The command syntax is generic but it is currently
	only feasible for class IN in memory data source.  To reload a
	zone "example.com" via bindctl, execute the command as follows:
	> Auth loadzone origin = example.com
	(Trac #467, svn rTBD)

comment:3 in reply to: ↑ 2 ; follow-up: Changed 8 years ago by jreed

Replying to jinmei:

Proposed changelog entry:

  143.?	[func]*		jinmei
	b10-auth: added a new command 'loadzone' for (re)loading a
	specific zone.  The command syntax is generic but it is currently
	only feasible for class IN in memory data source.  To reload a
	zone "example.com" via bindctl, execute the command as follows:
	> Auth loadzone origin = example.com
	(Trac #467, svn rTBD)

Where does it load / reload it from? (Maybe changelog entry should have answer.)

comment:4 in reply to: ↑ 3 ; follow-up: Changed 8 years ago by jinmei

Replying to jreed:

Where does it load / reload it from? (Maybe changelog entry should have answer.)

How about this?

  143.?	[func]*		jinmei
	b10-auth: added a new command 'loadzone' for (re)loading a
	specific zone.  The command syntax is generic but it is currently
	only feasible for class IN in memory data source.  To reload a
	zone "example.com" via bindctl, execute the command as follows:
	> Auth loadzone origin = example.com
	The zone file to be loaded is the one specified in the
	configuration (see change #138). (Trac #467, svn rTBD)

comment:5 in reply to: ↑ 4 Changed 8 years ago by jreed

Replying to jinmei:

Replying to jreed:

Where does it load / reload it from? (Maybe changelog entry should have answer.)

How about this?

  143.?	[func]*		jinmei
	b10-auth: added a new command 'loadzone' for (re)loading a
	specific zone.  The command syntax is generic but it is currently
	only feasible for class IN in memory data source.  To reload a
	zone "example.com" via bindctl, execute the command as follows:
	> Auth loadzone origin = example.com
	The zone file to be loaded is the one specified in the
	configuration (see change #138). (Trac #467, svn rTBD)

Yes. Thanks for clarification (thanks for reminder!).

comment:6 Changed 8 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:7 follow-up: Changed 8 years ago by vorner

  • Owner changed from vorner to jinmei

There are two small things.

The AuthSrv::io_service doesn't seem to be set anywhere (or, I did find no occurrence of setting it). So I guess this will cause the server to crash with the FatalError? every time a shutdown is called on it, which isn't nice. Or, did I overlook something?

Second one, the framework for commands looks like it could (and should) be shared with the recursive server. Wouldn't it be worth putting it into some library? Now it looks like the servers are completely independent products, even at places where they do nearly the same thing and the differences keep growing. Is it planned to be unified at some time?

comment:8 in reply to: ↑ 7 ; follow-up: Changed 8 years ago by jinmei

Replying to vorner:

Thanks for the review.

There are two small things.

The AuthSrv::io_service doesn't seem to be set anywhere (or, I did find no occurrence of setting it). So I guess this will cause the server to crash with the FatalError? every time a shutdown is called on it, which isn't nice. Or, did I overlook something?

It's set via setIOService() from main():

        auth_server->setIOService(io_service);
        cout << "[b10-auth] IOService created." << endl;

Second one, the framework for commands looks like it could (and should) be shared with the recursive server. Wouldn't it be worth putting it into some library? Now it looks like the servers are completely independent products, even at places where they do nearly the same thing and the differences keep growing. Is it planned to be unified at some time?

Yes, I was aware of that possibility. This stuff and (with some modification/generalization) the config framework will better be shared by both auth and recurse (or any other C++ server apps). There's no specific plan for it though. It should go to a separate task anyway, but I'll create a placeholder ticket for it for the record if you want.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 8 years ago by vorner

Replying to jinmei:

It's set via setIOService() from main():

Ah, it is in the header file, I overlooked it. Sorry.

Yes, I was aware of that possibility. This stuff and (with some modification/generalization) the config framework will better be shared by both auth and recurse (or any other C++ server apps). There's no specific plan for it though. It should go to a separate task anyway, but I'll create a placeholder ticket for it for the record if you want.

It would be nice, thanks.

So otherwise it looks OK, so please merge.

comment:10 in reply to: ↑ 9 Changed 8 years ago by jinmei

Replying to vorner:

Yes, I was aware of that possibility. This stuff and (with some modification/generalization) the config framework will better be shared by both auth and recurse (or any other C++ server apps). There's no specific plan for it though. It should go to a separate task anyway, but I'll create a placeholder ticket for it for the record if you want.

It would be nice, thanks.

Created a new ticket (#481).

So otherwise it looks OK, so please merge.

Merged, closing ticket.

comment:11 Changed 8 years ago by jinmei

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.