Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#2025 closed task (fixed)

system test for notify

Reported by: jinmei Owned by: muks
Priority: high Milestone: Sprint-20120703
Component: xfrin Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 5 Add Hours to Ticket: 0
Total Hours: 2 Internal?: no

Description

We don't seem to have one. And there was a user report that
it didn't seem to work at least in some environment:
https://lists.isc.org/pipermail/bind10-users/2012-June/000358.html

(what should be mainly tested is whether an incoming valid notify can
trigger immediate xfrin for the zone)

Subtickets

Change History (15)

comment:1 Changed 8 years ago by jinmei

  • Priority changed from medium to high

comment:2 Changed 8 years ago by jaspain

Adding a comment in order to receive updates on this ticket.

comment:3 Changed 8 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20120619

comment:4 Changed 7 years ago by muks

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

Picking

comment:5 Changed 7 years ago by muks

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

Up for review.

This work is in the trac2025_2 branch. Please go through it thoroughly.

comment:6 Changed 7 years ago by jinmei

Please don't use 127.0.0.1:47807. It's explicitly noted in
lettuce/configurations/NOTES (while I was afraid it'd be easy to miss).

comment:7 Changed 7 years ago by muks

I just noticed your commit to master and came back here to add to this bug :)

I think we can change it all to use IPv6. Will fix it in the branch when it comes back after review.

comment:8 Changed 7 years ago by vorner

  • Owner changed from UnAssigned to vorner

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

  • Owner changed from vorner to muks

Hello

Does auth really process the notify, or does it only forward it to some other process?

% AUTH_PROCESS_NOTIFY processing incoming NOTIFY for zone name %1, zone class %2
This is a debug message reporting that an incoming NOTIFY is being processed.

I think the .toText() calls are not needed here, as the .arg() can take whatever has the operator <<:

.arg(question->getName().toText()).arg(question->getClass().toText());

The test_notify_slaves ‒ I don't like the idea of having „test“ options in the configuration. Also, what is the difference between this and the also_notify configuration we plan to have?

What is the purpose of transfer_acl on the test_notify_slaves? This is for Xfr Out, we are sending the notify out here.

Also, the lettuce tests failed for me:

 Given I have bind10 running with configuration xfrin/retransfer_master.conf with cmdctl port 47804 as master # features/terrain/bind10_control.py:107
    And wait for master stderr message BIND10_STARTED_CC                                                         # features/terrain/steps.py:34
    And wait for master stderr message CMDCTL_STARTED                                                            # features/terrain/steps.py:34
    And wait for master stderr message AUTH_SERVER_STARTED                                                       # features/terrain/steps.py:34
    And wait for master stderr message XFROUT_STARTED                                                            # features/terrain/steps.py:34
    And wait for master stderr message ZONEMGR_STARTED                                                           # features/terrain/steps.py:34
    And I have bind10 running with configuration xfrin/inmem_slave.conf                                          # features/terrain/bind10_control.py:107
    And wait for bind10 stderr message BIND10_STARTED_CC                                                         # features/terrain/steps.py:34
    And wait for bind10 stderr message CMDCTL_STARTED                                                            # features/terrain/steps.py:34
    And wait for bind10 stderr message AUTH_SERVER_STARTED                                                       # features/terrain/steps.py:34
    And wait for bind10 stderr message XFRIN_STARTED                                                             # features/terrain/steps.py:34
    And wait for bind10 stderr message ZONEMGR_STARTED                                                           # features/terrain/steps.py:34
    A query for www.example.org should have rcode NOERROR                                                        # features/terrain/querying.py:204
      | """                                                        |
      | www.example.org.        3600    IN      A       192.0.2.63 |
      | """                                                        |
    A query for mail.example.org should have rcode NXDOMAIN                                                      # features/terrain/querying.py:204
    When I send bind10 the command Xfrin retransfer example.org IN ::1 47807                                     # features/terrain/bind10_control.py:310
    Then wait for new bind10 stderr message XFRIN_TRANSFER_SUCCESS not XFRIN_XFR_PROCESS_FAILURE                 # features/terrain/steps.py:34
    Traceback (most recent call last):
      File "/home/vorner/.local/lib64/python2.7/site-packages/lettuce/core.py", line 117, in __call__
        ret = self.function(self.step, *args, **kw)
      File "/home/vorner/work/bind10/tests/lettuce/features/terrain/steps.py", line 52, in wait_for_stderr_message
        (found, line) = world.processes.wait_for_stderr_str(process_name, strings, new, int(times))
      File "/home/vorner/work/bind10/tests/lettuce/features/terrain/terrain.py", line 342, in wait_for_stderr_str
        matches)
      File "/home/vorner/work/bind10/tests/lettuce/features/terrain/terrain.py", line 242, in wait_for_stderr_str
        strings, only_new, matches)
      File "/home/vorner/work/bind10/tests/lettuce/features/terrain/terrain.py", line 226, in _wait_for_output_str
        assert False, "Timeout waiting for process output: " + str(strings)
    AssertionError: Timeout waiting for process output: [u'XFRIN_TRANSFER_SUCCESS', u'XFRIN_XFR_PROCESS_FAILURE']
    Then wait for new bind10 stderr message AUTH_LOAD_ZONE                                                       # features/terrain/steps.py:34
    A query for www.example.org should have rcode NOERROR                                                        # features/terrain/querying.py:204
    The answer section of the last query response should be                                                      # features/terrain/querying.py:286
      | """                                                       |
      | www.example.org.        3600    IN      A       192.0.2.1 |
      | """                                                       |
    A query for mail.example.org should have rcode NOERROR                                                       # features/terrain/querying.py:204

comment:10 in reply to: ↑ 9 ; follow-up: Changed 7 years ago by muks

  • Owner changed from muks to vorner

Hi vorner

Replying to vorner:

Does auth really process the notify, or does it only forward it to some other process?

% AUTH_PROCESS_NOTIFY processing incoming NOTIFY for zone name %1, zone class %2
This is a debug message reporting that an incoming NOTIFY is being processed.

It is some kind of processing. :) The function where it is logged is called AuthSrvImpl::processNotify(). I have changed the message id and description from processed to received.

I think the .toText() calls are not needed here, as the .arg() can take whatever has the operator <<:

.arg(question->getName().toText()).arg(question->getClass().toText());

*nod*. Done.

The test_notify_slaves ‒ I don't like the idea of having „test“ options in the configuration. Also, what is the difference between this and the also_notify configuration we plan to have?

"test_" is in the key because it is only meant to be used in testing.

On the difference between this and also_notify, I don't know what also_notify is supposed to do. But test_notify_slaves is a list of slave addresses which get sent NOTIFYs by Xfrout. It is necessary for having this test run under lettuce right now. If also_notify is implemented in the future, we can revisit this test

What is the purpose of transfer_acl on the test_notify_slaves? This is for Xfr Out, we are sending the notify out here.

Indeed, this was erroneously left in the schema. It was not used. Removed now.

Also, the lettuce tests failed for me:

    When I send bind10 the command Xfrin retransfer example.org IN ::1 47807

This is the inmemory_over_sqlite3.feature ? Strange that it passed for me here! In any case I have made the changes that jinmei suggested about address config so it all runs in IPv6.

comment:11 in reply to: ↑ 10 ; follow-up: Changed 7 years ago by vorner

  • Owner changed from vorner to muks

Hello

Replying to muks:

The test_notify_slaves ‒ I don't like the idea of having „test“ options in the configuration. Also, what is the difference between this and the also_notify configuration we plan to have?

"test_" is in the key because it is only meant to be used in testing.

On the difference between this and also_notify, I don't know what also_notify is supposed to do. But test_notify_slaves is a list of slave addresses which get sent NOTIFYs by Xfrout. It is necessary for having this test run under lettuce right now. If also_notify is implemented in the future, we can revisit this test

Well, I understand the reason and the meaning of the configuration variable. And that is exactly what I don't like ‒ having user-facing configuration that should not be used.

The also_notify is something a user might want to use when they need to send notifies to addresses not listed in the NS records. It seems to me this is exactly the same feature. So I propose to rename it to also-notify, make a note about it to #1992 and put it to next-sprint-proposed to add tests for it. Would that be OK with you?

This is the inmemory_over_sqlite3.feature ? Strange that it passed for me here! In any case I have made the changes that jinmei suggested about address config so it all runs in IPv6.

It passed now.

comment:12 in reply to: ↑ 11 Changed 7 years ago by muks

  • Owner changed from muks to vorner

Replying to vorner:

Well, I understand the reason and the meaning of the configuration variable. And that is exactly what I don't like ‒ having user-facing configuration that should not be used.

The also_notify is something a user might want to use when they need to send notifies to addresses not listed in the NS records. It seems to me this is exactly the same feature. So I propose to rename it to also-notify, make a note about it to #1992 and put it to next-sprint-proposed to add tests for it. Would that be OK with you?

I've updated the key to also_notify now, though it's still limited to just address and port keys. Also did some minor cleanup. Will update #1992 when it hits master.

comment:13 Changed 7 years ago by vorner

  • Owner changed from vorner to muks
  • Total Hours changed from 0 to 2

Thanks. It looks OK to merge now.

comment:14 Changed 7 years ago by muks

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

Merged to master:

* e4743cb [2025] Cleanup code
* d8a4785 [2025] Rename test_notify_slaves to also_notify
* 9b410a8 [2025] Make xfr master/slaves run on IPv6 entirely (to avoid issues with certain ports on 127.0.0.1)
* d6c5f06 [2025] Remove spec schema that was left in by mistake
* 05b81ac [2025] Remove redundant .getText() calls
* 84d2432 [2025] Rename message id and description
* 0cd3e78 [2025] Fix logic in notify_out.add_slave()
* 1b235c7 [2025] Reconnect after getting SOA for the actual xfrin
* 8e376a0 [2025] Add a way to specify slaves for use in lettuce tests
* 2f691c2 [2025] Add lettuce test for notify
* 73d7104 [2025] Make the master run on 127.0.0.1
* d249e34 [2025] Change syntax to fix cmdctl port handling in lettuce
* ca4a440 [2025] Log a message when a notify is received by b10-auth

Resolving bug as fixed. Thank you for the reviews vorner.

comment:15 Changed 7 years ago by muks

Updated #1992 with a comment too.

Note: See TracTickets for help on using tickets.