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

Add DNS or GW for use by dnsmasq #389

Merged
merged 1 commit into from
Nov 3, 2022
Merged

Conversation

shibaPuppy
Copy link
Contributor

Depending on your environment, you should be able to use either GW or DNS.

@metal3-io-bot metal3-io-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 14, 2022
Copy link
Member

@elfosardo elfosardo left a comment

Choose a reason for hiding this comment

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

@shibaPuppy
Copy link
Contributor Author

@@ -12,8 +12,19 @@ log-dhcp
dhcp-range={{ env.DHCP_RANGE }}

# Disable default router(s) and DNS over provisioning network
# It can be used when setting DNS or GW variables.
{%- if env["GW_IP"] is undefined %}
Copy link
Member

Choose a reason for hiding this comment

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

Please don't shorten words: GATEWAY_IP

@@ -12,8 +12,19 @@ log-dhcp
dhcp-range={{ env.DHCP_RANGE }}

# Disable default router(s) and DNS over provisioning network
Copy link
Member

Choose a reason for hiding this comment

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

This comment should be moved

Copy link
Member

Choose a reason for hiding this comment

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

... or even removed completely since you have another comment just below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove that comment.

@dtantsur
Copy link
Member

In addition to the user guide, let's also update the README in this repo?

@dtantsur
Copy link
Member

I'd be also interested in being able to use DNS_IP=IRONIC_IP, but I can look into that in a follow-up.

@dtantsur
Copy link
Member

Something like

diff --git a/scripts/rundnsmasq b/scripts/rundnsmasq
index 7a4fa45..09ce32b 100755
--- a/scripts/rundnsmasq
+++ b/scripts/rundnsmasq
@@ -8,6 +8,10 @@ export DNS_PORT=${DNS_PORT:-0}
 
 wait_for_interface_or_ip
 
+if [[ "${DNS_IP:-}" == "provisioning" ]]; then
+    export DNS_IP=$IRONIC_IP
+fi
+
 mkdir -p /shared/tftpboot
 mkdir -p /shared/tftpboot/arm64-efi
 mkdir -p /shared/html/images

@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 22, 2022
Comment on lines +10 to +13
if [[ "${DNS_IP:-}" == "provisioning" ]]; then
export DNS_IP=$IRONIC_IP
fi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dtantsur
added it, but I don't know if it can act as an 'ironic' dns.

@dtantsur
Copy link
Member

/approve
/test-ubuntu-integration-main

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtantsur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2022
@dtantsur
Copy link
Member

/test-ubuntu-integration-main

I have a feeling that the CI is broken

@shibaPuppy
Copy link
Contributor Author

@dtantsur
is it my broken?

@shibaPuppy
Copy link
Contributor Author

/test-ubuntu-integration-main

@dtantsur
Copy link
Member

/test-ubuntu-integration-main

No, it's an upstream Ironic breakage. A fix has been merged, we need to wait until it gets into repositories.

@dtantsur
Copy link
Member

/test-ubuntu-integration-main

The last attempt before I merge a CI workaround.

@shibaPuppy
Copy link
Contributor Author

@dtantsur
thx,
may i ask review below?
metal3-io/baremetal-operator#1167 (comment)

@elfosardo
Copy link
Member

/test-centos-integration-main

@elfosardo
Copy link
Member

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2022
@metal3-io-bot metal3-io-bot merged commit c8b98fe into metal3-io:main Nov 3, 2022
elfosardo pushed a commit to elfosardo/ironic-image that referenced this pull request Aug 29, 2023
OCPBUGS-17472: Expand regex for fcos/okd packages list
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants