Adding pvlan option for l2 and fixing passing none for no pvlan type#373
Conversation
|
@borisstoyanov @vladimirpetrov - please ping me once you confirm testing the feature |
|
@rhtyd a Jenkins job has been kicked to build primate packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️centos ✔️debian ✔️archive. JID-1959 |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build primate packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️centos ✔️debian ✔️archive. |
|
Does it cover all cases for apache/cloudstack#3732 ? @davidjumani ? |
|
@rhtyd Yes it does! |
|
@blueorangutan test |
shwstppr
left a comment
There was a problem hiding this comment.
Minor code refactoring comments, functionality LGTM
| initialValue: this.isolatePvlanType | ||
| }]" | ||
| buttonStyle="solid" | ||
| @change="selected => { this.handleIsolatedPvlanTypeChange(selected.target.value) }"> |
There was a problem hiding this comment.
minor: can be simplified
| @change="selected => { this.handleIsolatedPvlanTypeChange(selected.target.value) }"> | |
| @change="selected => { this.isolatePvlanType = selected.target.value }"> |
| handleIsolatedPvlanTypeChange (pvlan) { | ||
| this.isolatePvlanType = pvlan | ||
| }, |
There was a problem hiding this comment.
| handleIsolatedPvlanTypeChange (pvlan) { | |
| this.isolatePvlanType = pvlan | |
| }, |
| if (this.isValidValueForKey(values, 'isolatedpvlan')) { | ||
| params.isolatedpvlan = values.isolatedpvlan | ||
| } |
There was a problem hiding this comment.
Not sure if this really needs to go inside isolatedpvlantype if block. Anyway we are not showing isolatedpvlan field for none type
There was a problem hiding this comment.
so we're verifying it only if the pvlan type is not none
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build primate packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️centos ✔️debian ✔️archive. |
|
@blueorangutan package |
|
@davidjumani a Jenkins job has been kicked to build primate packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️centos ✔️debian ✔️archive. |
|
@blueorangutan package |
|
@davidjumani a Jenkins job has been kicked to build primate packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️centos ✔️debian ✔️archive. |
01caf67 to
d221b86
Compare
|
@blueorangutan package |
|
@davidjumani a Jenkins job has been kicked to build primate packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️centos ✔️debian ✔️archive. |
|
@blueorangutan package |
|
@davidjumani can you check and fix label on L2 form? (label.promiscuous) |
d221b86 to
24fef94
Compare
|
@blueorangutan package |
|
@davidjumani a Jenkins job has been kicked to build primate packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️centos ✔️debian ✔️archive. |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build primate packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️centos ✔️debian ✔️archive. |
| networkOfferingLoading: false, | ||
| selectedNetworkOffering: {}, | ||
| accountVisible: this.isAdminOrDomainAdmin() | ||
| accountVisible: this.isAdminOrDomainAdmin(), |
There was a problem hiding this comment.
@davidjumani did you check against legacy UI if we should show pvlan options to domain admins?
yadvr
left a comment
There was a problem hiding this comment.
LGTM, tested and compared legacy UI form with this as root admin, domain admin and user
…lan type (#373) This adds the option to create PVLAN for L2 networks, as well as fixes the issue caused by passing 'none' as the PVLAN type when no secondary VLAN type is selected.
This adds the option to create PVLAN for L2 networks, as well as fixes the issue caused by passing 'none' as the PVLAN type when no secondary VLAN type is selected