-
Notifications
You must be signed in to change notification settings - Fork 62
[internal-dns] First pass at integrating service discovery codebase #800
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for doing this! CC @rcgoodfellow. I think it's worth keeping the CLI tool. Maybe make it a binary in internal-dns-client? |
Done, added as |
davepacheco
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't really reviewed most of the code here. But at this point I think the risk is low and urgency is important -- we should land this so we can make forward progress on the things that need it.
|
The prototype also as a testing workflow that runs the automated tests if you want to pick that up. |
rcgoodfellow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I did port the tests over, but Omicron has its own automation that should be running them: https://github.com/oxidecomputer/omicron/blob/main/.github/workflows/rust.yml |
Ports https://github.com/davepacheco/toy-dns into Omicron as
internal-dns:internal-dns-client, so clients can depend on the bindings without depending on the serverinternal-dns, which holds the server library, binary, and API generator. The "toyadm" interface has, for the moment, been removed.internal-dns/tests. Also adds a test for the generated OpenAPI file, and makes some minor tweaks viaserde(rename)to ensure it passes our linter.