Skip to content

Wait for either a IPv4 or IPv6 IP address#108

Closed
derekhiggins wants to merge 1 commit intometal3-io:masterfrom
derekhiggins:get-ipv6-ip
Closed

Wait for either a IPv4 or IPv6 IP address#108
derekhiggins wants to merge 1 commit intometal3-io:masterfrom
derekhiggins:get-ipv6-ip

Conversation

@derekhiggins
Copy link
Copy Markdown
Member

Add support for IPv6 IP addfresses on the provisioning
interface. Now greps for any ip address containeing a "." or
beginning in "fd".

@metal3-io-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekhiggins

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

The pull request process is described here

Details 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 approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 18, 2019
Comment thread runhttpd.sh Outdated
PROVISIONING_INTERFACE=${PROVISIONING_INTERFACE:-"provisioning"}
HTTP_PORT=${HTTP_PORT:-"80"}
HTTP_IP=$(ip -4 address show dev "$PROVISIONING_INTERFACE" | grep -oP '(?<=inet\s)\d+(\.\d+){3}' | head -n 1)
HTTP_IP=$(ip -br addr show dev em2 | grep -Po "[^\s]+/[0-9]+" | grep -e "^fd" -e "\." | sed -e 's%/.*%%' | head -n 1)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't we keep "$PROVISIONING_INTERFACE" instead of em2 ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yup, thanks, that was a mistake from where I was testing, will fix

Comment thread runhttpd.sh Outdated
echo "Waiting for ${PROVISIONING_INTERFACE} interface to be configured"
sleep 1
HTTP_IP=$(ip -4 address show dev "$PROVISIONING_INTERFACE" | grep -oP '(?<=inet\s)\d+(\.\d+){3}' | head -n 1)
HTTP_IP=$(ip -br addr show dev em2 | grep -Po "[^\s]+/[0-9]+" | grep -e "^fd" -e "\." | sed -e 's%/.*%%' | head -n 1)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto

Add support for IPv6 IP addfresses on the provisioning
interface. Now greps for any ip address containeing a "." or
beginning in "fd".
Comment thread runhttpd.sh
PROVISIONING_INTERFACE=${PROVISIONING_INTERFACE:-"provisioning"}
HTTP_PORT=${HTTP_PORT:-"80"}
HTTP_IP=$(ip -4 address show dev "$PROVISIONING_INTERFACE" | grep -oP '(?<=inet\s)\d+(\.\d+){3}' | head -n 1)
HTTP_IP=$(ip -br addr show dev $PROVISIONING_INTERFACE | grep -Po "[^\s]+/[0-9]+" | grep -e "^fd" -e "\." | sed -e 's%/.*%%' | head -n 1)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we need to replace the grep -Po "[^\s]+/[0-9]+" because the minikube busybox shell doesn't support the -P option, and also I think that numeric regex is now wrong for the hex ipv6 addresses?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually this doesn't run on the host so ignore the busybox remark, but I think the regex still needs adjustment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we need to replace the grep -Po "[^\s]+/[0-9]+" because the minikube busybox shell doesn't support the -P option,

I can submit an alternative without the -P, is this scripts being used somewhere outside of the container is being built in?

and also I think that numeric regex is now wrong for the hex ipv6 addresses?

I'm not sure what you mean here which regex is wrong?

@hardys hardys referenced this pull request 28 minutes ago
Listen IPv6 addresses for HTTPD #107

Sorry I hadn't seen this had already started in another PR, I can abandon if ye want to continue with the other approach.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah sorry, I misread the regex - it's fine as it's matching string/NN - the reason I hit the -P issue is I tried testing it via copy/paste on busybox, so that's probably not an issue.

Re the other PR there has been some good discussion there so lets see if we can reach conclusion re the approach there then update whichever PR makes sense

Copy link
Copy Markdown
Member

@stbenjam stbenjam left a comment

Choose a reason for hiding this comment

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

If we go with this approach you would need to make sure to have a way to detect its IPv6 so the inspector url is correctly surrounded with brackets as well

@derekhiggins
Copy link
Copy Markdown
Member Author

Killing this in favour of the earlier PR that has had more discussion #107

elfosardo pushed a commit to elfosardo/ironic-image that referenced this pull request Oct 9, 2020
…ift-4.7-ironic

Updating ironic builder & base images to be consistent with ART
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. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants