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

chore: resolving DNS IP and publishing it when no extIp provided #2030

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

gabrielmer
Copy link
Contributor

@gabrielmer gabrielmer commented Sep 13, 2023

Description

When running a node and no external IP is provided but a DNS4 address is, we want the ENR to use the resolved DNS IP instead of using 0.0.0.0, which is currently the case.

Changes

  • If there's no external IP but a DNS address was provided, do a DNS IP lookup and use the result as external IP in the node's network configuration
  • Updated procedure newTestWakuNode to account for the above scenario
  • Added test cases for the success and failure flows of the DNS lookup

Issue

closes #1576

@github-actions
Copy link

github-actions bot commented Sep 13, 2023

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2030

Built from 13732cd

@gabrielmer gabrielmer self-assigned this Sep 15, 2023
@gabrielmer gabrielmer force-pushed the fixing-enr-ip-with-dns-enabled branch from 70fdb01 to 2b225c6 Compare September 19, 2023 12:05
tests/test_wakunode.nim Outdated Show resolved Hide resolved
tests/test_wakunode.nim Outdated Show resolved Hide resolved
Copy link
Contributor

@SionoiS SionoiS left a comment

Choose a reason for hiding this comment

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

LGTM!

tests/wakunode2/test_app.nim Show resolved Hide resolved
@gabrielmer gabrielmer marked this pull request as ready for review September 26, 2023 09:39
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

Approving to not be a bottleneck here and the general idea makes sense. The important thing is to get approving reviews from the reviewers that left some comments by re-requesting reviews once you've addressed their concerns. You could also mark specific comments as "resolved" as you go along.

@gabrielmer gabrielmer requested a review from vpavlin September 27, 2023 11:03
@gabrielmer gabrielmer force-pushed the fixing-enr-ip-with-dns-enabled branch from 4083a2a to cb7e1c3 Compare September 27, 2023 12:29
@gabrielmer gabrielmer merged commit 7797b2c into master Sep 27, 2023
9 of 10 checks passed
@gabrielmer gabrielmer deleted the fixing-enr-ip-with-dns-enabled branch September 27, 2023 13:02
ABresting added a commit that referenced this pull request Sep 30, 2023
* chore: add retention policy with GB or MB limitation #1885

* chore: add retention policy with GB or MB limitation

* chore: updated code post review- retention policy

* ci: extract discordNotify to separate file

Signed-off-by: Jakub Sokołowski <jakub@status.im>

* ci: push images to new wakuorg/nwaku repo

Signed-off-by: Jakub Sokołowski <jakub@status.im>

* ci: enforce default Docker image tags strictly

Signed-off-by: Jakub Sokołowski <jakub@status.im>

* ci: push GIT_REF if it looks like a version

Signed-off-by: Jakub Sokołowski <jakub@status.im>

* fix: update wakuv2 fleet DNS discovery enrtree

https://github.com/status-im/infra-misc/issues/171

* chore: resolving DNS IP and publishing it when no extIp is provided (#2030)

* feat(coverage): Add simple coverage (#2067)

* Add test aggregator to all directories.
* Implement coverage script.

* fix(ci): fix name of discord notify method

Also use absolute path to load Groovy script.

Signed-off-by: Jakub Sokołowski <jakub@status.im>

* chore(networkmonitor): refactor setConnectedPeersMetrics, make it partially concurrent, add version (#2080)

* chore(networkmonitor): refactor setConnectedPeersMetrics, make it partially concurrent, add version

* add more metrics, refactor how most metrics are calculated

* rework metrics table fillup

* reset connErr to make sure we honour successful reconnection

* chore(cbindings): Adding cpp example that integrates the 'libwaku' (#2079)

* Adding cpp example that integrates the `libwaku`

---------

Co-authored-by: NagyZoltanPeter <113987313+NagyZoltanPeter@users.noreply.github.com>

* fix(ci): update the dependency list in pre-release WF (#2088)

* chore: adding NetConfig test suite (#2091)

---------

Signed-off-by: Jakub Sokołowski <jakub@status.im>
Co-authored-by: Jakub Sokołowski <jakub@status.im>
Co-authored-by: Anton Iakimov <yakimant@gmail.com>
Co-authored-by: gabrielmer <101006718+gabrielmer@users.noreply.github.com>
Co-authored-by: Álex Cabeza Romero <alex93cabeza@gmail.com>
Co-authored-by: Vaclav Pavlin <vaclav@status.im>
Co-authored-by: Ivan Folgueira Bande <128452529+Ivansete-status@users.noreply.github.com>
Co-authored-by: NagyZoltanPeter <113987313+NagyZoltanPeter@users.noreply.github.com>
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.

chore: update resolved enr ip when using dns4-domain-name flag
4 participants