model for add/remove interface on network router

[sseago]

This PR adds model code for adding and removing router interfaces.

BZ for this is:

https://bugzilla.redhat.com/show_bug.cgi?id=1394284

Euwe backport depends on merging #13005

[miq-bot]

Checked commit sseago@4db629b with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
7 files checked, 3 offenses detected

app/models/manageiq/providers/openstack/network_manager/network_router.rb

• ❕ - Line 107, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for remove_interface is too high. [20.32/20]
• ❕ - Line 77, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for add_interface is too high. [20.32/20]

spec/lib/miq_automation_engine/service_methods/miq_ae_service_manageiq-providers-openstack-network_manager-network_router_spec.rb

• ❗ - Line 1, Col 1 - Style/FileName - Use snake_case for source file names.

[tzumainn]

👍 looks good to me. The only question I might have is whether we should use the raw_ method pattern, but I could see the argument that it's more important to make this update consistent with the rest of the model, and save the refactoring for a non-BZ fix.

[tzumainn]

@miq-bot assign @chessbyte

[sseago]

Yeah, the move back to the raw_ pattern was mentioned on a prior feature. Since this was just adding two new operations to the existing CRUD operations, it seemed out-of-place to do it differently for just two operations. If/when we refactor this to use the raw_ pattern, it needs to be done on the basic CRUD operations at the same time -- if we have some refactoring sprint set for a non-BZ release, it might be more appropriate there (and there are several other models to make this same change)

[blomquisg]

@gmcculloug any issues with removing these? I only ask because un-exposing methods might be tricky ...

I wonder if there's a deprecation model in Automate?

[sseago]

@blomquisg In this case I removed them because the underlying methods are no longer there. When I was adding the new methods for this PR I realized that a prior change had removed these two without removing the expose calls.

[gmcculloug]

Did these methods ever exist in the active_record models? They do not exist on the commit that introduced the service_model methods (060254c) and the tests are only checking that the service model contains the method.

Also git log -S "raw_update_network_router" only references the commit above. So I'm guessing these were never usable in the first place. So ✂️ .

[sseago]

@gmcculloug So if they were never in the history, I suspect it was a refactoring of the initial commit/PR before it was merged that somehow forgot to refactor the automate bits.

[blomquisg]

I'm generally good with this ... One question for @gmcculloug

[tzumainn]

@miq-bot add_label euwe/yes
@miq-bot add_label bug
@miq-bot add_label blocker

[sseago]

@blomquisg This one still needs to be backported to euwe before my UI backport PR will work.

[simaishi]

Euwe backport details:

$ git log -1
commit 98d8e34d887592e091a8ba009f0c79df171bf5bb
Author: Greg Blomquist <blomquisg@gmail.com>
Date:   Thu Dec 22 13:41:35 2016 -0500

    Merge pull request #13032 from sseago/router-interfaces-model
    
    model for add/remove interface on network router
    (cherry picked from commit c2bf3960cc213e5d8baeff16fa423936931e27b6)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1413212