Skip to content

Conversation

@mandre
Copy link
Member

@mandre mandre commented Feb 18, 2020

This pulls all the recent changes from BM to OpenStack platform.

#1388: Bug 1790823: [IPI][BAREMETAL] Verify that mdns-publisher starts after hostname is set
#1219: [Baremetal] - update haproxy statistics url
#1256: [Baremetal] Haproxy add support for IPv6 frontend
#1396: baremetal: ipv6, switch to NM dispatcher for DNS VIP prepending
#1395: Bug 1798637: baremetal: IPv6 add dhcp-duid to NetworkManager config
#1419: Bug 1796148: baremetal: Remove .template from path in dhcp-dhclient-conf.yaml
#1436: Bug 1798146: [baremetal] Ipv6 non virtual ip fix
#1444: Bug 1798788: Set Kubelet node IP to non-vip
#1456: Bug 1801662: baremetal: all resolvconf editing to NM dispatcher
#1455: Bug 1790823: [baremetal] Verify that MDNS doesn't advertise localhost

This change verifies that mdns-publisher pod advertises node's
name only after hostname being set.

This ports the BM change from
openshift#1388 to
OpenStack platform.
Update statistics url in Haproxy config to support both
IPv4 and IPv6 (single stack).

This ports BM fix from
openshift#1219 to
OpenStack platform.
This ports the BM fix from
openshift#1256 to
OpenStack platform
The prepend via dhclient doesn't work via ipv6, so switch to
a NetworkManager dispatcher that runs after dhclient instead as
a workaround.

This ports the BM fix from
openshift#1396 to
OpenStack platform.
Adding ipv6.dhcp-duid=ll means we get a predictable client ID so that
reservations can be made on the DHCP server - this is useful if you
want to provide the hostname via DHCP (although a reverse DNS lookup
should also work)

Reservations are also required to ensure the controlplane IP of the
masters won't ever change post-deployment which will be necessary if
the DNS lookup approach is taken to set the hostnames.

This ports the BM fix from
openshift#1395 to
OpenStack platform.
This appears to get written out as /etc/dhcp/dhclient.conf.template
with no subsequent render-config templating to create the actual
/etc/dhcp/dhclient.conf - modifying the path results in the file being
correctly written.

This ports the BM fix from
openshift#1419 to
OpenStack platform.
In IPv6 DHCPv6 environments, the prefix for the IPv6 address that we
must use to bind services may be 128. In case that happens, the current
detection code will fail to consider the address as being in the right
subnet.

Thankfully, thanks to the Route Advertisements, we get the prefix
information we need from the routing table. This patch adds the extra
code to generate pseudo CIDRs that allow us to check if an address is
in the right subnet.

This ports the BM fix from
openshift#1436 to
OpenStack platform.
@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 18, 2020
This change prevents:
* Kubelet choosing as node IP an address from a non control plane subnet
  (like the provisioning network)
* Kubelet choosing a deprecated IPv6 address as its node IP that in
  several platform can be a VIP

This ports the BM change from
openshift#1444 to
OpenStack platform.
Before this patch, the search domain modification was dependant on
dhclient. Unfortunately, the interaction of both components sometimes
fails. This patch makes sure that we preserve what comes from the dhcp
client (whichever may be) but we set what we need in VIP managing
environments.

This ports the BM change
openshift#1456 to
OpenStack platform.
MDNS publisher static pod should start advertising the node
only after node's hostname was set by DHCP.

The idea is to add an init container that will monitor host's hostname
until hostname != localhost.

The issue was how host's hostname should be retrieved by the init container.

Since the MDNS static pod doesn't use host's uts namespace, reading 'hostname'
is not an option.
Reading hostname using dbus didn't work due to security constraints(SElinux).

The suggested solution composed of:
 1. NM dispatcher script will monitor & store hostname in a file
 2. Init container will mount this file, and read the value

This ports the BM change
openshift#1455 to
OpenStack platform.
@mandre mandre force-pushed the openstack-ipv6-fixes-from-bm branch from 92d994b to 304dfd6 Compare February 18, 2020 12:49
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 18, 2020
@mandre
Copy link
Member Author

mandre commented Feb 18, 2020

/test e2e-openstack

1 similar comment
@mandre
Copy link
Member Author

mandre commented Feb 18, 2020

/test e2e-openstack

@kikisdeliveryservice
Copy link
Contributor

/retest

@kikisdeliveryservice
Copy link
Contributor

(scaleup test is broken..)

/skip

@pierreprinetti
Copy link
Member

/test e2e-gcp-upgrade

@vrutkovs
Copy link
Contributor

Please don't create more python scripts, these won't work on OKD. Can it be wrapped in a container?

@mandre
Copy link
Member Author

mandre commented Feb 19, 2020

Please don't create more python scripts, these won't work on OKD. Can it be wrapped in a container?

The nodeip-finder script should indeed be moved to baremetal-runtimecfg image, the same you did for non-virtual-ip in openshift/baremetal-runtimecfg#46

@pierreprinetti
Copy link
Member

/lgtm
/hold

IPv4 deployment successful, feel free to /hold cancel when ready

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 19, 2020
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 19, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mandre, pierreprinetti

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

@mandre
Copy link
Member Author

mandre commented Feb 19, 2020

/hold cancel

Moving nodeip-finder to baremetal-runtimecfg should be done in a separate PR.

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 19, 2020
@mandre
Copy link
Member Author

mandre commented Feb 19, 2020

/retest

@openshift-bot
Copy link
Contributor

/retest

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

@mandre
Copy link
Member Author

mandre commented Feb 19, 2020

/retest

@openshift-bot
Copy link
Contributor

/retest

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

@kikisdeliveryservice
Copy link
Contributor

/test e2e-gcp-upgrade

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Feb 19, 2020

@mandre: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-scaleup-rhel7 304dfd6 link /test e2e-aws-scaleup-rhel7

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

@openshift-bot
Copy link
Contributor

/retest

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

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

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

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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants