Bug 1337445 - machine dialect should provide metadata - validValues, default, hidden
Summary: machine dialect should provide metadata - validValues, default, hidden
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: otopi
Classification: oVirt
Component: Plugins.dialog
Version: master
Hardware: Unspecified
OS: Unspecified
urgent
urgent
Target Milestone: ovirt-4.0.0-rc
: 1.5.0
Assignee: Yedidyah Bar David
QA Contact: Nikolai Sednev
URL:
Whiteboard:
: 1336250 (view as bug list)
Depends On: 1344900
Blocks: 1331626
TreeView+ depends on / blocked
 
Reported: 2016-05-19 08:45 UTC by Yedidyah Bar David
Modified: 2019-04-28 13:16 UTC (History)
14 users (show)

Fixed In Version:
Clone Of: 1331626
Environment:
Last Closed: 2016-07-05 08:02:49 UTC
oVirt Team: Integration
Embargoed:
rule-engine: ovirt-4.0.0+
rule-engine: blocker+
rule-engine: planning_ack+
sbonazzo: devel_ack+
mavital: testing_ack+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 56955 0 master MERGED machine_dialog: provide some metadata for queryString 2021-01-14 15:23:27 UTC

Description Yedidyah Bar David 2016-05-19 08:45:16 UTC
+++ This bug was initially created as a clone of Bug #1331626 +++

Description of problem:
It always report "Invalid number of cpu specified" and will led to HE deploy can't continue.

Version-Release number of selected component (if applicable):
rhev-hypervisor7-ng-3.6-20160426.0.x86_64
imgbased-0.6-0.1.el7ev.noarch
ovirt-hosted-engine-ha-1.3.5.3-1.el7ev.noarch
ovirt-hosted-engine-setup-1.3.6.0-1.el7ev.noarch
rhevm-appliance-20160428.0-1.x86_64.rhevm.ova

How reproducible:
100%

Steps to Reproduce:
1. Install rhev-hypervisor7-ng-3.6-20160426.0.x86_64
2. Disabled NetworkManager and start hosted-engine setup from cockpit.
3. Setup Hosted Engine step by step.
4. Focus on "Please specify the number of virtual CPUs for the VM" step.
5. Press Enter key with default value.


Actual results:
It always report "Invalid number of cpu specified" and will led to HE deploy can't continue.

Expected results:
Press Enter key with default value can be accepted.

Additional info:

--- Additional comment from shaochen on 2016-04-29 08:02 IDT ---



--- Additional comment from shaochen on 2016-04-29 08:25:24 IDT ---

Mark this bug testblocker due to it blocked our HE testing.

--- Additional comment from Red Hat Bugzilla Rules Engine on 2016-04-29 10:53:07 IDT ---

Bug tickets must have version flags set prior to targeting them to a release. Please ask maintainer to set the correct version flags and only then set the target milestone.

--- Additional comment from Red Hat Bugzilla Rules Engine on 2016-04-29 10:54:51 IDT ---

This bug report has Keywords: Regression or TestBlocker.
Since no regressions or test blockers are allowed between releases, it is also being identified as a blocker for this release. Please resolve ASAP.

--- Additional comment from Ryan Barry on 2016-04-29 17:08:09 IDT ---

The entry needs to be "2" or similar.

The entry box has that text because it's the suggestion from ovirt-hosted-engine-setup (this is also the case for the next input, which is memory, it's something like "Defaults to 4096").

The problem is in ovirt-hosted-engine-setup, and that the default output (even in machine readable format) does not output a suggested input which can be used without modification.

The output from otopi should be corrected. You can pass this in cockpit by entering a literal number, which should be suggested in the first place. Using "2" instead of "Defaults to minimum requirement: 2" will continue. This should not be blocking testing.

Changing the component. I'll submit a patch to ovirt-hosted-engine-setup.

--- Additional comment from Red Hat Bugzilla Rules Engine on 2016-04-29 17:08:17 IDT ---

Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.

--- Additional comment from Red Hat Bugzilla Rules Engine on 2016-04-29 17:08:48 IDT ---

Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.

--- Additional comment from Fabian Deutsch on 2016-04-29 19:37:20 IDT ---

A workaround is to remove the complete text from the input field and just enter the number (2)

--- Additional comment from shaochen on 2016-05-03 11:07:46 IDT ---

(In reply to Fabian Deutsch from comment #8)
> A workaround is to remove the complete text from the input field and just
> enter the number (2)

The workaround can work well.

Test version:
rhev-hypervisor7-ng-3.6-20160429.0
imgbased-0.6-0.1.el7ev.noarch
ovirt-hosted-engine-ha-1.3.5.3-1.el7ev.noarch
ovirt-hosted-engine-setup-1.3.6.0-1.el7ev.noarch
rhevm-appliance-20160428.0-1.x86_64.rhevm.ova

Test steps:
1. Install rhev-hypervisor7-ng-3.6-20160429.0
2. Disabled NetworkManager and start hosted-engine setup from cockpit.
3. #rpm -ivh rhevm-appliance-20160428.0-1.el7ev.noarch.rpm 
4. Setup Hosted Engine step by step(OVA).
5. Focus on "Please specify the number of virtual CPUs for the VM" step.
6. Remove the complete text from the input field and just enter the number (2)

Test result:
Deploy HE can successful.

--- Additional comment from shaochen on 2016-05-03 11:34 IDT ---



--- Additional comment from Fabian Deutsch on 2016-05-03 11:57:17 IDT ---

Logs:

2016-05-03 14:14:09 DEBUG otopi.context context._executeMethod:142 Stage customization METHOD otopi.plugins.ovirt_hosted_engine_setup.vm.cpu.Plugin._customization
2016-05-03 14:14:10 DEBUG otopi.plugins.otopi.dialog.machine dialog.__logString:219 DIALOG:SEND       ***Q:STRING ovehosted_vmenv_cpu
2016-05-03 14:14:10 DEBUG otopi.plugins.otopi.dialog.machine dialog.__logString:219 DIALOG:SEND       ### Please specify the number of virtual CPUs for the VM [Defaults to appliance OVF value: 2]: 
2016-05-03 14:14:11 DEBUG otopi.plugins.otopi.dialog.machine dialog.__logString:219 DIALOG:RECEIVE    Defaults to appliance OVF value: 2
2016-05-03 14:14:11 ERROR otopi.plugins.ovirt_hosted_engine_setup.vm.cpu cpu._customization:173 Invalid number of cpu specified: Defaults to appliance OVF value: 2
2016-05-03 14:14:11 DEBUG otopi.plugins.otopi.dialog.machine dialog.__logString:219 DIALOG:SEND       ***Q:STRING ovehosted_vmenv_cpu
2016-05-03 14:14:11 DEBUG otopi.plugins.otopi.dialog.machine dialog.__logString:219 DIALOG:SEND       ### Please specify the number of virtual CPUs for the VM [Defaults to appliance OVF value: 2]: 
2016-05-03 14:14:16 DEBUG otopi.plugins.otopi.dialog.machine dialog.__logString:219 DIALOG:RECEIVE    2
2016-05-03 14:14:16 DEBUG otopi.context context.dumpEnvironment:500 ENVIRONMENT DUMP - BEGIN
2016-05-03 14:14:16 DEBUG otopi.context context.dumpEnvironment:510 ENV OVEHOSTED_VM/vmVCpus=str:'2'
2016-05-03 14:14:16 DEBUG otopi.context context.dumpEnvironment:514 ENVIRONMENT DUMP - END

To me it makes sense that there is an additional text ("Defaults to appliance OVF value") accompanying the default value.
But this causes the parse error on the UI side.

An easy fix could be to just pull the additional text out of the square brackets:
OLD ### Please specify the number of virtual CPUs for the VM [Defaults to appliance OVF value: 2]: 
NEW ### Please specify the number of virtual CPUs for the VM. Defaults to appliance OVF value: [2]:

Would this be suitable as a short-term fix?

--- Additional comment from Fabian Deutsch on 2016-05-03 11:58:04 IDT ---

There are three places where such an adjustment must be made to fix he-setup in Cockpit.

--- Additional comment from Simone Tiraboschi on 2016-05-03 12:11:01 IDT ---

I'm not that sure:
on hosted-engine-setup the dialog is set in this way:

                default = self.environment[ohostedcons.VMEnv.APPLIANCEVCPUS]
                self.environment[
                    ohostedcons.VMEnv.VCPUS
                ] = self.dialog.queryString(
                    name='ovehosted_vmenv_cpu',
                    note=_(
                        'Please specify the number of virtual CPUs for the VM '
                        '[Defaults to {default_msg}: @DEFAULT@]: '
                    ).format(default_msg=default_msg),
                    prompt=True,
                    default=default,
                )

You have to consume the default value that is tagged as "default", not to parse the dialog text to extract it.

If machine dialog dialect doesn't provide the default value we need to patch otopi for that but parsing the text output isn't the way to go.

--- Additional comment from Fabian Deutsch on 2016-05-03 13:22:44 IDT ---

Yes - Ryan can probably answer what is getting parsed.

--- Additional comment from Ryan Barry on 2016-05-03 16:30:18 IDT ---

(In reply to Fabian Deutsch from comment #14)
> Yes - Ryan can probably answer what is getting parsed.

We're parsing the text output (in otopi machine readable format) because, as noted, even the matchine readable format does not provide the default value as a separate field.

We get something like:

***Q:STRING DEPLOY_PROCEED
### Continuing will configure this host for serving as hypervisor and create a VM where you have to install the engine afterwards.
### Are you sure you want to continue? (Yes, No)[Yes]: 

The patch under discussion for otopi is the correct fix, though it requires changing the parser, since even machine dialect does not contain a separate entry for the default

I'm concerned about two things:

1) Will this land in 3.6?

2) Even if the patch to otopi makes it to 3.6, gerrit 56935 should still be merged in my opinion, since it provides consistency. In other input fields where information which is helpful to users is presented, it is not inside []. It appears to be these three fields only. For example:

Please specify the storage you would like to use (glusterfs, iscsi, fc, nfs3, nfs4)[nfs3]
Please specify the full shared storage connection path to use (example: host:/path):

--- Additional comment from Yedidyah Bar David on 2016-05-03 16:49:46 IDT ---

(In reply to Ryan Barry from comment #15)
> (In reply to Fabian Deutsch from comment #14)
> > Yes - Ryan can probably answer what is getting parsed.
> 
> We're parsing the text output (in otopi machine readable format) because, as
> noted, even the matchine readable format does not provide the default value
> as a separate field.
> 
> We get something like:
> 
> ***Q:STRING DEPLOY_PROCEED
> ### Continuing will configure this host for serving as hypervisor and create
> a VM where you have to install the engine afterwards.
> ### Are you sure you want to continue? (Yes, No)[Yes]: 
> 
> The patch under discussion for otopi is the correct fix, though it requires
> changing the parser, since even machine dialect does not contain a separate
> entry for the default
> 
> I'm concerned about two things:
> 
> 1) Will this land in 3.6?

If we must, we must...

> 
> 2) Even if the patch to otopi makes it to 3.6, gerrit 56935 should still be
> merged in my opinion, since it provides consistency.

+1

--- Additional comment from wanghui on 2016-05-10 12:06:22 IDT ---

Also encounter this issue when setup HE with OVA type.

1. Please specify the number of virtual CPUs for the VM [Defaults to appliance OVF value: 2]:

2. Please specify the memory size of the VM in MB [Defaults to appliance OVF value: 4096]:

--- Additional comment from Ryan Barry on 2016-05-10 18:44:22 IDT ---

Comment 1 Red Hat Bugzilla Rules Engine 2016-05-19 09:34:54 UTC
This bug report has Keywords: Regression or TestBlocker.
Since no regressions or test blockers are allowed between releases, it is also being identified as a blocker for this release. Please resolve ASAP.

Comment 2 Yedidyah Bar David 2016-05-26 07:15:27 UTC
*** Bug 1336250 has been marked as a duplicate of this bug. ***

Comment 3 Nikolai Sednev 2016-06-09 13:05:46 UTC
Please provide me with relevant link to NGN ISO image, as we don't have such yet.

Comment 4 Fabian Deutsch 2016-06-09 13:17:46 UTC
This bug can also be verified on a regular RHEL-H host with the correct hosted-engine-setup.

Comment 5 Ryan Barry 2016-06-09 13:19:13 UTC
To follow up:

hosted-engine --deploy --otopi-environment="DIALOG/dialect=str:machine"

Will show the appropriate %QStart, %QEnd, %QHidden, etc.

Comment 6 Yedidyah Bar David 2016-06-09 13:23:08 UTC
Indeed, and also plain engine-setup with same option. All you need is recent enough otopi (1.5.0 should be ok).

Comment 7 Yedidyah Bar David 2016-06-14 13:34:56 UTC
Why does this bug depend on bug 1344900?

See above comments - you can verify it with any otopi-based tool that allows passing options. E.g.:

engine-setup --otopi-environment="DIALOG/dialect=str:machine"

Run it with an unpatched otopi and with a patched one and you can see the added lines.

Comment 8 Nikolai Sednev 2016-06-14 13:47:41 UTC
(In reply to Yedidyah Bar David from comment #7)
> Why does this bug depend on bug 1344900?
> 
> See above comments - you can verify it with any otopi-based tool that allows
> passing options. E.g.:
> 
> engine-setup --otopi-environment="DIALOG/dialect=str:machine"
> 
> Run it with an unpatched otopi and with a patched one and you can see the
> added lines.

I've set the dependency because of the initial requirement for step 3. was "Setup Hosted Engine step by step."
Unless 1344900 fixed, the deployment will fail. But generally you're correct, just to see if during the deployment step 5. "Press Enter key with default value." with default value of 2 virtual CPUs pass, then this bug can be verified.

Where can I get ISO image with latest 4.0 NGN?

Comment 10 Ryan Barry 2016-06-14 13:52:48 UTC
However, I would also say that bz#1344900 should not block verification of this bug, since the entire flow up until the point where it fails depends on proper parsing of the new otopi output.

Comment 11 Yedidyah Bar David 2016-06-14 14:20:42 UTC
This bug was cloned from hosted-engine-setup to otopi, because we decided the fix should be in otopi.

The bug subject line is all that matters. Comment 0 was left as-is only to provide context.

For current bug, otopi was changed to provide additional meta-data when using machine dialect: Default value, whether the reply should be Hidden from the user, and the list of Valid Values.

You can verify with any otopi tool that interacts with the user and has these in at least one of its questions. For full coverage, you need questions with:
1. No default
e.g. admin password in engine-setup, or ISO domain ACL (choose 'yes' to configure it)
2. Some default
e.g. 'Configure Engine on this host (Yes, No) [Yes]'
3. No valid values (meaning, any value is valid)
e.g. admin password
4. Some valid values
e.g. 'Configure Engine on this host (Yes, No)'
5. Answer should be hidden
e.g. admin password
6. Answer should not be hidden
e.g. 'Configure Engine on this host (Yes, No) [Yes]'

For documentation of the additions to the machine dialect protocol, see otopi sources:

https://gerrit.ovirt.org/#/c/56955/16/README.dialog

Also note that at some point, the original bug was fixed by parsing the text meant for humans, which is imperfect (e.g. can fail due to translation). I think this is bug 1298160. Not sure if new code was later written to use the new additions of this bug. But this is irrelevant for current bug - current is on otopi.

Comment 12 Nikolai Sednev 2016-06-15 16:01:20 UTC
Works for me on these components:
Red Hat Enterprise Linux release 7.2 Beta
Linux 3.10.0-327.18.2.el7.x86_64 #1 SMP Fri Apr 8 05:09:53 EDT 2016 x86_64 x86_64 x86_64 GNU/Linux
Linux version 3.10.0-327.18.2.el7.x86_64 (mockbuild.eng.bos.redhat.com) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-4) (GCC) ) #1 SMP Fri Apr 8 05:09:53 EDT 2016
qemu-kvm-rhev-2.3.0-31.el7_2.13.x86_64
ovirt-setup-lib-1.0.2-1.el7ev.noarch
ovirt-host-deploy-1.5.0-1.el7ev.noarch
ovirt-engine-sdk-python-3.6.5.0-2.el7ev.noarch
sanlock-3.2.4-1.el7.x86_64
libvirt-client-1.2.17-13.el7_2.4.x86_64
mom-0.5.4-1.el7ev.noarch
vdsm-4.18.2-0.el7ev.x86_64
ovirt-hosted-engine-ha-2.0.0-1.el7ev.noarch
ovirt-hosted-engine-setup-2.0.0.1-1.el7ev.noarch
ovirt-vmconsole-1.0.3-1.el7ev.noarch
ovirt-vmconsole-host-1.0.3-1.el7ev.noarch


Deployment did not crushed on previous steps.

Comment 13 Sandro Bonazzola 2016-07-05 08:02:49 UTC
oVirt 4.0.0 has been released, closing current release.


Note You need to log in before you can comment on or make changes to this bug.