Bug 1443412 - [UI] -Gray out 'Default Route' checkbox on management network for Network>Clusters flow
Summary: [UI] -Gray out 'Default Route' checkbox on management network for Network>Clu...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: ovirt-engine
Classification: oVirt
Component: BLL.Network
Version: future
Hardware: x86_64
OS: Linux
medium
low
Target Milestone: ovirt-4.2.0
: ---
Assignee: Martin Mucha
QA Contact: Michael Burman
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-04-19 08:20 UTC by Ori Ben Sasson
Modified: 2017-12-20 11:42 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-12-20 11:42:51 UTC
oVirt Team: Network
Embargoed:
rule-engine: ovirt-4.2+


Attachments (Terms of Use)
screen recording (2.23 MB, application/ogg)
2017-04-19 08:20 UTC, Ori Ben Sasson
no flags Details
record verification of the bug in the subject (719.11 KB, application/x-gzip)
2017-08-03 08:39 UTC, Michael Burman
no flags Details


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 76978 0 master MERGED core: one should not be able to deassign defaultRole from mgmt network 2017-05-18 12:37:16 UTC
oVirt gerrit 77544 0 master MERGED webadmin: deassign defaultRole from mgmt network within single cluster 2017-05-30 13:47:23 UTC

Description Ori Ben Sasson 2017-04-19 08:20:38 UTC
Created attachment 1272519 [details]
screen recording

Description of problem:
Default route parameter does not save after unchecked it.

Version-Release number of selected component (if applicable):
ovirt-engine-4.2.0-0.0.master.20170417082636.git8790a6f.el7.centos.noarch


How reproducible:
100

Steps to Reproduce:
1.Click on Network tab --> Cluster --> Manage Network 
2.Unchecked default route --> Click Ok

Actual results:
Default route checkbox checked after unchecked it 

Expected results:
Default route should be unchecked 

Additional info:
- If it is allowed that the host can live without any network as the default route, then it's should be possible to uncheck the default route property via the 'Networks'>'Clusters' flow.
- If it is not allowed, then it's should be grayed out like all other roles.

Comment 1 Dan Kenigsberg 2017-04-23 10:55:09 UTC
It should be grayed-out, just like the other roles.

Comment 2 Martin Mucha 2017-05-04 15:19:31 UTC
(In reply to Dan Kenigsberg from comment #1)
> It should be grayed-out, just like the other roles.

having bugs like: 
https://bugzilla.redhat.com/show_bug.cgi?id=1447014 
gives an idea, that mgmt can replace default route. Backend is even implemented like that. Therefore it's very cheap to do that, and I believe user should be able to untick default route here.

I can confirm, that there was problem with unticking this network role. I started debugging it, because this should really work. But after several unsuccessful sessions, without any change to code at all, it now works and I cannot reproduce it. Default route role is now successfully and reliably (withing current time window) controlled from withing this manage dialog.

Comment 3 Martin Mucha 2017-05-10 14:21:47 UTC
tried again after some delay, and rebased on different master, and I cannot reproduce it any more. Can you retry, if you still run into this scenario, and if so, perhaps provide more details about your env, because on my it (now) works 100%.

Comment 4 Ori Ben Sasson 2017-05-10 15:27:28 UTC
(In reply to Martin Mucha from comment #3)
> tried again after some delay, and rebased on different master, and I cannot
> reproduce it any more. Can you retry, if you still run into this scenario,
> and if so, perhaps provide more details about your env, because on my it
> (now) works 100%.

It 100% reproduce in our env latest master, please contact me in a private message and I will provide you our env

Comment 5 Martin Mucha 2017-05-18 08:00:50 UTC
more specific info about bug: 'unchecking' default route role on specific network works OK and it is actually saved, but if this role is unassigned from network, it is reassigned back to management network. Therefore unchecking should be disabled on mgmt network, since otherwise it looks like, that command does nothing.

Comment 6 Michael Burman 2017-07-04 13:52:02 UTC
Default route checkbox is now grayed out(on the Network>Cluster flow) and unchecking it is disabled on the management network. 

Verified on - 4.2.0-0.0.master.20170702100738.git46a9f67.el7.centos

Comment 7 Michael Burman 2017-08-02 12:14:38 UTC
Why patches added to this bug?? it was verified. No patches should be added here!

Comment 8 Martin Mucha 2017-08-02 15:32:21 UTC
probably it was badly verified.

Comment 9 Michael Burman 2017-08-03 08:37:18 UTC
(In reply to Martin Mucha from comment #8)
> probably it was badly verified.

"probably it was badly verified"?
Please explain what this means?
This bug was reported against a small scenario, the unchecking of the 'Default route' checkbox on the 'Networks'>Clusters' flow for ovirtmgmt network, that's it. 
We decided to block such option and this is exactly what was tested here.
It is no longer possible to uncheck the 'Default route' checkbox for the ovirtmgmt network, only to check a different non-mgmt network.

The bug was moved to ON_QA and tested. 
Your current and latest patch has nothing to do with the bug in the subject.

You say it was badly verified, then please say what exactly was badly verified here and add the exact steps to test what you think needed to be test here.

Comment 10 Michael Burman 2017-08-03 08:39:46 UTC
Created attachment 1308641 [details]
record verification of the bug in the subject

Comment 11 Martin Mucha 2017-08-03 10:17:57 UTC
former patch did exactly what was in title: "Gray out 'Default Route' checkbox on management network for Network>Clusters flow." In turn, it became impossible to lose default route role on mgmt network, because no action is allowed for default route on mgmt network. This is invalid.

One has to be able to grant default route role for mgmt network in this dialog. Therefore 'graying' was done incorrectly. Therefore this bug is not fixed, even if it was verified.

If you want to do it differently, please fill a bug for newly discovered failure, I will remove patch from here, revert it on master, and push it as a new patch related to new bug. But really it seems to be just an extra work without benefit.

Comment 12 Meni Yakove 2017-08-03 10:44:58 UTC
It should be a new bug, the benefit is that for the new bug we will verify the new scenario and for this one, we will never verify since it 's already verified. 

The extra patch that you add fix a new bug that the original patch adds.

The bug was verified as it should and another bug should be opened for the new failure.

Comment 13 Michael Burman 2017-08-03 10:52:27 UTC
(In reply to Martin Mucha from comment #11)
> former patch did exactly what was in title: "Gray out 'Default Route'
> checkbox on management network for Network>Clusters flow." In turn, it
> became impossible to lose default route role on mgmt network, because no
> action is allowed for default route on mgmt network. This is invalid.
> 
> One has to be able to grant default route role for mgmt network in this
> dialog. Therefore 'graying' was done incorrectly. Therefore this bug is not
> fixed, even if it was verified.
> 
> If you want to do it differently, please fill a bug for newly discovered
> failure, I will remove patch from here, revert it on master, and push it as
> a new patch related to new bug. But really it seems to be just an extra work
> without benefit.

We have no idea what is this newly discovered failure.
Please remove the extra patch from this report, open a new bug and describe exactly what is this bug!(as we have no idea what are you talking about), what should be tested and how, with the full steps to test your newly failures. Thanks,

Comment 14 Sandro Bonazzola 2017-12-20 11:42:51 UTC
This bugzilla is included in oVirt 4.2.0 release, published on Dec 20th 2017.

Since the problem described in this bug report should be
resolved in oVirt 4.2.0 release, published on Dec 20th 2017, it has been closed with a resolution of CURRENT RELEASE.

If the solution does not work for you, please open a new bug report.


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