Skip to content
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

Remove nsupdate module #1645

Closed
wants to merge 13 commits into from
Closed

Remove nsupdate module #1645

wants to merge 13 commits into from

Conversation

dappnodedev
Copy link
Contributor

Overrides #1588

@github-actions github-actions bot temporarily deployed to commit October 17, 2023 15:50 Inactive
@github-actions
Copy link

github-actions bot commented Oct 17, 2023

@github-actions
Copy link

github-actions bot commented Oct 17, 2023

DAppNode bot has built and pinned the release to an IPFS node, for commit: d5c64ca

This is a development version and should only be installed for testing purposes, install link

/ipfs/QmdCfn37h3bjNUKHg2BrKTd16X2tV5eeLhyrbecd3M998W

(by dappnodebot/build-action)

@@ -211,6 +211,7 @@ export default async function initializeDb(): Promise<void> {
// - Updates the domain: db.domain.set(domain);
dyndns.generateKeys(); // Auto-checks if keys are already generated

// TODO: Move this functionality to Bind v2 package
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we are mapping the resolution of the dyndns to the private IP, instead of the public one, to improve UX. This should be done in the bind package instead. We can leave this here only until this is implemented

@github-actions github-actions bot temporarily deployed to commit October 18, 2023 08:13 Inactive
@@ -327,6 +327,9 @@ export class ComposeEditor {
!Array.isArray(service.environment)
)
service.environment = stringifyEnvironment(service.environment);

// Remove hardcoded DNS
service.dns = undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We must check that this does not cause any trouble. Also, we should make sure the compose files of the packages are created without the dns instruction

@@ -55,6 +55,7 @@ describe("modules / compose / networks", () => {
networks: {
[networkName]: { aliases },
},
dns: undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The deep equal test requires this to be added. Is it correct?

@github-actions github-actions bot temporarily deployed to commit October 18, 2023 08:15 Inactive
@@ -69,6 +70,7 @@ describe("modules / compose / networks", () => {
"sample.dnp.dappnode.eth": {
container_name,
image,
dns: undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor

@pablomendezroyo pablomendezroyo left a comment

Choose a reason for hiding this comment

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

Unit test failing

@pablomendezroyo pablomendezroyo linked an issue Oct 23, 2023 that may be closed by this pull request
@dappnodedev dappnodedev changed the title Remove bind dependency Remove nsupdate module Oct 23, 2023
@github-actions github-actions bot temporarily deployed to commit October 23, 2023 10:45 Inactive
@dappnodedev
Copy link
Contributor Author

Overwritten by #1748

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.

Remove nsUpdate module in favor of native docker dns in dappmanager Remove nsUpdate
2 participants