Skip to content

Revert "DNS servers should have NS and SOA records (#8047)"#8272

Closed
iximeow wants to merge 1 commit into
mainfrom
ixi/revert-8047
Closed

Revert "DNS servers should have NS and SOA records (#8047)"#8272
iximeow wants to merge 1 commit into
mainfrom
ixi/revert-8047

Conversation

@iximeow

@iximeow iximeow commented Jun 4, 2025

Copy link
Copy Markdown
Member

This reverts commit 3e68262.

The change in 3e68262 does not handle upgrade sufficiently: when internal DNS starts it tries to parse a CurrentConfig from a json blob describing the previous server version's config. As Angela found on dogfood, this means internal DNS will fail to start with an error about "missing field serial at line 1 column ...". Internal DNS failing to start means we'll never get Nexus up, so we can't Reconfigurator our way to a new blueprint and the desired DNS configuration with a serial number.

We're reverting this to unblock dogfood, but will add this back in with additional logic to handle this case.

Comment thread dns-server/tests/basic_test.rs Outdated
This reverts commit 3e68262.

The change in 3e68262 does not handle upgrade sufficiently: when
internal DNS starts it tries to parse a `CurrentConfig` from a json blob
describing the previous server version's config. As Angela found on
dogfood, this means internal DNS will fail to start with an error about
"missing field `serial` at line 1 column ..."

We're reverting this to unblock dogfood, but will add this back in with
additional logic to handle this case.

@karencfv karencfv left a comment

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.

Looks good!

@iximeow

iximeow commented Jun 5, 2025

Copy link
Copy Markdown
Member Author

since Dave got to #8278 early this morning i went ahead and merged that one - that fixes things for the upgrade path, so we won't need to unwind the whole change. thank you for giving this a look in case we needed to go with this one, @karencfv !

@iximeow iximeow closed this Jun 5, 2025
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