Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.

Bug 2095135

Summary: [RFE] Wrap notes with long lines more nicely
Product: [oVirt] otopi Reporter: Yedidyah Bar David <didi>
Component: Plugins.dialogAssignee: Yedidyah Bar David <didi>
Status: CLOSED CURRENTRELEASE QA Contact: Nikolai Sednev <nsednev>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 1.10.0CC: bugs, dfodor, lsvaty, michal.skrivanek, nsednev
Target Milestone: ovirt-4.5.2Keywords: FutureFeature, Triaged
Target Release: ---Flags: sbonazzo: ovirt-4.5+
pm-rhel: planning_ack?
sbonazzo: devel_ack+
pm-rhel: testing_ack?
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Enhancement
Doc Text:
otopi based tools will display long lines in a nicer way when executed in smaller console windows.
 Story Points: ---
Clone Of: Environment:
Last Closed: 2022-08-30 08:47:42 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Integration RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 2114928    

Description Yedidyah Bar David 2022-06-09 06:11:07 UTC 
Description of problem:

otopi-based tools that want to output longer texts to the user, using (usually) Plugin.dialog.note, prepend spaces to the first line, to align it with lines having a status prepended (such as "[INFO]"), but longer lines that do not fit in a single terminal line are wrapped by the terminal to the next line unaligned. This makes the output somewhat ugly.

It would be nice if otopi did the wrapping/alignment itself, based on the terminal width, instead of relying on the terminal.

This was discussed in bug 1881280. See also screenshots/outputs there.

Version-Release number of selected component (if applicable):
Forever

How reproducible:
Always

Steps to Reproduce:
1. Run e.g. engine-setup or 'hosted-engine --deploy' on a narrow terminal
2.
3.

Actual results:
E.g. (with bug 1881280 fixed):

          Please enter the name of the data center where you want to deploy this
 hosted-engine host.
          Please note:        
          If you are restoring a backup of a hosted-engine, this value should be
 the data center of the HostedEngine VM and hosts. This can be useful for moving
 an existing hosted-
          engine setup to new storage which will be attached to the existing dat
a center of HostedEngine.
          If you are restoring a backup of a hosted-engine, this value should be
 the data center of the HostedEngine VM and hosts.
          If you are migrating from a standalone engine to a hosted-engine, this
 value should be the target data center you wish to add the HostedEngine VM and 
this host to.
          The default, for new deployments, is "Default", but is not supplied he
re, because you are restoring from a backup - please make sure that the value yo
u supply is correct.
          Data center:   

Expected results:

This is the output with the proposed patch. I think it's ok for "expected results":

          Please enter the name of the data center where
          you want to deploy this hosted-engine host.
          Please note:
          If you are restoring a backup of a hosted-
          engine, this value should be the data center
          of the HostedEngine VM and hosts. This can be
          useful for moving an existing hosted-engine
          setup to new storage which will be attached to
          the existing data center of HostedEngine.
          If you are restoring a backup of a hosted-
          engine, this value should be the data center
          of the HostedEngine VM and hosts.
          If you are migrating from a standalone engine
          to a hosted-engine, this value should be the
          target data center you wish to add the
          HostedEngine VM and this host to.
          The default, for new deployments, is
          "Default", but is not supplied here, because
          you are restoring from a backup - please make
          sure that the value you supply is correct.
          Data center:

Additional info:
Comment 1 Yedidyah Bar David 2022-06-09 06:13:05 UTC 
QE: Please approve. This affects most outputs to the user of 'engine-setup' and 'hosted-engine --deploy', so all should pass at least some sanity check quickly reviewing the output.
Comment 2 Nikolai Sednev 2022-06-14 12:35:39 UTC 
(In reply to Yedidyah Bar David from comment #1)
> QE: Please approve. This affects most outputs to the user of 'engine-setup'
> and 'hosted-engine --deploy', so all should pass at least some sanity check
> quickly reviewing the output.

As I had originally reported the issue here https://bugzilla.redhat.com/show_bug.cgi?id=1881280#c24, I've no objections as long as it doesn't break any functionality and presented accordingly to your patch and looks like this https://bugzilla.redhat.com/show_bug.cgi?id=1881280#c25.
Comment 3 Yedidyah Bar David 2022-06-14 12:45:52 UTC 
(In reply to Nikolai Sednev from comment #2)
> (In reply to Yedidyah Bar David from comment #1)
> > QE: Please approve. This affects most outputs to the user of 'engine-setup'
> > and 'hosted-engine --deploy', so all should pass at least some sanity check
> > quickly reviewing the output.
> 
> As I had originally reported the issue here
> https://bugzilla.redhat.com/show_bug.cgi?id=1881280#c24, I've no objections
> as long as it doesn't break any functionality and presented accordingly to
> your patch and looks like this
> https://bugzilla.redhat.com/show_bug.cgi?id=1881280#c25.

I only did brief verification, and it worked well. Didn't test engine-setup
nor non-standard flows where we ask/show different information.
Comment 4 Michal Skrivanek 2022-06-20 12:08:46 UTC 
missed 4.5.1 deadline
Comment 5 Yedidyah Bar David 2022-07-20 06:59:26 UTC 
QE:

Please run manually both engine-setup and 'hosted-engine --deploy' on various terminal widths - I tested 190 colums (my default full-screen, things look almost identical) and 80 column (looks better even if still not perfect).

I made sure to try and not break long URLs and filenames so that double-click to copy them would still work.

I also didn't combine existing separate lines - only broke long ones - so in some cases things look a bit ugly. In other cases, it might make sense to add explicit line breaks. E.g.:
========================================================================================
          * Please note * : Data Warehouse is required for the engine.
          If you choose to not configure it on this host, you have to
          configure
          it on a remote host, and then configure the engine on this host
          so
          that it can access the database of the remote Data Warehouse
          host.
          Configure Data Warehouse on this host (Yes, No) [Yes]:
========================================================================================
Or:
========================================================================================
          Use Engine admin password as initial keycloak admin [admin] and
          [admin@ovirt] administration panel user password (Yes, No)
          [Yes]:
========================================================================================
If you think that's ugly enough to be worth a bug/fix, please file one (on the respective utilities' products/components, not otopi). I still think this is an improvement, so merged it.
Comment 7 Nikolai Sednev 2022-08-11 15:17:19 UTC 
I tested 110 columns and 28 rows.
I still see some not nicely breaking lines.



          If you run "hosted-engine --deploy" without the "--4" or "--6" option in a dual-stack
          environment, the default is IPv6.
          You must ensure that your DNS returns only IPv6 addresses.
          See:
          https://ovirt.org/documentation/installing_ovirt_as_a_self-hosted_engine_using_the_command_line/index.html#D
eploying_the_Self-Hosted_Engine_Using_the_CLI_install_RHVM
          Do you want to continue anyway? (Yes, No)[No]:yes
          Configuration files:

[ INFO  ] The Engine VM FQDN was resolved into: '10.35.232.207'.
[WARNING] The Engine VM ('10.35.232.207') and this host (2620:52:0:23e8:e643:4bff:fef6:6dd8/64) will not be in the sam
e IP subnet.
         Static routing configuration are not supported on automatic VM configuration.
         
          OK?  (Yes, No, Abort) [No]:Abort


[ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Fail if only VLAN devices with invalid naming convention are availab
le]
[ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Validate selected bridge interface if management bridge does not exi
st]
[ INFO  ] skipping: [localhost]
[ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Fail if only VLAN devices with invalid naming convention are availab
le]
[ INFO  ] skipping: [localhost]
[ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Validate selected bridge interface if management bridge does not exi
st]
[ INFO  ] skipping: [localhost]
[ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Force the local VM FQDN to temporary resolve on the natted network a
ddress]
[ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Allow the user to connect to the bootstrap engine VM and change conf
iguration]
[ INFO  ] skipping: [localhost]
[ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Set Engine public key as authorized key without validating the TLS/S
SL certificates]
[ INFO  ] changed: [localhost]


There are still impurities exists. 

In case you think that it takes too much of the resources to refine, please consider closing the bug.
Comment 8 Yedidyah Bar David 2022-08-15 06:35:22 UTC 
(In reply to Nikolai Sednev from comment #7)
> I tested 110 columns and 28 rows.
> I still see some not nicely breaking lines.
> 
> 
> 
>           If you run "hosted-engine --deploy" without the "--4" or "--6"
> option in a dual-stack
>           environment, the default is IPv6.
>           You must ensure that your DNS returns only IPv6 addresses.
>           See:
>          
> https://ovirt.org/documentation/installing_ovirt_as_a_self-
> hosted_engine_using_the_command_line/index.html#D
> eploying_the_Self-Hosted_Engine_Using_the_CLI_install_RHVM
>           Do you want to continue anyway? (Yes, No)[No]:yes

In any of the above, do you see any problem?

The URL is long, and I deliberately do not break "words" (e.g. such a URL).

>           Configuration files:
> 
> [ INFO  ] The Engine VM FQDN was resolved into: '10.35.232.207'.
> [WARNING] The Engine VM ('10.35.232.207') and this host
> (2620:52:0:23e8:e643:4bff:fef6:6dd8/64) will not be in the sam
> e IP subnet.

The current patch does not affect at all output of lines with 'INFO', 'WARNING' etc. - because these are always single long lines, and if being wrapped by the terminal, should still be considered single lines. I don't mind trying to break these too, but decided it's not worth it for current bug, which is about longer blocks of "note" texts.

>          Static routing configuration are not supported on automatic VM
> configuration.
>          
>           OK?  (Yes, No, Abort) [No]:Abort
> 
> 
> [ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Fail if only VLAN devices
> with invalid naming convention are availab
> le]
> [ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Validate selected bridge
> interface if management bridge does not exi
> st]
> [ INFO  ] skipping: [localhost]
> [ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Fail if only VLAN devices
> with invalid naming convention are availab
> le]
> [ INFO  ] skipping: [localhost]
> [ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Validate selected bridge
> interface if management bridge does not exi
> st]
> [ INFO  ] skipping: [localhost]
> [ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Force the local VM FQDN to
> temporary resolve on the natted network a
> ddress]
> [ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Allow the user to connect
> to the bootstrap engine VM and change conf
> iguration]
> [ INFO  ] skipping: [localhost]
> [ INFO  ] TASK [ovirt.ovirt.hosted_engine_setup : Set Engine public key as
> authorized key without validating the TLS/S
> SL certificates]
> [ INFO  ] changed: [localhost]

Same.

> 
> 
> There are still impurities exists. 

I agree it's not very nice, but not sure exactly how to improve it without risking bigger breakage. I especially do not like (as a user) when searching the net/bugzilla/docs/whatever for error messages, to have to spend time on picking the "right amount" of the message - and breaking error messages across lines will make this more likely to be a problem, IMO. If at all, I'd spend time on shortening the longer warnings/errors and have "notes" before or after them if needed, but that's much more work than I intended to invest for the current bug.

> 
> In case you think that it takes too much of the resources to refine, please
> consider closing the bug.

If you see breakage, as in regressions, compared to previous versions, please provide details. If it's serious, I'll fix. Or just give up and revert the patch, if you think it's not an improvement overall. Otherwise, if you have concrete opinions about what I wrote above and want to suggest some change, please do. Otherwise, I don't mind that we close this bug one way or another.

TL;DR:

1. Most important: Do not add regressions

2. Improve. No need to improve to perfection, at this stage.

Thanks!
Comment 9 Nikolai Sednev 2022-08-15 09:22:17 UTC 
Forth to previous comment, moving to verified as I have no reasonable objections at this stage of the product.
Tested on:
ovirt-engine-setup-4.5.2.2-0.1.el8ev.noarch 
ovirt-hosted-engine-ha-2.5.0-1.el8ev.noarch
ovirt-hosted-engine-setup-2.6.5-1.1.el8ev.noarch
Linux 4.18.0-372.19.1.el8_6.x86_64 #1 SMP Mon Jul 18 11:14:02 EDT 2022 x86_64 x86_64 x86_64 GNU/Linux
Comment 10 Sandro Bonazzola 2022-08-30 08:47:42 UTC 
This bugzilla is included in oVirt 4.5.2 release, published on August 10th 2022.
Since the problem described in this bug report should be resolved in oVirt 4.5.2 release, it has been closed with a resolution of CURRENT RELEASE.
If the solution does not work for you, please open a new bug report.