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

Enable Nuage network manager #33

Merged
merged 1 commit into from
Oct 31, 2017

Conversation

gberginc
Copy link
Contributor

Prior to this patch, the UI could not show the option to create new
network managers because these are typically created along with cloud
manager. Nuage is a dedicated SDN requiring ability to add the provider
manually.

The UI depends on the product.nuage setting to enable the network
providers which this patch adds directly into the Nuage.

@miq-bot assign @blomquisg

@miq-bot add_label enhancement

Prior to this patch, the UI could not show the option to create new
network managers because these are typically created along with cloud
manager. Nuage is a dedicated SDN requiring ability to add the provider
manually.

The UI depends on the `product.nuage` setting to enable the network
providers which this patch adds directly into the Nuage.

Signed-off-by: Gregor Berginc <[email protected]>
@gberginc gberginc force-pushed the enable_nuage_product branch from e57147f to 24ddb09 Compare October 30, 2017 21:35
@miq-bot
Copy link
Member

miq-bot commented Oct 30, 2017

Checked commit gberginc@24ddb09 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

@blomquisg blomquisg merged commit 419b771 into ManageIQ:master Oct 31, 2017
@blomquisg blomquisg added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 31, 2017
@djberg96
Copy link
Contributor

@blomquisg @gberginc IMO this PR should be reverted because it's causing an issue where it will always enable the UI edit/new menu. Instead we should rely on the SupportsFeatureMixin in conjunction with a modification to the UI code.

ManageIQ/manageiq-ui-classic#3976

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

@gberginc
Copy link
Contributor Author

If I recal correctly, this is indeed not enabled in downstream product, so this should not be a probem to revert.

I fully support the decision not to have Nuage-specific check in the UI.

@miha-plesko what do you think?

@miha-plesko
Copy link
Contributor

Will test tomorrow but I think the PR will prevent user from adding the first Nuage manager. I'm thinking if we should instead check for config/permissions.yaml on the UI (where we decide to show button or not) if it contains entry for Nuage so that user would only have to enable Nuage there?

@djberg96
Copy link
Contributor

@miha-plesko Pretty sure you're correct. I'm not familiar with config/permissions.yaml, but it sounds promising.

@miha-plesko
Copy link
Contributor

So there is another PR available to fix the issue: ManageIQ/manageiq-ui-classic#3979, I suggest we continue our discussion there.

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.

5 participants