-
Notifications
You must be signed in to change notification settings - Fork 8
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.
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
sap_vm_provision: Add Red Hat OpenShift Virtualization as target platform #47
sap_vm_provision: Add Red Hat OpenShift Virtualization as target platform #47
Conversation
ff19a1a
to
6e2b861
Compare
Could you please review @0xFelix? |
playbooks/vars/sample-variables-sap-vm-provision-redhat-ocpv.yml
Outdated
Show resolved
Hide resolved
roles/sap_vm_provision/tasks/platform_ansible/redhat_ocp_virt/execute_main.yml
Outdated
Show resolved
Hide resolved
roles/sap_vm_provision/tasks/platform_ansible/redhat_ocp_virt/execute_provision.yml
Outdated
Show resolved
Hide resolved
6e2b861
to
08477e7
Compare
@newkit Is there justification for the code split from
|
@sean-freeman |
@newkit I do not understand the justification:
This code looks 95%+ compatible with any KubeVirt Control Plane for underlying KVM Hypervisor nodes. There is zero requirement to test the codeline with upstream KubeVirt, in the same way as the codeline has not been tested with upstream OVirt or other vendor's control plane products for OVirt (e.g. Oracle Virtualization). |
@dominikholler @fabiand What are your thoughts on this? |
Howdy. The goal of this role was to provide an easy way and robust way to provision a VM on OpenShift. A few issues I'd expect if this role is generalized to KubeVirt:
If this changed towards KubeVirt, then I wonder if this role would still meet the expectations and provides the value (of bein easy, robust, and delivering performance as expected). Due to @newkit 's testing we know that this role works at least nicely with OpenShift. Thus: Sure, as allmost (?) all software, this role could be adjusted to be agnostic. Thus we have to choose between:
Again, just my 2cts. HTH |
Initiative objectives and Ansible Role goalsThe mission of the SAP LinuxLab Open-Source Initiative, is vendor-neutrality and creating a like-for-like outcome across multiple vendors. In the case of the To meet the objective, the goals are therefore to reduce variance as much as possible. So, no @fabiand the goal was not to provision a VM on OpenShift. The goal was to create a VM on every distinct Infrastructure Platform, therein lies the challenge of scope control and what is distinct. The example I provided before with Open Virtualization (OVirt) is accurate. The code itself is for compatibility with OVirt infrastructure platforms, therefore it would likely provision to Oracle Virtualization products with 0 code changes. Anything that is specific to RHEL KVM or legacy RHEV, uses a To provide an example where it went the opposite way, VMs to IBM PowerVM use the IBM PowerVC controller (akin to VMware vCenter) to perform the provisioning; this is actually an OpenStack infrastructure platform. However because the code is so specific with (a lot) of additional code bespoke to IBM PowerVM (and therein the IBM Power HMC) handled through the OpenStack API calls metadata - it is in subdir This PRIn this PR, performing a diff shows 95%+ same code as the existing subdir for
I do not seek to change this towards exclusively KubeVirt, that is not the goal. The goal is to allow any KubeVirt infrastructure platform to use this code, as you describe "agnostic" but I prefer "vendor-neutral"; therein we all benefit from the fixes a common codeline that has "switch execution logic" for the <10% of difference in the code. You'll see many examples of this "switch execution logic" through the rest of the initiative, for example:
|
08477e7
to
09fd4e9
Compare
@sean-freeman The code for redhat_ocpv has replaced the kubevirt_vm code now. |
@sean-freeman thanks for the extensive reply - and sorry for the PTO incurred delay. If the functional, performance, and testing relevance is as you described, then it might indeed to be tied to OCP+V. |
09fd4e9
to
ce8d6cc
Compare
I believe all issues are addressed and resolved, would you please take a look @sean-freeman ? |
After Reviewing the PR it LGTM. The separated code has a justification just as RHEL and SLES related code is required and no Linux generic code. That said, the role itself abstracts generic kubevirt, Rancher or OCP-V and hence gets platform independent. For the future, if SAP certifies kubevirt - if ever - it is likely they will support Rancher and Openshift as products and not an OpenSource generic one and we need to apply dedicated configs for these two. From my point a view this justifies a separate code tree if it cannot be handled with simple variables any more. |
Extended kubevirt_vm specific tasks to handle Red Hat OpenShift Virtualization
ce8d6cc
to
f44ece9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
With this PR the sap_vm_provision role can create VMs that are tuned for running SAP worloads on Red Hat OpenShift Virtualization.