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.

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_hypervisor_node_preconfigure: replace use of oc with kubernetes.core.k8s #7

Closed
wants to merge 2 commits into from

Conversation

geetikakay
Copy link
Contributor

@geetikakay geetikakay commented Apr 9, 2024

roles/sap_hypervisor_node_preconfigure/tasks/platform/redhat_ocp_virt/main.yml

  • Check and fail is added if any node is not available

roles/sap_hypervisor_node_preconfigure/tasks/platform/redhat_ocp_virt/configure-worker-node.yml

  • Check and fail is added if any node is not available
  • Change oc label node to use kubernetes.core

@sean-freeman
Copy link
Member

@geetikakay Agree with oc command removal, this should be agonstic for KubeVirt provisioning.

I do not have access to a KubeVirt instance, 60mins seems sensible because hardware can vary and take a long time for provisioning of large VMs (e.g. 6TiB DRAM) for SAP HANA.

@sean-freeman
Copy link
Member

@newkit Can you please review and test this PR ?

At the same time, can you please confirm that the latest kubevirt.core.kubevirt_vm Ansible Module has solved the int parsing issues and does not require workaround usage of ANSIBLE_JINJA2_NATIVE env var?

@sean-freeman sean-freeman requested a review from newkit April 10, 2024 17:14
@sean-freeman sean-freeman changed the title Replace use of oc with kubernetes.core.k8s sap_hypervisor_node_preconfigure: replace use of oc with kubernetes.core.k8s Apr 18, 2024
@sean-freeman sean-freeman changed the base branch from main to dev April 18, 2024 11:10
@geetikakay geetikakay force-pushed the main branch 2 times, most recently from 3af85c0 to 915b45b Compare April 25, 2024 11:20
@geetikakay
Copy link
Contributor Author

geetikakay commented Apr 25, 2024

@geetikakay Agree with oc command removal, this should be agonstic for KubeVirt provisioning.

I do not have access to a KubeVirt instance, 60mins seems sensible because hardware can vary and take a long time for provisioning of large VMs (e.g. 6TiB DRAM) for SAP HANA.

@sean-freeman Done, I have increased the retries.

Copy link
Contributor

@newkit newkit left a comment

Choose a reason for hiding this comment

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

Finished review

release: merge dev to main for 1.0.1
@sean-freeman sean-freeman marked this pull request as draft May 7, 2024 16:15
roles/sap_hypervisor_node_preconfigure/tasks/platform/redhat_ocp_virt/main.yml
- Check and fail is added if any node is not available

roles/sap_hypervisor_node_preconfigure/tasks/platform/redhat_ocp_virt/configure-worker-node.yml
- Check and fail is added if any node is not available
- Change oc label node to use kubernetes.core

Signed-off-by: Geetika Kapoor <[email protected]>
@geetikakay
Copy link
Contributor Author

Finished review

@newkit Thanks , I did requested changes. Please have a look.

@newkit
Copy link
Contributor

newkit commented Jun 18, 2024

@newkit Can you please review and test this PR ?

Reviewed and tested

At the same time, can you please confirm that the latest kubevirt.core.kubevirt_vm Ansible Module has solved the int parsing issues and does not require workaround usage of ANSIBLE_JINJA2_NATIVE env var?

We are still trying to fix the int issue but it's a tough nut. So for now, the ANSIBLE_JINJA2_NATIVE=true workaround is still needed when running the redhat_ocp_virt flavor of the sap_vm_provision role.

@newkit
Copy link
Contributor

newkit commented Jun 26, 2024

This PR became obsolete since there is a better version of this fix in this PR.

@newkit newkit closed this Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants