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

Model relation "SecurityGroup belongs to NetworkRouter or CloudSubnet" #258

Merged
merged 1 commit into from
Aug 24, 2018

Conversation

miha-plesko
Copy link
Contributor

With this commit we add a foreign key on SecurityGroup to follow what Nuage needs. In Nuage, security group is something that only exists in a scope of either L3 domain (NetworkRouter) or L2 domain (CloudSubnet::L2).

image

MIQ is currently not able to model such relation yet, hence this PR.

@miq-bot add_label enhancement
@miq-bot assign @Fryguy

@Ladas @agrare looking forward to your opinion. Need for this was revealed when enhancing targeted refresh where we would need something like network_router.security_groups, but there is no such relation yet... Is schema already frozen for Hammer?

Copy link
Contributor

@Ladas Ladas left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@Ladas
Copy link
Contributor

Ladas commented Aug 20, 2018

@miq-bot assign @agrare

@miq-bot miq-bot assigned agrare and unassigned Fryguy Aug 20, 2018
@miha-plesko
Copy link
Contributor Author

@agrare are you okay with this?

@miha-plesko
Copy link
Contributor Author

Kindly ask either @Fryguy or @agrare to get this one in. Need to do a bunch of followup PRs, namely:

  • inform Rails about the two foreign keys in miq-core repo
  • update ui-classic to render relation
  • update Nuage targeted refresh that relies on this relation

And would't want to get too close to code-freeze.

@agrare
Copy link
Member

agrare commented Aug 24, 2018

@miha-plesko you don't need the parens for add_reference no matter what miq-bot says :)

With this commit we add a foreign key on SecurityGroup to
follow what Nuage needs. In Nuage, security group is something
that only exists in a scope of either L3 domain (NetworkRouter) or
L2 domain (CloudSubnet::L2).

MIQ is currently not able to model such relation yet, hence this PR.

Signed-off-by: Miha Pleško <miha.plesko@xlab.si>
@miha-plesko miha-plesko force-pushed the foreign-key-on-security-group branch from 3d80875 to 4bef54a Compare August 24, 2018 12:59
@miha-plesko
Copy link
Contributor Author

@agrare so no parens shall be.

@miq-bot
Copy link
Member

miq-bot commented Aug 24, 2018

Checked commit miha-plesko@4bef54a with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 2 offenses detected

db/migrate/20180817152200_add_network_router_id_to_security_group.rb

db/migrate/20180817152201_add_cloud_subnet_id_to_security_group.rb

@agrare agrare merged commit 06efa7f into ManageIQ:master Aug 24, 2018
@agrare agrare added this to the Sprint 93 Ending Aug 27, 2018 milestone Aug 24, 2018
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