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

Fix single security group in provisioning #17094

Merged
merged 1 commit into from
Mar 7, 2018

Conversation

alexander-demicev
Copy link

This PR fixes issue with selecting security groups in provisioning. Security groups are not required to perform provisioning and it is not possible to leave field empty when there was only one security group in list. Requires restarting memcached and running bin/update.
screen shot 2018-03-06 at 16 59 40
ManageIQ/manageiq-providers-openstack#178

@alexander-demicev
Copy link
Author

ping @aufi @skateman @agrare

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Seal of Approval
Note for testing: you need a bin/update

@agrare agrare self-assigned this Mar 7, 2018
@agrare agrare requested review from aufi and AlexanderZagaynov March 7, 2018 13:46
@agrare
Copy link
Member

agrare commented Mar 7, 2018

@AlexanderZagaynov can you confirm for Amazon and @aufi for OpenStack?

@AlexanderZagaynov
Copy link
Contributor

It's impossible to go without security group in AWS: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-network-security.html#default-security-group

@alexander-demicev
Copy link
Author

@AlexanderZagaynov @agrare Ok, I will remove it then, but there is still :required: false in yaml.

@miq-bot
Copy link
Member

miq-bot commented Mar 7, 2018

Checked commit alexander-demicev@500726e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@skateman
Copy link
Member

skateman commented Mar 7, 2018

@alexander-demichev I think you should set the required field too then, maybe in a separate PR...ask the others' opinion 😉

@agrare
Copy link
Member

agrare commented Mar 7, 2018

Yeah I agree @skateman ... @AlexanderZagaynov if we should be setting requred: true for amazon can you post a PR to do that?

Copy link
Member

@aufi aufi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works for me 👍

OpenStack doesn't require a Security Group for provisioning Instance - https://developer.openstack.org/api-ref/compute/#create-server

@agrare agrare merged commit fe51a01 into ManageIQ:master Mar 7, 2018
@agrare agrare added this to the Sprint 81 Ending Mar 12, 2018 milestone Mar 7, 2018
@alexander-demicev alexander-demicev deleted the fix-security-groups branch March 7, 2018 15:56
@agrare
Copy link
Member

agrare commented Mar 7, 2018

Thanks @alexander-demichev !

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