Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable CRUD for Network provider elements for non-OpenStack providers #1007

Conversation

AparnaKarve
Copy link
Contributor

@AparnaKarve AparnaKarve commented Apr 11, 2017

Disable CRUD for Network provider elements (like Network Routers, Floating IPs, Security Groups) for non-Openstack providers.

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

Configuration Menu for Networks -> Security Groups when at least one OpenStack Network Manager exists
Add a New Security Group is enabled

screen shot 2017-04-12 at 4 58 44 pm

Configuration Menu for Networks -> Security Groups when OpenStack Network Managers do not exist
Add a New Security Group is disabled

screen shot 2017-04-12 at 4 09 18 pm

Screenshots for Add a New Network Router and Add a new Floating IP would be similar in the presense/absence of an OpenStack Network Manager

@AparnaKarve
Copy link
Contributor Author

@Ladas Please review.

I also need a confirmation on the following --

2.3.1 :009 > EmsNetwork.first
  ManageIQ::Providers::NetworkManager Load (2.4ms)  SELECT  "ext_management_systems".* FROM "ext_management_systems" WHERE "ext_management_systems"."type" IN ('ManageIQ::Providers::NetworkManager', 'ManageIQ::Providers::Google::NetworkManager', 'ManageIQ::Providers::Nuage::NetworkManager', 'ManageIQ::Providers::Openstack::NetworkManager', 'ManageIQ::Providers::Amazon::NetworkManager', 'ManageIQ::Providers::Azure::NetworkManager', 'ManageIQ::Providers::Vmware::NetworkManager') ORDER BY "ext_management_systems"."id" ASC LIMIT $1  [["LIMIT", 1]]
  ManageIQ::Providers::NetworkManager Inst Including Associations (0.2ms - 1rows)
 => #<ManageIQ::Providers::Openstack::NetworkManager id: 1000000000025, name: "OSP7 Packstack (10.39.167.117) Network Manager", created_on: "2017-04-11 19:24:28", updated_on: "2017-04-11 19:24:28", guid: "7b279290-1eec-11e7-a56e-b8e856429d10", zone_id: nil, type: "ManageIQ::Providers::Openstack::NetworkManager", api_version: nil, uid_ems: nil, host_default_vnc_port_start: nil, host_default_vnc_port_end: nil, provider_region: nil, last_refresh_error: nil, last_refresh_date: nil, provider_id: nil, realm: nil, tenant_id: nil, project: nil, parent_ems_id: 1000000000007, subscription: nil, last_metrics_error: nil, last_metrics_update_date: nil, last_metrics_success_date: nil, tenant_mapping_enabled: nil> 

Note that the EmsNetwork.first record is OpenStack.

2.3.1 :010 > NetworkRouter.class_by_ems(EmsNetwork.first).supports_create?
  ManageIQ::Providers::NetworkManager Load (1.5ms)  SELECT  "ext_management_systems".* FROM "ext_management_systems" WHERE "ext_management_systems"."type" IN ('ManageIQ::Providers::NetworkManager', 'ManageIQ::Providers::Google::NetworkManager', 'ManageIQ::Providers::Nuage::NetworkManager', 'ManageIQ::Providers::Openstack::NetworkManager', 'ManageIQ::Providers::Amazon::NetworkManager', 'ManageIQ::Providers::Azure::NetworkManager', 'ManageIQ::Providers::Vmware::NetworkManager') ORDER BY "ext_management_systems"."id" ASC LIMIT $1  [["LIMIT", 1]]
  ManageIQ::Providers::NetworkManager Inst Including Associations (0.2ms - 1rows)
 => false 

Does that seem right that supports_create? returns a false for NetworkRouter for OpenStack?

It returns a true for SecurityGroup and FloatingIP

@Ladas
Copy link
Contributor

Ladas commented Apr 12, 2017

@tzumainn could you verify which actions are supported on OpenStack?

@tzumainn
Copy link
Contributor

@Ladas This looks right to me - we support :create for all three of these.

@AparnaKarve
Copy link
Contributor Author

This looks right to me - we support :create for all three of these.

@tzumainn In that case, NetworkRouter does not seem to be working correctly like I have demonstrated above. It's returning false for supports_create?
Can you verify that? Thanks.

@tzumainn
Copy link
Contributor

@AparnaKarve Ah, sorry - it looks like the network router code hasn't been updated yet; it supports :create_network_router instead

@AparnaKarve AparnaKarve force-pushed the bz1440684_disable_network_crud_for_non_openstack branch from abf6fe8 to d0f2689 Compare April 12, 2017 23:52
@miq-bot
Copy link
Member

miq-bot commented Apr 12, 2017

Checked commits AparnaKarve/manageiq-ui-classic@a4dc1fa~...d0f2689 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
9 files checked, 0 offenses detected
Everything looks good. 🍰

@AparnaKarve
Copy link
Contributor Author

Thanks @tzumainn.
I'm now using supports_create_network_router? to enable/disable the Add action for Network Routers.

@mzazrivec mzazrivec added this to the Sprint 59 Ending Apr 24, 2017 milestone Apr 13, 2017
@mzazrivec mzazrivec merged commit 7cf860a into ManageIQ:master Apr 13, 2017
simaishi pushed a commit that referenced this pull request Apr 13, 2017
…rud_for_non_openstack

Disable CRUD for Network provider elements for non-OpenStack providers
(cherry picked from commit 7cf860a)

https://bugzilla.redhat.com/show_bug.cgi?id=1442150
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit b7e435bf8f6552f51369936b92b706d839047aae
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Thu Apr 13 10:34:15 2017 +0200

    Merge pull request #1007 from AparnaKarve/bz1440684_disable_network_crud_for_non_openstack
    
    Disable CRUD for Network provider elements for non-OpenStack providers
    (cherry picked from commit 7cf860a3d44c8fdd2d38d436b9e0310c170391c4)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1442150

@AparnaKarve
Copy link
Contributor Author

@miq-bot remove_label euwe/conflict

@AparnaKarve
Copy link
Contributor Author

EUWE PR: ManageIQ/manageiq#15077

@simaishi
Copy link
Contributor

Backported to Euwe via ManageIQ/manageiq#15077

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants