Opened 9 years ago

Closed 9 years ago

#294 closed defect (fixed)

friendlier error for incompatible/old configuration database

Reported by: jreed Owned by: jelte
Priority: medium Milestone: 06. 4th Incremental Release
Component: configuration 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.5 Internal?: no

Description

[bind10] Starting b10-cfgmgr
[bind10] Started b10-cfgmgr (PID 5520)
Traceback (most recent call last):
  File "/home/reed/opt/bind10/lib/python3.1/site-packages/isc/config/cfgmgr.py", line 70, in read_from_file
    file_config = json.loads(file.read())
  File "/usr/pkg/lib/python3.1/json/__init__.py", line 291, in loads
    return _default_decoder.decode(s)
  File "/usr/pkg/lib/python3.1/json/decoder.py", line 325, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/pkg/lib/python3.1/json/decoder.py", line 341, in raw_decode
    obj, end = self.scan_once(s, idx)
ValueError: Expecting property name: line 1 column 1 (char 1)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/reed/opt/bind10/libexec/bind10-devel/b10-cfgmgr", line 61, in <module>
    main()
  File "/home/reed/opt/bind10/libexec/bind10-devel/b10-cfgmgr", line 49, in main
    cm.read_config()
  File "/home/reed/opt/bind10/lib/python3.1/site-packages/isc/config/cfgmgr.py", line 210, in read_config
    self.config = ConfigManagerData.read_from_file(self.data_path)
  File "/home/reed/opt/bind10/lib/python3.1/site-packages/isc/config/cfgmgr.py", line 81, in read_from_file
    raise ConfigManagerDataReadError("Config file unreadable")
isc.config.cfgmgr.ConfigManagerDataReadError: Config file unreadable

Have a nice error message instead of this trace. Like if fails on input of old db, then that could say "out of date or corrupt".

I understand that documentation may be used to help teach how to update and this is a development release so no tool is needed for upgrade.

Also bump the version number (even though that would not have helped this time).

Subtickets

Change History (16)

comment:1 Changed 9 years ago by jreed

  • Add Hours to Ticket changed from 0.0 to 0.25
  • Total Hours changed from 0.0 to 0.25

comment:2 Changed 9 years ago by jelte

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

Error message should be better now (also includes file path btw).

Updated the code around that read as well.

I've also bumped the version, and fixed the other occurrences that had hardcoded version number here. In order to do that right, i had to move the constant out of cfgmgr.py (which isn't a 'general library' module, and i put it under another name in config_data.py (which is).

Iff it can read the old (version 1) file, cfgmgr automatically 'updates' it to 2.

changes in branches/trac294, relevant revs are r2609 and r2610

comment:3 Changed 9 years ago by jelte

Oh, and b10-cfgmgr now exits with a non-zero value if it a config file exists but can't be read

comment:4 follow-up: Changed 9 years ago by jreed

  • Add Hours to Ticket changed from 0.0 to 0.25
  • Total Hours changed from 0.25 to 0.5

error message is fine. I think we will need a different ticket opened to handle the following problem: b10-msgq is running but appears to be hanging and bind10 just hangs too.

t1:trac294$ /home/reed/opt/bind10/sbin/bind10 --verbose                        
BIND 10 v20100531
[bind10] Checking for already running b10-msgq
[bind10] Started b10-msgq (PID 4882)
[bind10] Starting b10-cfgmgr
[bind10] Started b10-cfgmgr (PID 9236)
Config file out of date or corrupt, please update or remove /home/reed/opt/bind10/var/bind10-devel/b10-config.db
[b10-msgq] Closing socket fd 5
[b10-msgq] Receive error: EOF
[bind10] starting ccsession

comment:5 Changed 9 years ago by jreed

I also tested the update (1 to 2). It worked for me. Please consider having it output/log a message that this was done.

comment:6 in reply to: ↑ 4 Changed 9 years ago by jreed

Config file out of date or corrupt, please update or remove /home/reed/opt/bind10/var/bind10-devel/b10-config.db

Also please prefix the message to know what component reported this problem. or maybe that doesn't matter.

comment:7 Changed 9 years ago by jelte

Better print in r2613, added a print about update in r2614 (will need to do this in 'real' logging as INFO, but it doesn't use real logging yet).

As for the failing startup; in this case we can solve it when i implement timeouts on cc/session.X, but another thing is that bob shouldn't continue in the first place, which is something that we should implement when we fix the hardcoded startups in there. (i just tried simply doing shutdown() if the failing process is b10-cfgmgr but then you get weird errors for performing shutdown while it is still in initialization, so it needs something smarter)

comment:8 Changed 9 years ago by jreed

I don't see any way (currently) for it to show the "Old version" message. I tried by manually setting to 0.

Also if the version is newer (like downgrading software) it may say "out of date". Maybe it should be specific and say version in database is newer (3) than the software supports (2).

comment:9 Changed 9 years ago by jelte

yeah the nested try and catch-all didn't work out, too old/too new should now be shown in r2615

comment:10 Changed 9 years ago by jelte

  • Milestone set to 06. 4th Incremental Release

comment:11 Changed 9 years ago by jreed

  • Owner changed from UnAssigned to each

comment:12 Changed 9 years ago by each

  • Owner changed from each to jelte

I don't see any problems except for cosmetic ones:

+ if config_data.BIND10_CONFIG_DATA_VERSION > file_configversion?:
+ raise ConfigManagerDataReadError?("Version of config data too old")
+ else:
+ raise ConfigManagerDataReadError?("Version of config data too new")

Those error messages strike me as a little terse and potentially confusing. Maybe something like "Cannot load configuration file: format version %d is no longer supported" or "...is not yet supported"?

Also sometimes the text says "config file" and other times "configuration file"; we should probably pick one or the other.

comment:13 Changed 9 years ago by jelte

  • Owner changed from jelte to each

ok, applied your suggestions in r2692, please verify

i also made the catch-all only catch json's ValueError? (which is raised on a parse error), for which it was intended (i thought i had removed that one already; it catched our own exceptions there)

comment:14 Changed 9 years ago by jelte

r2694 changes three more instances of 'config file' to 'configuration file'

comment:15 Changed 9 years ago by each

  • Owner changed from each to jelte

Looks fine. Go ahead and merge.

comment:16 Changed 9 years ago by jelte

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

Thanks!

merged to trunk in r2695, closing ticket.

Note: See TracTickets for help on using tickets.