Skip to content

Conversation

@celdrake
Copy link
Contributor

Whenever the "cloud-provider-config" ConfigMap has different data that the UI expects, the edition of the vsphere connection details fails and there's no helpful information about why it failed.

Now, we clearly display what data that we cannot find in the initial ConfigMap is preventing us from making the changes the user requested.

cm-more-info

@openshift-ci openshift-ci bot requested review from mareklibra and rawagner August 27, 2025 07:53
@celdrake celdrake force-pushed the vsphere-configMap-info branch from 72c52fb to ea76515 Compare August 27, 2025 10:02
@celdrake
Copy link
Contributor Author

/retest

@logonoff
Copy link
Member

/label NO-JIRA: Improve error message in vsphere-plugin

@logonoff
Copy link
Member

/retitle NO-JIRA: Improve error message in vsphere-plugin

@openshift-ci openshift-ci bot changed the title Improve error message in vsphere-plugin NO-JIRA: Improve error message in vsphere-plugin Aug 28, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 28, 2025
@openshift-ci-robot
Copy link
Contributor

@celdrake: This pull request explicitly references no jira issue.

Details

In response to this:

Whenever the "cloud-provider-config" ConfigMap has different data that the UI expects, the edition of the vsphere connection details fails and there's no helpful information about why it failed.

Now, we clearly display what data that we cannot find in the initial ConfigMap is preventing us from making the changes the user requested.

cm-more-info

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.

throw new PersistError(
t('Failed to parse cloud provider config {{cm}}', { cm: cloudProviderConfig.metadata.name }),
t('Unknown format'),
t('Failed to parse cloud provider config {{cm}}. ', {
Copy link
Member

Choose a reason for hiding this comment

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

the extra space and punctuation in this string is causing duplicate strings to appear in the locales json file

Suggested change
t('Failed to parse cloud provider config {{cm}}. ', {
t('Failed to parse cloud provider config {{cm}}', {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely right, thanks! I'm fixing it now.

@logonoff
Copy link
Member

/approve

/label tide/merge-method-squash
/label px-approved
/label docs-approved

qe review:
/assign @yapei

code review:
/assign @rhamilto

@openshift-ci openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 28, 2025
@openshift-ci openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 28, 2025
@yapei
Copy link
Contributor

yapei commented Aug 29, 2025

@yanpzhan is working on PR verification

@yanpzhan
Copy link
Contributor

@celdrake I'm trying to verify the pr. Could you point me which resource field you modified to trigger the error? Thanks in advance.

@celdrake
Copy link
Contributor Author

@yanpzhan What I did is to create a cluster using ClusterBot with the instruction "launch 4.17 vsphere".
This created a configuration where the folder and the resourcepool-path mismatch between several CRDs.

Then any modification should trigger the issue. Try modifying the folder for example.

@yanpzhan
Copy link
Contributor

@celdrake This is the warning info when I update 'Virtual Machine Folder * ' field with '/DEVQEdatacenter-2/vm/prtest', seems not the expected info.
Screenshot from 2025-08-29 18-02-03

@celdrake
Copy link
Contributor Author

@yanpzhan In this case, it seems your vsphere settings are all correct. Therefore no error is displayed when you make any changes to the Vsphere connection details, and they can be successfully updated (which is the happy path).

My PR only adds extra information when the vsphere settings have mismatches between different CRDs. Is it possible for you to check with the clusterBot's cluster?

@yanpzhan
Copy link
Contributor

Reproduced the issue on 4.17 cluster, bridge console with the pr code, now there is more detailed failure info.
before on 4.17:
Screenshot from 2025-08-29 22-40-34

after:
Screenshot from 2025-08-29 22-50-06
/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Aug 29, 2025
@celdrake celdrake requested a review from logonoff September 1, 2025 09:03
Copy link
Member

@logonoff logonoff left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 2, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 2, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: celdrake, logonoff

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

@logonoff
Copy link
Member

logonoff commented Sep 8, 2025

/verified by yanpzhan

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Sep 8, 2025
@openshift-ci-robot
Copy link
Contributor

@logonoff: This PR has been marked as verified by yanpzhan.

Details

In response to this:

/verified by yanpzhan

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.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 0748392 and 2 for PR HEAD 30c5cf8 in total

1 similar comment
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 0748392 and 2 for PR HEAD 30c5cf8 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 80a9f24 and 1 for PR HEAD 30c5cf8 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 80a9f24 and 2 for PR HEAD 30c5cf8 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD e76feca and 2 for PR HEAD 30c5cf8 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 9, 2025

@celdrake: all tests passed!

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-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 343b6dd into openshift:main Sep 9, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. docs-approved Signifies that Docs has signed off on this PR jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants