Skip to content

Add Kubevirt provider #716

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
Merged
openshift-merge-robot merged 4 commits into from
Nov 24, 2020
Merged

Add Kubevirt provider #716

openshift-merge-robot merged 4 commits into from
Nov 24, 2020

Conversation

@nirarg
Copy link
Contributor

@nirarg nirarg commented Oct 1, 2020 •
edited
Loading

This PR is part of task done to add KubeVirt platform as infra provider for Openshift.

The change introduced in this PR purpose is to provide a way to install Openshift on KubeVirt infrastructure

For more information, please find the enhancement PR:
openshift/enhancements#417

Related PRs/changes done as part of this task:

  1. Created openshift/cluster-api-provider-kubevirt repository, hosts an implementation of a provider for KubeVirt for the OpenShift machine-api: https://github.com/openshift/cluster-api-provider-kubevirt
    Also adding this project's image to openshift is in progress
  2. openshift/installer: Add KubeVirt platform as infrastructure for Openshift installation installer#4350
  3. openshift/cloud-credential-operator: Add kubevirt platform cloud-credential-operator#260
  4. openshift/machine-config-operator: Add kubevirt platform machine-config-operator#2098
  5. openshift/api: Add KubeVirt platform type api#734

Additional job is done to have tests and CI coverage (work in progress in github.com/openshift/release repository)

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 1, 2020
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 1, 2020

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

Test name Commit Details Rerun command
ci/prow/e2e-gcp d97e695 link /test e2e-gcp
ci/prow/e2e-metal-ipi d97e695 link /test e2e-metal-ipi
ci/prow/e2e-gcp-operator d97e695 link /test e2e-gcp-operator
ci/prow/e2e-aws-workers-rhel7 d97e695 link /test e2e-aws-workers-rhel7
ci/prow/e2e-aws d97e695 link /test e2e-aws
ci/prow/images d97e695 link /test images
ci/prow/e2e-azure-operator d97e695 link /test e2e-azure-operator
ci/prow/e2e-aws-upgrade d97e695 link /test e2e-aws-upgrade
ci/prow/e2e-azure d97e695 link /test e2e-azure
ci/prow/e2e-aws-operator d97e695 link /test e2e-aws-operator

Full PR test history. Your PR dashboard.

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-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2020
@rgolangh
Copy link
Contributor

rgolangh commented Oct 4, 2020 •
edited
Loading

CI images creation depends on openshift/release#12318 (hopefully merged soon)
Need to add the ticket for origin image as well

@nirarg nirarg force-pushed the kubevirt-provider branch 2 times, most recently from 7672ef2 to 8cf2f1a Compare November 5, 2020 09:50
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 5, 2020
chenyosef
chenyosef reviewed Nov 5, 2020
View reviewed changes
install/0000_30_machine-api-operator_00_credentials-request.yaml Outdated
spec:
secretRef:
namespace: openshift-machine-api
# This is the of the new secret
Copy link

@chenyosef chenyosef Nov 5, 2020

Choose a reason for hiding this comment

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

Should be "This is the name of the new secret"

Copy link
Contributor Author

@nirarg nirarg Nov 5, 2020

Choose a reason for hiding this comment

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

I removed the comment

@nirarg nirarg mentioned this pull request Nov 5, 2020
Merged
@nirarg nirarg force-pushed the kubevirt-provider branch from 8cf2f1a to cb0d32e Compare November 5, 2020 19:22
@nirarg
Copy link
Contributor Author

nirarg commented Nov 8, 2020

/retest

@JoelSpeed
Copy link
Contributor

JoelSpeed commented Nov 9, 2020

Changes here look fine, please add a description to the PR before we merge

We have a script that pushes updates to MAO out to repositories that depend on this repo, you may want to add kubevirt to this list

machine-api-operator/hack/push-updates.sh

Lines 45 to 51 in 1842a55

DEFAULT_REPOS="
cluster-api-provider-aws
cluster-api-provider-azure
cluster-api-provider-baremetal
cluster-api-provider-gcp
cluster-api-provider-openstack
"

@nirarg
Copy link
Contributor Author

nirarg commented Nov 11, 2020

Hi @JoelSpeed, thank you for the review
I added the description and also added kubevirt to the push-updates script
I will leave the [wip] flag until cluster-api-provider-kubevirt image would be added to openshift release

@JoelSpeed
Copy link
Contributor

JoelSpeed commented Nov 11, 2020

@nirarg Great, please ping me once that's done and you're ready to look at getting this merged, I don't think I have any further feedback at this time

@nirarg
Copy link
Contributor Author

nirarg commented Nov 23, 2020

/retest

1 similar comment
@nirarg
Copy link
Contributor Author

nirarg commented Nov 24, 2020

/retest

@nirarg nirarg changed the title [WIP] Add Kubevirt provider Add Kubevirt provider Nov 24, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 24, 2020
@nirarg
Copy link
Contributor Author

nirarg commented Nov 24, 2020

Hi @JoelSpeed , I removed the WIP tag, can you please review this again?

@JoelSpeed
Copy link
Contributor

JoelSpeed commented Nov 24, 2020

/approve

One question I do have though, seems we are using Kubevirt as the camel case capitalisation, is that correct? I thought it was KubeVirt with a capital V for the product name? Is kubeVirt not allowed? (Asking because we use VSphere and vSphere within the codebase because of the product name)

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 24, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed

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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 24, 2020
@nirarg
Copy link
Contributor Author

nirarg commented Nov 24, 2020

Hi @JoelSpeed
Thank you for the review/approve
I use KubeVirt when it is standalone parameter (defined in openshift/api repository)
I used Kubevirt when it part of longer parameter name (like KubevirtPlatformType), which there I followed the camel case capitalisation.

@chenyosef
Copy link

chenyosef commented Nov 24, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 24, 2020
@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 24, 2020

/retest

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

2 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 24, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 24, 2020

/retest

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

@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Nov 24, 2020 •
edited
Loading

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

Test name Commit Details Rerun command
ci/prow/e2e-libvirt 5f2c63c link /test e2e-libvirt
ci/prow/e2e-azure 5f2c63c link /test e2e-azure

Full PR test history. Your PR dashboard.

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-merge-robot openshift-merge-robot merged commit 0bb3304 into openshift:master Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexander-demicev alexander-demicev Awaiting requested review from alexander-demicev

@Danil-Grigorev Danil-Grigorev Awaiting requested review from Danil-Grigorev

1 more reviewer

@chenyosef chenyosef chenyosef left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@chenyosef chenyosef

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@nirarg @openshift-ci-robot @rgolangh @JoelSpeed @chenyosef @openshift-bot @openshift-merge-robot @bardielle