Bug 1788583: Node resolver: support IPv6 service IPs #151

ironcladlou · 2019-12-13T18:40:40Z

This is a quick fix to support IPv6 service IPs when processing services
for /etc/hosts on the node. If a service exists but no A record is found,
check for an AAAA record, and if found, use that instead.

The assumption is that no A record will exist in the IPv6 environment,
which seems to hold for now.

This should enable the container runtime to resolve internal image
registry names for image pulls.

This is a quick fix to support IPv6 service IPs when processing services for /etc/hosts on the node. If a service exists but no A record is found, check for an AAAA record, and if found, use that instead. The assumption is that no A record will exist in the IPv6 environment, which seems to hold for now. This should enable the container runtime to resolve internal image registry names for image pulls.

russellb · 2019-12-13T18:43:54Z

seems OK to me

ironcladlou · 2019-12-13T18:44:33Z

In a followup sometime during the release it might be nice to port this function into a subcommand of the Go binary, it's beyond hairy at this point.

ironcladlou · 2019-12-13T18:47:47Z

Result on a node:

sh-4.2# cat /etc/hosts
# Kubernetes-managed hosts file (host network).
127.0.0.1   localhost localhost.localdomain localhost4 localhost4.localdomain4
::1         localhost localhost.localdomain localhost6 localhost6.localdomain6
fd02::d2c8 image-registry.openshift-image-registry.svc image-registry.openshift-image-registry.svc.cluster.local # openshift-generated-node-resolver

Miciah · 2019-12-13T18:58:38Z

assets/dns/daemonset.yaml

@@ -90,10 +90,16 @@ spec:
            declare -A svc_ips
            for svc in "${services[@]}"; do
              # Fetch service IP from cluster dns if present
-              ips=($(dig @"${NAMESERVER}" +short "${svc}.${CLUSTER_DOMAIN}"))
+              ips=($(dig -t A @"${NAMESERVER}" +short "${svc}.${CLUSTER_DOMAIN}"))


How about dig "@$NAMESERVER" +short A "${svc}.${CLUSTER_DOMAIN}" AAAA "${svc}.${CLUSTER_DOMAIN}"?

Although that prints an extra line: ";; Warning, extra type option", which we would need to ignore (or maybe we shouldn't ignore it...).

This should avoid the warning: dig "@$NAMESERVER" +short "${svc}.${CLUSTER_DOMAIN}" AAAA "${svc}.${CLUSTER_DOMAIN}".

Refactor the whole thing to Go in a followup?

Miciah · 2019-12-16T15:48:37Z

/lgtm

openshift-bot · 2019-12-16T17:15:23Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-16T17:53:59Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-16T19:38:24Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-16T19:50:59Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-16T21:35:15Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-16T22:53:14Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-16T23:06:04Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-16T23:19:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T01:16:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T02:46:58Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T04:17:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T06:01:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T06:14:58Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T07:45:31Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T07:58:28Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T09:29:26Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T09:42:26Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T11:00:19Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-17T11:13:22Z

/retest

Please review the full test history for this PR and help us cut down flakes.

ironcladlou · 2019-12-17T11:50:15Z

I think something's up with the patch on AWS — seeing this in the node resolver container logs:

cmp: EOF on /etc/hosts

(That's the entirety of the logs.)

Miciah · 2019-12-17T14:45:24Z

I think something's up with the patch on AWS — seeing this in the node resolver container logs:
cmp: EOF on /etc/hosts
(That's the entirety of the logs.)

That suggests that /etc/hosts was initially empty, the script updated it, and no further changes were needed. How about catting /etc/hosts before the while loop so we can see whether that is the case? It would also make the output clearer to change cmp to diff -u (the exit codes are the same, but diff additionally gives output).

Another possibility is that something else is writing to /etc/hosts.tmp and is racing with the script and sometimes truncating /etc/hosts.tmp between when the script writes to it and when the script copies it over /etc/hosts. This seems unlikely, and catting /etc/hosts on startup would help us to rule that possibility out.

Edit: My last paragraph doesn't make sense. Substitute /etc/hosts for /etc/hosts.tmp (and substitute "when the script copies it to /etc/hosts.tmp" for "when the script writes to it") and it does.

ironcladlou · 2019-12-17T14:46:38Z

Now I think we might be experiencing collateral damage from other CI issues. Need to go over the latest build cop communications, etc.

ironcladlou · 2019-12-17T15:19:02Z

https://search.svc.ci.openshift.org/?search=failed.*proper+ImagePullPolicy&maxAge=24h&context=2&type=all

https://search.svc.ci.openshift.org/?search=failed.*58+characters&maxAge=24h&context=2&type=all

https://search.svc.ci.openshift.org/?search=failed.*Service+Catalog+should+check+basic+usages&maxAge=24h&context=2&type=all

https://search.svc.ci.openshift.org/?search=failed.*capture+build+stages+and+durations&maxAge=24h&context=2&type=all

https://search.svc.ci.openshift.org/?search=failed.*deploymentconfigs+keep+the+deployer+pod+invariant+valid&maxAge=24h&context=2&type=all

I don't think this patch is the cause of the aws-e2e failures.

Miciah · 2019-12-18T22:21:14Z

/lgtm

openshift-ci-robot · 2019-12-18T22:21:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ironcladlou, Miciah

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:

~~OWNERS~~ [Miciah,ironcladlou]

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

openshift-bot · 2019-12-18T22:34:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-18T22:47:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

russellb · 2019-12-19T14:27:29Z

/cherry-pick release-4.3

@ironcladlou FYI

openshift-cherrypick-robot · 2019-12-19T14:27:40Z

@russellb: new pull request created: #152

In response to this:

/cherry-pick release-4.3

@ironcladlou FYI

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-01-07T15:01:50Z

@ironcladlou: Bugzilla bug 1788583 is in an unrecognized state (MODIFIED) and will not be moved to the MODIFIED state.

In response to this:

Bug 1788583: Node resolver: support IPv6 service IPs

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 13, 2019

openshift-ci-robot requested review from frobware and Miciah December 13, 2019 18:40

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2019

ironcladlou force-pushed the ipv6 branch from 73de991 to 8d68ad4 Compare December 13, 2019 18:43

openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 13, 2019

Miciah reviewed Dec 13, 2019

View reviewed changes

openshift-ci-robot assigned Miciah Dec 16, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2019

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 17, 2019

ironcladlou force-pushed the ipv6 branch from 083d4af to f0ee5f1 Compare December 17, 2019 12:13

ironcladlou force-pushed the ipv6 branch from f0ee5f1 to 8d68ad4 Compare December 17, 2019 15:19

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 18, 2019

openshift-merge-robot merged commit dc11895 into openshift:master Dec 19, 2019

openshift-cherrypick-robot mentioned this pull request Dec 19, 2019

[release-4.3] Bug 1788591: Node resolver: support IPv6 service IPs #152

Merged

ironcladlou changed the title ~~Node resolver: support IPv6 service IPs~~ Bug 1788583: Node resolver: support IPv6 service IPs Jan 7, 2020

Bug 1788583: Node resolver: support IPv6 service IPs #151

Bug 1788583: Node resolver: support IPv6 service IPs #151

Conversation

ironcladlou commented Dec 13, 2019

russellb commented Dec 13, 2019

ironcladlou commented Dec 13, 2019

ironcladlou commented Dec 13, 2019

Miciah Dec 13, 2019

Choose a reason for hiding this comment

Miciah Dec 13, 2019

Choose a reason for hiding this comment

Miciah Dec 13, 2019

Choose a reason for hiding this comment

ironcladlou Dec 16, 2019

Choose a reason for hiding this comment

Miciah commented Dec 16, 2019

openshift-bot commented Dec 16, 2019

openshift-bot commented Dec 16, 2019

openshift-bot commented Dec 16, 2019

openshift-bot commented Dec 16, 2019

openshift-bot commented Dec 16, 2019

openshift-bot commented Dec 16, 2019

openshift-bot commented Dec 16, 2019

openshift-bot commented Dec 16, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

openshift-bot commented Dec 17, 2019

ironcladlou commented Dec 17, 2019

Miciah commented Dec 17, 2019 • edited

ironcladlou commented Dec 17, 2019

ironcladlou commented Dec 17, 2019

Miciah commented Dec 18, 2019

openshift-ci-robot commented Dec 18, 2019

openshift-bot commented Dec 18, 2019

openshift-bot commented Dec 18, 2019

russellb commented Dec 19, 2019

openshift-cherrypick-robot commented Dec 19, 2019

openshift-ci-robot commented Jan 7, 2020

Miciah commented Dec 17, 2019 •

edited