Bug 1337445

+++ 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 ---