openstack: get ignition via IP #1998
Conversation
openshift-ci-robot
added
the
size/L
pkg/types/openstack/platform.go
Outdated
There was a problem hiding this comment.
FYI the values that the (merged) Baremetal PlatfomStatus support uses APIServerInternalIP and NodeDNSIP names:
https://github.com/openshift/api/pull/348/files
And so does the current (unmerged) OpenStack equivalent: openshift/api#374
(not sure how much sense does it make for these to align)
There was a problem hiding this comment.
Since I needed to rebase this on what the baremetal platform landed I named the vars similar to theirs.
pkg/asset/ignition/machine/node.go
Outdated
There was a problem hiding this comment.
baremetal needs to do something similar - seen https://github.com/openshift/installer/pull/1873/files#diff-d7037682a68150f4dbbbf5cc0d6e8f54R17
There was a problem hiding this comment.
Yes, that PR is linked from the commit message here (as is the oVirt's equivalent: #1948).
The intention here is to merge this smallish patch so that all three platforms (baremetal, ovirt, openstack) can validate this is an acceptable solution and that they can build on.
pkg/asset/tls/mcscertkey.go
Outdated
There was a problem hiding this comment.
baremetal does similar here too
|
/retest |
pkg/asset/tls/mcscertkey.go
Outdated
There was a problem hiding this comment.
this should not be calculated when these fields exist in InstallConfig.
f5f4125#diff-26eeddc74b12e6afb9e2932066a9b49fR33
|
based on f5f4125#diff-d7037682a68150f4dbbbf5cc0d6e8f54R21 please move the PR to WIP... |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
trown
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
size/M
trown
force-pushed
the
ignition-via-ip
branch
3 times, most recently
from
22b71db to
1bc6dba
Compare
openshift-ci-robot
removed
the
size/M
For platforms without a DNS as a Service available, there is a chicken/egg scenario. This is because we cannot setup our own DNS solution on the nodes before they are booted via ignition. Instead, we get the ignition config via a virtual IP that is conifgured and maintained by keepalived. The setup of keepalived will be done via machine-config-operator, so it is not part of this patch. This patch only sets up the plumbing so that we can merge the keepalived work while keeping CI green. Then we can put up a patch that will switch to actually using this functionality. This patch also adds an interface for other platforms that need this functionality to work from. Specifically, these PRs can be rebased on this work: openshift#1873 openshift#1948
trown
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
@trown I did a rebase and fixed the conflict here: |
openshift-ci-robot
added
the
needs-rebase
openshift-ci-robot
removed
the
needs-rebase
|
/retest |
1 similar comment
|
/retest |
|
/lgtm From the OpenStack side, this works great and we need it. Bare Metal broke the ground with this approach so I'm all in favour of merging it. As it touches files not owned by OpenStack, it needs someone else to approve though. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tomassedovic, trown The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
The experimental OpenStack backend used to create an extra server running DNS and load balancer services that the cluster needed. OpenStack does not always come with DNSaaS or LBaaS so we had to provide the functionality the OpenShift cluster depends on (e.g. the etcd SRV records, the api-int records & load balancing, etc.). This approach is undesirable for two reasons: first, it adds an extra node that the other IPI platforms do not need. Second, this node is a single point of failure. The Baremetal platform has faced the same issues and they have solved them with a few virtual IP addresses managed by keepalived in combination with coredns static pod running on every node using the mDNS protocol to update records as new nodes are added or removed and a similar static pod haproxy to load balance the control plane internally. The VIPs are defined here in the installer and they use the PlatformStatus field to be passed to the necessary machine-config-operator fields: openshift/api#374 The Bare Metal IPI Networking Infrastructure document is broadly applicable here as well: https://github.com/openshift/installer/blob/master/docs/design/baremetal/networking-infrastructure.md Notable differences in OpenStack: * We only use the API and DNS VIPs right now * Instead of Baremetal's Ingress VIP (which is attached to the OpenShift routers) our haproxy static pods balance the 80 & 443 pods to the worker nodes * We do not run coredns on the bootstrap node. Instead, bootstrap itself uses one of the masters for DNS. These differences are not fundamental to OpenStack and we will be looking at aligning more closely with the Baremetal provider in the future. There is also a great oportunity to share some of the configuration files and scripts here. This change needs several other pull requests: Keepalived plus the coredns & haproxy static pods in the MCO: openshift/machine-config-operator/pull/740 Passing the API and DNS VIPs through the installer: openshift#1998 Vendoring the OpenStack PlatformStatus changes in the MCO: openshift/machine-config-operator#978 Allowing to use PlatformStatus in the MCO templates: openshift/machine-config-operator#943 Co-authored-by: Emilio Garcia <[email protected]> Co-authored-by: John Trowbridge <[email protected]> Co-authored-by: Martin Andre <[email protected]> Co-authored-by: Tomas Sedovic <[email protected]> Massive thanks to the Bare Metal and oVirt people!
|
/retest |
The experimental OpenStack backend used to create an extra server running DNS and load balancer services that the cluster needed. OpenStack does not always come with DNSaaS or LBaaS so we had to provide the functionality the OpenShift cluster depends on (e.g. the etcd SRV records, the api-int records & load balancing, etc.). This approach is undesirable for two reasons: first, it adds an extra node that the other IPI platforms do not need. Second, this node is a single point of failure. The Baremetal platform has faced the same issues and they have solved them with a few virtual IP addresses managed by keepalived in combination with coredns static pod running on every node using the mDNS protocol to update records as new nodes are added or removed and a similar static pod haproxy to load balance the control plane internally. The VIPs are defined here in the installer and they use the PlatformStatus field to be passed to the necessary machine-config-operator fields: openshift/api#374 The Bare Metal IPI Networking Infrastructure document is applicable here as well: https://github.com/openshift/installer/blob/master/docs/design/baremetal/networking-infrastructure.md There is also a great opportunity to share some of the configuration files and scripts here. This change needs several other pull requests: Keepalived plus the coredns & haproxy static pods in the MCO: openshift/machine-config-operator#740 Passing the API and DNS VIPs through the installer: openshift#1998 Co-authored-by: Emilio Garcia <[email protected]> Co-authored-by: John Trowbridge <[email protected]> Co-authored-by: Martin Andre <[email protected]> Co-authored-by: Tomas Sedovic <[email protected]> Massive thanks to the Bare Metal and oVirt people!
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/test e2e-openstack |
|
/retest |
|
@trown: The following tests failed, say
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. DetailsInstructions 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. |
The experimental OpenStack backend used to create an extra server running DNS and load balancer services that the cluster needed. OpenStack does not always come with DNSaaS or LBaaS so we had to provide the functionality the OpenShift cluster depends on (e.g. the etcd SRV records, the api-int records & load balancing, etc.). This approach is undesirable for two reasons: first, it adds an extra node that the other IPI platforms do not need. Second, this node is a single point of failure. The Baremetal platform has faced the same issues and they have solved them with a few virtual IP addresses managed by keepalived in combination with coredns static pod running on every node using the mDNS protocol to update records as new nodes are added or removed and a similar static pod haproxy to load balance the control plane internally. The VIPs are defined here in the installer and they use the PlatformStatus field to be passed to the necessary machine-config-operator fields: openshift/api#374 The Bare Metal IPI Networking Infrastructure document is applicable here as well: https://github.com/openshift/installer/blob/master/docs/design/baremetal/networking-infrastructure.md There is also a great opportunity to share some of the configuration files and scripts here. This change needs several other pull requests: Keepalived plus the coredns & haproxy static pods in the MCO: openshift/machine-config-operator#740 Passing the API and DNS VIPs through the installer: openshift#1998 Co-authored-by: Emilio Garcia <[email protected]> Co-authored-by: John Trowbridge <[email protected]> Co-authored-by: Martin Andre <[email protected]> Co-authored-by: Tomas Sedovic <[email protected]> Massive thanks to the Bare Metal and oVirt people!
|
This change is now out of date and it is carried in #1959 which we expected to merge soon. I'm going to close this PR but its spirit will live on! Thanks, John! /close |
|
@tomassedovic: Closed this PR. DetailsIn response to this:
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. |
6 participants
For platforms without a DNS as a Service available, there is a
chicken/egg scenario. This is because we cannot setup our own DNS
solution on the nodes before they are booted via ignition.
Instead, we get the ignition config via a virtual IP that is
conifgured and maintained by keepalived. The setup of keepalived
will be done via machine-config-operator, so it is not part of
this patch.
This patch only sets up the plumbing so that we can merge the
keepalived work while keeping CI green. Then we can put up a patch
that will switch to actually using this functionality.
This patch also adds an interface for other platforms that need this
functionality to work from. Specifically, these PRs can be rebased
on this work:
#1873
#1948