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 platform type #314

Merged
merged 1 commit into from
Jul 2, 2019
Merged

Add Ovirt platform type #314

merged 1 commit into from
Jul 2, 2019

Conversation

rgolangh
Copy link
Contributor

@rgolangh rgolangh commented May 14, 2019 •
edited
Loading

This will be the start of adding oVirt as a platform provider for openshift.
All work in progress is focusing IPI, and is relying on metal-3 work to self contain dns and load balancing. WIP can be found here - https://github.com/rgolangh/installer

Along with this we also need:

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 14, 2019
@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 May 14, 2019
@openshift-ci-robot
Copy link

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.

@cgwalters cgwalters mentioned this pull request May 16, 2019
Merged
@cgwalters
Copy link
Member

Worth adding some high level details here of the platform.

Are there any plans to implement machine API for oVirt? Or will this feel more like a baremetal-in-virt platform, i.e. node provisioning is admin controlled?

Do you have any work on an RHCOS image? How does Ignition get userdata?

wking
wking reviewed May 16, 2019
View reviewed changes
config/v1/types_infrastructure.go Outdated
@@ -91,6 +91,9 @@ const (

// VSpherePlatformType represents VMWare vSphere infrastructure.
VSpherePlatformType PlatformType = "VSphere"

// OvirtPlatformType represent oVirt/RHV infrastructure.
OvirtPlatformType PlatformType = "Ovirt"
Copy link
Member

Choose a reason for hiding this comment

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

OVirtPlatformType PlatformType = "oVirt"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Officially yes, I wasn't sure the other platforms care about that. I hope that when we compare platform we ignore cases. This would be very unfortunate to fall over casing.
Anyway I'll change that.

Copy link
Member

@wking wking May 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I hope that when we compare platform we ignore cases.

Most code I've seen uses switch or equality checks without Lower wrapping. If we expect folks to use case-insensitive comparison (I'm fine either way), we'll probably want to land strong language about it here. And also find a way to test non-canonical casing vs. our e2e suite.

@rgolangh
Copy link
Contributor Author

ping?

@kikisdeliveryservice kikisdeliveryservice mentioned this pull request Jun 3, 2019
Closed
@abhinavdahiya
Copy link
Contributor

/ok-to-test

@openshift-ci-robot openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 3, 2019
@abhinavdahiya
Copy link
Contributor

/lgtm

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

ping

@cgwalters
Copy link
Member

/approve

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

deads2k commented Jul 1, 2019

reapplying from @abhinavdahiya

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 1, 2019
@openshift-bot
Copy link

/retest

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

2 similar comments
@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

20 similar comments
@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2019
@rgolangh
Add Ovirt platform type
89195ee 
Signed-off-by: Roy Golan <rgolan@redhat.com>
@deads2k
Copy link
Contributor

deads2k commented Jul 2, 2019

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, cgwalters, deads2k, rgolangh

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-merge-robot openshift-merge-robot merged commit a2298de into openshift:master Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deads2k deads2k deads2k left review comments

@wking wking wking left review comments

@abhinavdahiya abhinavdahiya Awaiting requested review from abhinavdahiya

@adambkaplan adambkaplan Awaiting requested review from adambkaplan

Assignees

@abhinavdahiya abhinavdahiya

@deads2k deads2k

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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@rgolangh @openshift-ci-robot @cgwalters @abhinavdahiya @deads2k @openshift-bot @wking @openshift-merge-robot