search

openshift / openshift-sdn

Apache License 2.0
69 stars 63 forks source link

[DO_NOT_MERGE] Refactor VNID allocator #210

Closed pravisankar closed 8 years ago

pravisankar commented 8 years ago
pravisankar commented 8 years ago

@openshift/networking PTAL

danwinship commented 8 years ago

Is there a corresponding origin branch? It seems like this removes a bunch of code and doesn't replace it with anything...

pravisankar commented 8 years ago

Updated

pravisankar commented 8 years ago

@openshift/ui-review commit 3baa342 adds CLI command to support network isolation for projects (undo merged project networks).

fabianofranz commented 8 years ago

CLI bits LGTM

pravisankar commented 8 years ago

@openshift/networking Updated, ptal

danwinship commented 8 years ago

vnidallocator doesn't get used by anything else in openshift-sdn... could it be moved to origin instead? Then all the code would be in one place, which seems like a win.

pravisankar commented 8 years ago

Yes, vnidallocator is only used in origin. It makes sense to move the code to origin.

dcbw commented 8 years ago

I'm wondering how this would all work out when splitting all the plugin stuff out of openshift[-sdn]. The VNID allocator would go into our future Kube network provider, I guess?

pravisankar commented 8 years ago

Moved VNID allocator related code to origin. This also eliminated unnecessary copy of some k8s files under Godeps which were needed for the test cases.

pravisankar commented 8 years ago

@danwinship @dcbw rebased, ptal

dcbw commented 8 years ago

LGTM assuming the origin bits get OKd.

pravisankar commented 8 years ago

Rebased, resolved minor conflicts.

danwinship commented 8 years ago

LGTM. Fine to merge once someone approves the origin side to go. (ie, let's not land this until we know it's not going to need more changes to address comments on the origin PR)

pravisankar commented 8 years ago

Closing this PR in favor of https://github.com/openshift/openshift-sdn/pull/303