Skip to content

Conversation

@tomassedovic
Copy link
Contributor

This brings in changes from openshift/api#374 which add PlatformStatus for BareMetal and OpenStack.

This brings in PlatformStatus for BareMetal and OpenStack.
@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 17, 2019
@tomassedovic
Copy link
Contributor Author

I ran this command: dep ensure -update github.com/openshift/api. It printed out this warning:

Warning: the following project(s) have [[constraint]] stanzas in Gopkg.toml:

  ✗  github.com/blang/semver

However, these projects are not direct dependencies of the current project:
they are not imported in any .go files, nor are they in the 'required' list in
Gopkg.toml. Dep only applies [[constraint]] rules to direct dependencies, so
these rules will have no effect.

Either import/require packages from these projects so that they become direct
dependencies, or convert each [[constraint]] to an [[override]] to enforce rules
on these projects, if they happen to be transitive dependencies.

And it removed github.com/blang/semver from Gopkg.lock. I'm not sure whether that's a problem or not so I'm just highlighting it here. Everything builds fine for me locally.

@runcom
Copy link
Member

runcom commented Jul 17, 2019

And it removed github.com/blang/semver from Gopkg.lock. I'm not sure whether that's a problem or not so I'm just highlighting it here. Everything builds fine for me locally.

probably side effect of #977 which is fine

@runcom
Copy link
Member

runcom commented Jul 17, 2019

/retest
/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 17, 2019
@tomassedovic
Copy link
Contributor Author

/test e2e-openstack

@kikisdeliveryservice
Copy link
Contributor

@runcom is correct, our only semver usage was removed in 977 yesterday

@tomassedovic
Copy link
Contributor Author

/retest

@tomassedovic
Copy link
Contributor Author

@runcom in an accompanying vendor patch in the installer, @vrutkovs said the prow/e2e-aws-upgrade job will keep failing until openshift/release#4279 is merged and that we can /skip it for now:

openshift/installer#2015 (comment)

Are you okay with skipping it here too and reviewing as is?

@runcom
Copy link
Member

runcom commented Jul 18, 2019

@runcom in an accompanying vendor patch in the installer, @vrutkovs said the prow/e2e-aws-upgrade job will keep failing until openshift/release#4279 is merged and that we can /skip it for now:

openshift/installer#2015 (comment)

there's no mention of e2e-aws-upgrade in the PRs you referenced - those are about the scaleup test which isn't in the matrix at all.
Also, we can't just skip e2e-aws-upgrade - and we don't even have power if we wanted to

@runcom
Copy link
Member

runcom commented Jul 18, 2019

/retest

@tomassedovic
Copy link
Contributor Author

Reading comprehension fail, I'm so sorry!

@tomassedovic
Copy link
Contributor Author

/retest

@tomassedovic
Copy link
Contributor Author

@runcom All tests pass, sorry for the confusion yesterday. Is there anything else you'd like to see?

@runcom
Copy link
Member

runcom commented Jul 19, 2019

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: runcom, tomassedovic

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

@openshift-merge-robot openshift-merge-robot merged commit a78a6a4 into openshift:master Jul 19, 2019
mandre pushed a commit to mandre/installer that referenced this pull request Jul 26, 2019
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!
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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants