Skip to content

sql: re-seed lookup rows on upgrade - #63

Open
aberoham wants to merge 2 commits into
NicTool:mainfrom
aberoham:upgrade-lookup-rows
Open

sql: re-seed lookup rows on upgrade#63
aberoham wants to merge 2 commits into
NicTool:mainfrom
aberoham:upgrade-lookup-rows

Conversation

@aberoham

Copy link
Copy Markdown
Contributor

A database created by 2.x has the nameserver export types and record types of its day; nothing in sql/upgrade added the ones v3 introduced, so an upgraded install can't create a coredns or native nameserver (lib/nameserver/test/runtime.js fails with unknown nameserver type: native on such a database). Found by running a 2.41 production dump through the upgrade path; with this file applied that database passes the runtime tests. INSERT IGNORE keeps an operator's own row under a clashing id; the header says how to check.

Comment thread sql/upgrade/06_reseed_lookup_tables.sql Outdated
@@ -0,0 +1,54 @@
# Copyright 2004-2024 The Network People, Inc.
#
# Lookup rows added since NicTool 2.x. New installs get these from

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Lookup rows added since NicTool 2.x. New installs get these from
# Lookup rows added in NicTool 3.0. New installs get these from

@msimerson

Copy link
Copy Markdown
Contributor
# Safe to re-run: INSERT IGNORE keeps
# existing rows. That also means a row you added yourself under one of
# these ids keeps its name and the v3 type it shadows stays unavailable;
# check with
#   SELECT id, name FROM nt_nameserver_export_type WHERE id >= 9;
# and renumber yours first if it clashes.

That sounds like really dangerous advice:

 NicTool/server/sql ➜ grep nt_nameserver_id *                            on  master [$!] via 🐪 v5.40.4 
04_nt_nameserver.sql:    nt_nameserver_id    SMALLINT UNSIGNED AUTO_INCREMENT NOT NULL,
04_nt_nameserver.sql:    PRIMARY KEY (`nt_nameserver_id`),
04_nt_nameserver.sql:    KEY `nt_nameserver_idx1` (`name`),
04_nt_nameserver.sql:    KEY `nt_nameserver_idx2` (`deleted`),
04_nt_nameserver.sql:    nt_nameserver_id    SMALLINT UNSIGNED NOT NULL,
04_nt_nameserver.sql:    KEY `nt_nameserver_log_idx1` (`nt_nameserver_id`),
04_nt_nameserver.sql:INSERT INTO nt_nameserver_log(nt_group_id,nt_user_id, action, timestamp, nt_nameserver_id) VALUES (1,1,'added',UNIX_TIMESTAMP(), 1);
04_nt_nameserver.sql:INSERT INTO nt_nameserver_log(nt_group_id,nt_user_id, action, timestamp, nt_nameserver_id) VALUES (1,1,'added',UNIX_TIMESTAMP(), 2);
04_nt_nameserver.sql:INSERT INTO nt_nameserver_log(nt_group_id,nt_user_id, action, timestamp, nt_nameserver_id) VALUES (1,1,'added',UNIX_TIMESTAMP(), 3);
04_nt_nameserver.sql:    nt_nameserver_id                SMALLINT UNSIGNED NOT NULL,
04_nt_nameserver.sql:    KEY `nt_nameserver_export_log_idx1` (`nt_nameserver_id`)
08_nt_zone.sql:    nt_nameserver_id     smallint(5) unsigned NOT NULL,
08_nt_zone.sql:    UNIQUE KEY `zone_ns_id` (`nt_zone_id`,`nt_nameserver_id`)

@aberoham
aberoham force-pushed the upgrade-lookup-rows branch 3 times, most recently from c810950 to 3e97628 Compare August 29, 2026 18:13
@aberoham

aberoham commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

sorry I was having my clanker reason about its wanker move here, add tests to avoid similar facepalm

aberoham and others added 2 commits August 29, 2026 22:21
A database created by 2.x has the nameserver export types and record
types of its day; nothing in sql/upgrade added the ones v3 introduced,
so an upgraded install cannot create a coredns or native nameserver
(lib/nameserver/test/runtime.js fails with "unknown nameserver type:
native" on such a database).

The header claims an id a site already uses keeps its own row and skips
the one below it. A test runs that claim rather than asserting it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014AUjj218gbhUHLi7kvDPtd
@aberoham
aberoham force-pushed the upgrade-lookup-rows branch from 774aaf4 to 073887b Compare August 29, 2026 21:21
@aberoham

Copy link
Copy Markdown
Contributor Author

The ubuntu-latest, 24 failure isn't from this PR. It only touches sql/, and
the same commit passed test (ubuntu-latest, 22), test-mac, test-win and
test-docker. GET /zone_record paginates has been intermittent since at
least 2026-08-24, which is when the DIAGZR instrumentation went in.

That instrumentation did catch something this run. The three records the test
creates come back with ids from the POST, then aren't in the table:

DIAGZR createdIds= [60002,60003,60211,60212,60213]
DIAGZR all-records-for-zid= [5096, 60002, 60003]

So it isn't the pagination or the search filter — rows that reported
themselves created are absent. I'm digging into that separately. This job
likely just needs a re-run to go green.

aberoham added a commit to NicTool/metarepo that referenced this pull request Aug 29, 2026
NicTool/api#63 added a new sql file whose header read `# Copyright
2004-2024`, copied
from an older file two years out of date. It took the maintainer
noticing to fix
it.

A copyright line in the first ten lines of a file now has to name the
current
year as its newest. Run against that original commit, it says:

```
sql/upgrade/06_reseed_lookup_tables.sql:1: copyright runs to 2024, not 2026 (a line you add carries today's year)
```

The ten-line window is what separates a banner from a string that builds
one:
without it the rule flags its own test fixtures, which is how the first
attempt
at this went. Checking only comments doesn't work either — `.sql`
comments are
`--` here, so the `#` header that carried the bug classifies as code.
aberoham added a commit to NicTool/metarepo that referenced this pull request Aug 29, 2026
NicTool/api#63 fails intermittently on node 24 while the same head
passes node 22, macOS, Windows, and docker. Local repetitions need to
keep the failed run's diagnostics without paying the virtiofs cost of a
bind-mounted `node_modules`.

Add an opt-in `make stress-api RUNTIME=node:24 N=25` target. It copies
the api checkout into a runtime-derived image, runs all iterations
against the compose db, prints failed output, retains every log, and
reports the measured flake rate. `make test` is unchanged.

At api#63 head `073887b`, node v24.20.0 passed 25/25 runs (395 tests
each): 0 failures, skips, or cancellations; 0.00% flake rate; no
`DIAGZR` block. Single runs also passed with node 25 and node 24 alpine.
Test concurrency is the next variable to change because that revision
passes all mysql test files to one unconstrained `node --test`
invocation.

Checks:

- `uv run --with pyyaml==6.0.2 python -m unittest discover -v`
- `uvx ruff check .`
- `docker compose --env-file docker/.env --profile all --profile test
--profile stress config --quiet`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants