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 bind as DNS server from compose files #1646

Merged
merged 11 commits into from
Oct 24, 2023

Conversation

dappnodedev
Copy link
Contributor

This is the first step to take in the dappmanager towards current Bind functionality removal

@dappnodedev dappnodedev requested a review from a team as a code owner October 18, 2023 15:03
@github-actions
Copy link

github-actions bot commented Oct 18, 2023

@github-actions
Copy link

github-actions bot commented Oct 18, 2023

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

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

/ipfs/QmW63EKnYELjMzDbh7PWrbYzF3j3uNmT1sYgGqNjobmKg7

(by dappnodebot/build-action)

@pablomendezroyo
Copy link
Contributor

Is it documented somewhere the work of stopping from adding the bind to the compose file?

@dappnodedev
Copy link
Contributor Author

Testing needs to be added here before merging

@github-actions github-actions bot temporarily deployed to commit October 23, 2023 16:16 Inactive
@github-actions github-actions bot temporarily deployed to commit October 23, 2023 18:47 Inactive
const composeService = services[serviceName].get();
if (composeService.dns) {
logs.info(`Removing DNS from ${serviceName} in ${dnpName} compose file`);
composeService.dns = undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this instructions completely removes the dns field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! This will remove the dns field from every docker-compose.yml file that still includes it

composeAlreadyMigrated
);
// Redeclare global variables
params.DNCORE_DIR = composeDnsRemovalTestPath;
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not have to do this, the env TEST=true should change the dirs already allowing you to test propertly

@github-actions github-actions bot temporarily deployed to commit October 24, 2023 08:45 Inactive
@github-actions github-actions bot temporarily deployed to commit October 24, 2023 09:25 Inactive
@github-actions github-actions bot temporarily deployed to commit October 24, 2023 09:32 Inactive
@github-actions github-actions bot temporarily deployed to commit October 24, 2023 09:39 Inactive
@github-actions github-actions bot temporarily deployed to commit October 24, 2023 11:57 Inactive
@github-actions github-actions bot temporarily deployed to commit October 24, 2023 12:09 Inactive
aliases:
- ${serviceName}.test-migration.dappnode
- ${serviceName}.dappnode`;

const composeAlreadyMigrated = `
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that this file has alias migration and dns migration tests, I would rename:
composeAlreadyMigrated to composeAliasesToBeMigrated
composeToBeMigratedBefore to composeAliasesAlreadyMigrated

Or something like that, so we know that these are used for alias migration and not DNS

});

it("Should remove DNS from compose file", async () => {
removeDnsFromPackageComposeFile(container.dnpName, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we test also the removeDnsFromPackageComposeFile for core packages or is not worth it?

@github-actions github-actions bot temporarily deployed to commit October 24, 2023 14:16 Inactive
@pablomendezroyo pablomendezroyo merged commit a7e690d into develop Oct 24, 2023
5 of 6 checks passed
@pablomendezroyo pablomendezroyo deleted the dappnodedev/migrate-dns-compose branch October 24, 2023 15:27
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 dns from existing compose files and from the pkg installer
3 participants