OCPBUGS-65950: Update template whenever settings are updated#16143
OCPBUGS-65950: Update template whenever settings are updated#16143celdrake wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
jira/valid-reference
|
@celdrake: This pull request references Jira Issue OCPBUGS-65950, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (dasulin@redhat.com), skipping review request. The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: celdrake The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@celdrake: This pull request references Jira Issue OCPBUGS-65950, which is valid. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (dasulin@redhat.com), skipping review request. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis pull request extends the VSphere infrastructure configuration by introducing template path normalization and schema updates. The 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/packages/vsphere-plugin/src/components/persist.ts (1)
514-518: Consider extracting to a helper function for consistency (optional).For symmetry with
getInfrastructureResourcePoolPath(lines 465-481), you could extract this to something likegetInfrastructureTemplatePath. Not strictly necessary given the simpler logic here, but it would improve consistency and testability.♻️ Optional: Extract to helper function
+const getInfrastructureTemplatePath = ( + datacenter: string, + originalTemplate: string | undefined, +): string | undefined => { + const extraPartMatch = (originalTemplate || '').match(/\/vm\/(.+)$/); + return extraPartMatch ? `/${datacenter}/vm/${extraPartMatch[1]}` : originalTemplate; +}; +Then at usage site:
- // Template is in the form: /<datacenter>/vm/<extraPart> - const extraPartMatch = (vCenterDomainCfg.topology.template || '').match(/\/vm\/(.+)$/); - if (extraPartMatch) { - vCenterDomainCfg.topology.template = `/${values.datacenter}/vm/${extraPartMatch[1]}`; - } + vCenterDomainCfg.topology.template = getInfrastructureTemplatePath( + values.datacenter, + vCenterDomainCfg.topology.template, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/vsphere-plugin/src/components/persist.ts` around lines 514 - 518, Extract the template-rewriting logic into a new helper named getInfrastructureTemplatePath and use it where vCenterDomainCfg.topology.template is currently mutated; specifically, move the logic that parses (vCenterDomainCfg.topology.template || '').match(/\/vm\/(.+)$/) and constructs `/${values.datacenter}/vm/${extraPart}` into getInfrastructureTemplatePath(template, datacenter) and replace the inline code with vCenterDomainCfg.topology.template = getInfrastructureTemplatePath(vCenterDomainCfg.topology.template, values.datacenter); this mirrors getInfrastructureResourcePoolPath and improves consistency and testability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/packages/vsphere-plugin/src/components/persist.ts`:
- Around line 514-518: Extract the template-rewriting logic into a new helper
named getInfrastructureTemplatePath and use it where
vCenterDomainCfg.topology.template is currently mutated; specifically, move the
logic that parses (vCenterDomainCfg.topology.template ||
'').match(/\/vm\/(.+)$/) and constructs `/${values.datacenter}/vm/${extraPart}`
into getInfrastructureTemplatePath(template, datacenter) and replace the inline
code with vCenterDomainCfg.topology.template =
getInfrastructureTemplatePath(vCenterDomainCfg.topology.template,
values.datacenter); this mirrors getInfrastructureResourcePoolPath and improves
consistency and testability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c73067a-396f-48d5-b0c7-d2009f263bac
📒 Files selected for processing (2)
frontend/packages/vsphere-plugin/src/components/persist.tsfrontend/packages/vsphere-plugin/src/resources/infrastructure.ts
|
/retest |
|
@celdrake: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
2 participants
The Vsphere Plugin was not managing the
platform.vsphere.failureDomains.topology.templatefield.If it's defined, we'll ensure it has the expected pattern
/<datacenter>/vm/<existing-value>.This covers all possible cases:
datacenterplaceholderfrom Assisted Installer)datacentervalue is modified later by the userSummary by CodeRabbit
Release Notes