Skip to content

Conversation

@newkit
Copy link
Member

@newkit newkit commented Dec 24, 2025

Changed order of CNV HyperConverged instance creation and changed webhook endpoint name waiting.

This fixes issue 146 by adding more conditionals that have to be true when waiting for webhook endpoint in order to continue.

@newkit newkit requested a review from marcelmamula December 24, 2025 11:27
@marcelmamula marcelmamula changed the title Fix installation of CNV operator sap_hypervisor_node_preconfigure/kubevirt_vm: Fix installation of CNV operator Jan 5, 2026
@marcelmamula
Copy link
Contributor

@newkit This PR does not fix #146

It just moves webhook wait into task file created in previous PR, it does not change any order.

@newkit
Copy link
Member Author

newkit commented Jan 5, 2026

@newkit This PR does not fix #146

It just moves webhook wait into task file created in previous PR, it does not change any order.

@marcelmamula
Well, it does fix #146 since it adds more conditionals waiting for the webhook endpoint to be ready:

until: - __sap_hypervisor_node_preconfigure_register_webhook_check is defined - __sap_hypervisor_node_preconfigure_register_webhook_check.resources | length > 0 - __sap_hypervisor_node_preconfigure_register_webhook_check.resources[0].subsets is defined - __sap_hypervisor_node_preconfigure_register_webhook_check.resources[0].subsets | length > 0 - __sap_hypervisor_node_preconfigure_register_webhook_check.resources[0].subsets[0].addresses is defined - __sap_hypervisor_node_preconfigure_register_webhook_check.resources[0].subsets[0].addresses | length > 0

vs. before

until: webhook_service.resources | default([]) | length > 0

@marcelmamula
Copy link
Contributor

@newkit Issue you mentioned says order:

With the latest addition of waiting for the CNV webhook endpoint, the order is wrong.
First, we need to create the CNV HyperConverged instance and then wait for the endpoint.

This PR enhances until conditional as you said, but it does not change any order. All I am asking is that you ensure that Issue description is clear enough so this PR can be logically linked to it, because this is what I see now:

  • Issue: Order problem between instance and webhook.
  • PR: Replace task with previously added reusable task file with better conditionals. No changes to order.

@newkit
Copy link
Member Author

newkit commented Jan 7, 2026

@marcelmamula Yes. you are absolutely right, the issue was mentioning the order which I believed would be the root cause when I opened the bug. But it turned out it's the conditionals that weren't fully reflecting the readyness of the webhook endpoint. I've edited the issue so that it's in line now with this PR.

Copy link
Contributor

@marcelmamula marcelmamula left a comment

Choose a reason for hiding this comment

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

LGTM

@newkit newkit merged commit 7d54125 into sap-linuxlab:dev Jan 7, 2026
18 checks passed
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.

2 participants