Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ovirt installer provider #1948

Merged
merged 16 commits into from Jan 13, 2020
Merged

Add ovirt installer provider #1948

merged 16 commits into from Jan 13, 2020

Conversation

rgolangh
Copy link
Contributor

@rgolangh rgolangh commented Jul 8, 2019 •
edited

This work establishes an IPI installation on oVirt platform.

Installation document:
https://github.com/openshift/installer/tree/master/docs/user/ovirt

The enhancement document:
openshift/enhancements#61

The installation uses a similar network infrastructure to baremetal.
https://github.com/openshift/installer/blob/master/docs/design/baremetal/networking-infrastructure.md

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 8, 2019
@openshift-ci-robot
Copy link
Contributor

Hi @rgolangh. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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 openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 8, 2019
abhinavdahiya
abhinavdahiya reviewed Jul 8, 2019
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 10, 2019
@rgolangh rgolangh mentioned this pull request Jul 11, 2019
Merged
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2019
@kikisdeliveryservice kikisdeliveryservice mentioned this pull request Jul 11, 2019
Merged
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 15, 2019
trown pushed a commit to trown/installer that referenced this pull request Jul 16, 2019
openstack: get ignition via IP
b8f39ac 
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 trown mentioned this pull request Jul 16, 2019
Closed
trown pushed a commit to trown/installer that referenced this pull request Jul 16, 2019
openstack: get ignition via IP
f5f4125 
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
tomassedovic pushed a commit to tomassedovic/openshift-installer that referenced this pull request Jul 16, 2019
@tomassedovic
openstack: get ignition via IP
7424ac2 
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
tomassedovic pushed a commit to tomassedovic/openshift-installer that referenced this pull request Jul 17, 2019
@tomassedovic
openstack: get ignition via IP
212a4e0 
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 pushed a commit to trown/installer that referenced this pull request Jul 17, 2019
openstack: get ignition via IP
602bf6f 
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 pushed a commit to trown/installer that referenced this pull request Jul 18, 2019
openstack: get ignition via IP
f2c9cf6 
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 pushed a commit to trown/installer that referenced this pull request Jul 18, 2019
openstack: get ignition via IP
986bbf7 
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 pushed a commit to trown/installer that referenced this pull request Jul 18, 2019
openstack: get ignition via IP
22b71db 
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 pushed a commit to trown/installer that referenced this pull request Jul 18, 2019
openstack: get ignition via IP
1bc6dba 
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 pushed a commit to trown/installer that referenced this pull request Jul 18, 2019
openstack: get ignition via IP
deb3a94 
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
tomassedovic pushed a commit to tomassedovic/openshift-installer that referenced this pull request Jul 18, 2019
@tomassedovic
openstack: get ignition via IP
4579e6c 
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
tomassedovic pushed a commit to tomassedovic/openshift-installer that referenced this pull request Jul 24, 2019
@tomassedovic
openstack: get ignition via IP
5a56a7e 
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
iamemilio pushed a commit to iamemilio/installer that referenced this pull request Jul 26, 2019
openstack: get ignition via IP
503bbc4 
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
mandre pushed a commit to mandre/installer that referenced this pull request Jul 26, 2019
@mandre
openstack: get ignition via IP
9680e78 
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
tomassedovic pushed a commit to tomassedovic/openshift-installer that referenced this pull request Jul 26, 2019
@tomassedovic
openstack: get ignition via IP
e63c1f9 
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
patrickdillon
patrickdillon reviewed Jan 10, 2020
View reviewed changes
pkg/asset/machines/master.go
@@ -407,6 +419,7 @@ func (m *Master) Machines() ([]machineapi.Machine, error) {
gcpapi.AddToScheme(scheme)
libvirtapi.AddToScheme(scheme)
openstackapi.AddToScheme(scheme)
ovirtprovider.AddToScheme(scheme)
decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(
awsprovider.SchemeGroupVersion,
azureprovider.SchemeGroupVersion,
Copy link
Contributor

Choose a reason for hiding this comment

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

probably need to add ovirtprovider.SchemeGroupVersion to decoder like all other platforms

patrickdillon
patrickdillon reviewed Jan 10, 2020
View reviewed changes
pkg/asset/machines/worker.go
@@ -369,6 +386,7 @@ func (w *Worker) MachineSets() ([]machineapi.MachineSet, error) {
gcpapi.AddToScheme(scheme)
libvirtapi.AddToScheme(scheme)
openstackapi.AddToScheme(scheme)
ovirtprovider.AddToScheme(scheme)
decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(
Copy link
Contributor

Choose a reason for hiding this comment

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

same as: https://github.com/openshift/installer/pull/1948/files#r365431334

@patrickdillon
Copy link
Contributor

I made a pair of comments above which should be fixed. They can be fixed here or in follow-up PRs.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 10, 2020
@sdodson
Copy link
Member

sdodson commented Jan 13, 2020

/approve
/test e2e-aws-upgrade

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sdodson

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:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 13, 2020
@openshift-bot
Copy link
Contributor

/retest

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

11 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 37d12b4 into openshift:master Jan 13, 2020
@openshift-ci-robot
Copy link
Contributor

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

Test name Commit Details Rerun command
ci/prow/e2e-libvirt d77c58a link /test e2e-libvirt

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.

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.

@Conan-Kudo
Copy link

🎉

rgolangh added a commit to rgolangh/machine-api-operator that referenced this pull request Jan 15, 2020
@rgolangh
Add support for oVirt platform
d7d1580 
This work is part of making oVirt a supported platform
as mentioned in this the enhancement document:
openshift/enhancements#61

oVirt's cluster-api-providr[1] is added to images.json and
it returned for platform type 'ovirt'.

[1] https://github.com/ovirt/cluster-api-provider-ovirt

Related-to: openshift/installer#1948
Signed-off-by: Roy Golan <rgolan@redhat.com>
@rgolangh rgolangh mentioned this pull request Jan 23, 2020
Merged
rgolangh added a commit to rgolangh/installer that referenced this pull request Feb 3, 2020
@rgolangh
ovirt: Add survey
45910d7 
Depens-on: openshift#1948

Signed-off-by: Roy Golan <rgolan@redhat.com>
@patrickdillon patrickdillon mentioned this pull request Feb 10, 2020
Merged
wking added a commit to wking/openshift-installer that referenced this pull request Apr 13, 2020
@wking
pkg/destroy/ovirt: Cleanup VM logging
ea5ed48 
Fixing some of the logging from 0517783 (ovirt: Implement destroy,
2019-10-28, openshift#1948) to avoid things like:

  $ openshift-install destroy cluster --dir one
  INFO searching VMs by tag=one-6xlb7
  INFO Found %!s(int=3) VMs
  INFO Stopping VM one-6xlb7-master-1 : errors: %s%!(EXTRA <nil>)
  ...

because the final 'err' had no %s in the template string to consume
it.  This commit fixes the two cases which had an 'errors: %s' using
up the final template %s, but leaves a lot of other potential logging
cleanup in this package alone.
@wking wking mentioned this pull request Apr 13, 2020
Merged
wking added a commit to wking/openshift-installer that referenced this pull request Apr 13, 2020
@wking
pkg/destroy/ovirt: Cleanup VM logging
d947a58 
Fixing some of the logging from 0517783 (ovirt: Implement destroy,
2019-10-28, openshift#1948) to avoid things like:

  $ openshift-install destroy cluster --dir one
  INFO searching VMs by tag=one-6xlb7
  INFO Found %!s(int=3) VMs
  INFO Stopping VM one-6xlb7-master-1 : errors: %s%!(EXTRA <nil>)
  ...

because the final 'err' had no %s in the template string to consume
it.  This commit fixes the two cases which had an 'errors: %s' using
up the final template %s, but leaves a lot of other potential logging
cleanup in this package alone.
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/installer that referenced this pull request Apr 17, 2020
@wking
pkg/destroy/ovirt: Cleanup VM logging
e80b6c9 
Fixing some of the logging from 0517783 (ovirt: Implement destroy,
2019-10-28, openshift#1948) to avoid things like:

  $ openshift-install destroy cluster --dir one
  INFO searching VMs by tag=one-6xlb7
  INFO Found %!s(int=3) VMs
  INFO Stopping VM one-6xlb7-master-1 : errors: %s%!(EXTRA <nil>)
  ...

because the final 'err' had no %s in the template string to consume
it.  This commit fixes the two cases which had an 'errors: %s' using
up the final template %s, but leaves a lot of other potential logging
cleanup in this package alone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richm richm richm left review comments

@sdodson sdodson sdodson left review comments

@enxebre enxebre enxebre left review comments

@abhinavdahiya abhinavdahiya abhinavdahiya left review comments

@patrickdillon patrickdillon patrickdillon left review comments

@zeenix zeenix zeenix requested changes

Assignees

@patrickdillon patrickdillon

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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet