You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/07/13 11:30:52 UTC

[GitHub] [cloudstack] mib1185 opened a new issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

mib1185 opened a new issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202


   <!--
   Verify first that your issue/request is not already reported on GitHub.
   Also test if the latest release and main branch are affected too.
   Always add information AFTER of these HTML comments, but no need to delete the comments.
   -->
   
   ##### ISSUE TYPE
   <!-- Pick one below and delete the rest -->
    * Bug Report
    * Improvement Request
   
   ##### COMPONENT NAME
   <!--
   Categorize the issue, e.g. API, VR, VPN, UI, etc.
   -->
   ~~~
   Server
   ~~~
   
   ##### CLOUDSTACK VERSION
   <!--
   New line separated list of affected versions, commit ID for issues on main branch.
   -->
   
   ~~~
   4.15.0
   ~~~
   
   ##### CONFIGURATION
   <!--
   Information about the configuration if relevant, e.g. basic network, advanced networking, etc.  N/A otherwise
   -->
   
   ##### OS / ENVIRONMENT
   <!--
   Information about the environment if relevant, N/A otherwise
   -->
   
   
   ##### SUMMARY
   <!-- Explain the problem/feature briefly -->
   As descripted in #5194, with #4040 changes were made, how the `broadcast_uri` for private VLAN network is build.
   This new approach is the more correct one, but does only apply to newly created networks (_networks created after upgrade to 4.15_), those already existent networks keep its old `broadcast_uri` which is not correct, since it always contains `-i` (_ex. `pvlan://63-i123`_) and those is declared as an isolated pvlan, even it was configured as community pvlan.
   
   In our case - since all pvlan are of type community - we simple updated all entries in DB with following SQL statement:
   ```sql
   UPDATE cloud.networks
   SET broadcast_uri = REPLACE(broadcast_uri, '-i', '-c')
   WHERE removed is null and broadcast_uri like '%pvlan%';
   ```
   
   But to be more generic, it is needed that the correct pvlan type is gathered from infrastructure (_ex. VMware portgroup_) and corresponding DB entry is proper updated.
   
   ##### STEPS TO REPRODUCE
   <!--
   For bugs, show exactly how to reproduce the problem, using a minimal test-case. Use Screenshots if accurate.
   
   For new features, show how the feature would be used.
   -->
   
   <!-- Paste example playbooks or commands between quotes below -->
   ~~~
   
   ~~~
   
   <!-- You can also paste gist.github.com links for larger files -->
   
   ##### EXPECTED RESULTS
   <!-- What did you expect to happen when running the steps above? -->
   
   ~~~
   
   ~~~
   
   ##### ACTUAL RESULTS
   <!-- What actually happened? -->
   
   <!-- Paste verbatim command output between quotes below -->
   ~~~
   
   ~~~
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DK101010 commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DK101010 commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-892406219


   @nvazquez when you merge this PR, The ingest will no longer work for all community networks with an older format. 
   
   @DaanHoogland How do you want check which pvlan's are community and  which isolated? I checked the network table and I couldn't see any infos to decide it.   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-881158186


   HI @DK101010 would you consider including this as part of your PR #5194?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-924620033


   Yes, will create a PR soon and update


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-885370657


   > sorry, That will leave people with disfunctional environments. -1
   
   Two reasons why I'm against adding anything to the upgrade path :
   1. It is a bad assumption that all pvlans created in vmware are community and correspondingly created as isolated in cloudstack. There could be admins who created isolated pvlans in vmware and cloudstack. Changing all isolated pvlans to community will break the correctly working functionality
   2. Pvlans are also created on Xen and KVM (implemented via OVS) and altering it in the db via an upgrade path will break their functionality
   
   That being said, any admin who feels that there is a mismatch between what is existing on vmware and cloudstack should manually update it


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-920712856


   > @rhtyd we have discussed some options to address this and found a potential issue on mixed environments with KVM and Vmware that can cause failures on the upgrade. Also, the discrepancy on DB can be solved by creating a new network with the correct type (previously all the pvlans contained the were isolated in DB as it was the only supported type) or may need manual update on DB only when its not intended to create a new network. Therefore, we are for adding a note in the documentation on how to proceed in this specific case. cc. @davidjumani
   
   +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani edited a comment on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani edited a comment on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-885370657


   > sorry, That will leave people with disfunctional environments. -1
   
   Two reasons why I'm against adding anything to the upgrade path :
   1. It is a bad assumption that all pvlans created in vmware are community and correspondingly created as isolated in cloudstack. There could be admins who created isolated pvlans in vmware and cloudstack. Changing all isolated pvlans to community will break the correctly working functionality
   2. Pvlans were created as community in vmware and correspondingly created as isolated in cloudstack (knowing that community wasn't supported at the time. That's kinda on the admin)
   3. Pvlans are also created on Xen and KVM (implemented via OVS) and altering it in the db via an upgrade path will break their functionality
   4. This update is probably a niche case, especially considering 2
   
   That being said, any admin who feels that there is a mismatch between what is existing on vmware and cloudstack should manually update it.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-884850858


   If I'm understanding this correctly, other existing pvlans (in vmware) were created as isolated in cloudstack as that's the only one that was supported at that time. If that's right, updating the isolated pvlans to community ones in the db via an upgrade path is not a good idea. Assuming there are isolated pvlans created in VMware and correctly created as isolated pvlans in cloudstack, a db upgrade would change them to community ones which is incorrect. Imo, since full-fledged pvlans is a new feature, the user will need to manually upgrade it, or allow editing the pvlan via the update API (which is risky as KVM supports pvlans as well, and updating it will require reapplying the appropriate ovs rules on all the hosts for every vm on that network)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-905227187


   Agree with @nvazquez


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-923723395


   @davidjumani have we agreed that you'll work on a doc PR instead? Thanks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] mib1185 commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
mib1185 commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-885253048


   > sorry, That will leave people with disfunctional environments. 👎
   
   This could be the case for every user, who has updated to 4.15.x, since approache, how `boadcast_uri` is build, was changed already with #4040 and does not depend on #5194.
   Therefore, yes to keep #5194 as is, but IMHO fix/update for existing private vlan networks should be implemented asap (_to become part of 4.15.2_) 🤔 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-905293066


   Agree with your suggestion @nvazquez, @davidjumani since you've worked in that area - please consider Nicolas's advice and work forward for the upgrade fix.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-885135622


   sorry, That will leave people with disfunctional environments. :-1: 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-881465165


   > Hi Daan, sure. The current implementation always expects an "i" in the broadcast uri and throws errors for community networks that are created in CS 4.15.0.0 or above. My PR add an "i" or a "c" depending on the pvlan type and would throws errors for community networks that are created in 4.14. and below.
   
   so we need to enhance #5194 to include a DB upgrade for old community pvlans! 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-890708536


   I'm for https://github.com/apache/cloudstack/pull/5194 but against any changes in a db upgrade path to fix existing pvlan uri


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DK101010 commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DK101010 commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-881325896


   > @DK101010
   > 
   > > Hi @nvazquez, this issue is needed for my PR in that case of existing community networks lower 4.15.0.0. But it could be necessary to add this as soon as possible to cloudstack because all already existing community networks in cloudstack are wrong. I can not estimate which impact it has for other CS functionality beside the ingest.
   > 
   > are you saying that that PR won't work without a DB upgrade for all URI fields? (bit worried here)
   
   Hi Daan, sure. The current implementation always expects an "i" in the broadcast uri and throws errors for community networks that are created in CS 4.15.0.0 or above. My PR add an "i" or a "c" depending on the pvlan type and would throws errors for community networks that are created in 4.14. and below. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-881285205


   @DK101010 
   
   > Hi @nvazquez, this issue is needed for my PR in that case of existing community networks lower 4.15.0.0. But it could be necessary to add this as soon as possible to cloudstack because all already existing community networks in cloudstack are wrong. I can not estimate which impact it has for other CS functionality beside the ingest.
   
   are you saying that that PR won't work without a DB upgrade for all URI fields? (bit worried here)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DK101010 commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DK101010 commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-902477693


   > @DK101010 Looking at the code history I see your point. The broadcast URI is stored incorrectly in the DB, although the network functions as a community PVLAN and can potentially cause issues for PVLANs created between 4.14 and 4.15. I'm +1 for a smart fix
   
   Hi @davidjumani not quite sure what do you mean with smart fix. I have currently no clue to find out which pvlans are isolated in which not. Therefore I think a database upgrade is not enough.      


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani edited a comment on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani edited a comment on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-885370657


   > sorry, That will leave people with disfunctional environments. -1
   
   Reasons why I'm against adding anything to the upgrade path :
   1. It is a bad assumption that all pvlans created in vmware are community and correspondingly created as isolated in cloudstack. There could be admins who created isolated pvlans in vmware and cloudstack. Changing all isolated pvlans to community will break the correctly working functionality
   2. Pvlans were created as community in vmware and correspondingly created as isolated in cloudstack (knowing that community wasn't supported at the time. That's kinda on the admin)
   3. Pvlans are also created on Xen and KVM (implemented via OVS) and altering it in the db via an upgrade path will break their functionality
   4. This update is probably a niche case, especially considering 2
   
   That being said, any admin who feels that there is a mismatch between what is existing on vmware and cloudstack should manually update it.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-881494498


   @DK101010 my idea is just what @DaanHoogland described. Including a DB upgrade only for older pvlans (created with the older format). New pvlans will have the new format and should work with your PR


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-920571774


   @rhtyd we have discussed some options to address this and found a potential issue on mixed environments with KVM and Vmware that can cause failures on the upgrade. Also, the discrepancy on DB can be solved by creating a new network with the correct type (previously all the pvlans contained the were isolated in DB as it was the only supported type) or may need manual update on DB only when its not intended to create a new network. Therefore, we are for adding a note in the documentation on how to proceed in this specific case. cc. @davidjumani 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-892446548


   @DK101010 the important question to me is: are there any installations conceivable that would have a pvlan with a 'c' in the broadcast URI that will after upgrade be expected to have an 'i' in the URI. If so we have a backwards compatibility issue on our hand and we need to deal with that.
   so
   What is the functional reason for the difference?
   if this is vital we'll need to deal during upgrade
   If no, I'd suggest removing the 'c' *and* the 'i' altogether.
   
   please explain, because I'm aware I am missing some knowledge here.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DK101010 commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DK101010 commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-893211651


   @DaanHoogland japp, I'm also the wrong person to answer this question. 
   
   But we are use this bugfix incl. uri update since 3 weeks in our prod VMware env. without problems. 
   
   Perhaps it helps to estimate the impact.  
   
      


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani edited a comment on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani edited a comment on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-885370657


   > sorry, That will leave people with disfunctional environments. -1
   
   Reasons why I'm against adding anything to the upgrade path :
   1. Pvlans are also created on Xen and KVM (implemented via OVS with different rules based on community / isolated / promiscuous) and altering it in the db via an upgrade path will break their functionality
   2. Pvlans were created as community in vmware and correspondingly created as isolated in cloudstack (knowing that community wasn't supported at the time. That's kinda on the admin)
   3. This update is probably a niche case, especially considering 2
   
   That being said, any admin who feels that there is a mismatch between what is existing on vmware and cloudstack should manually update it.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DK101010 commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DK101010 commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-881208327


   > HI @DK101010 would you consider including this as part of your PR #5194?
   
   Hi @nvazquez, this issue is needed for my PR in that case of existing community networks lower 4.15.0.0. But it could be necessary to add this as soon as possible to cloudstack because all already existing community networks in cloudstack are wrong. I can not estimate which impact it has for other CS functionality beside the ingest.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-891218055


   I'm also for #5194. If I understand correctly the PR is extending the importing of unmanaged instances to all the pvlan types. IMO that may not mean to upgrade the broadcast URI for previously created pvlans, as those pvlans were created with the pvlan type which was available at that time.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-905108905


   I think the fix would include checking the pvlans on the hypervisor side and compare the pvlan type matches the type on the DB entry for broadcast_uri. In case there is a mismatch, the broadcast_uri must be updated. @rhtyd can you discuss with @davidjumani and work on a way forward for the upgrade fix?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-890817659


   So the upgrade can check and change only pvlans specific to vmware hosts, can it @davidjumani , or would that give more issues? cc @nvazquez @DK101010 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez closed issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
nvazquez closed issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-890657968


   @mib1185 @davidjumani I agree - however, we had a different opinion from @DaanHoogland, after the latest replies are you still against moving on #5194 as it is now?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez closed issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
nvazquez closed issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-900124280


   @DK101010 Looking at the code history I see your point. The broadcast URI is stored incorrectly in the DB, although the network functions as a community PVLAN and can potentially cause issues for PVLANs created between 4.14 and 4.15. I'm +1 for a smart fix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-884864072


   The logic to upgrade older pvlans should be smarter than a DB upgrade. I would propose to proceed with the PR #5194 without any DB upgrade, leaving this ticket open for future fix @davidjumani @DaanHoogland @mib1185 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DK101010 commented on issue #5202: Addition to #5194 - update of `broadcast_uri` for all existing pvlan networks is necessary during upgrade to 4.15.x

Posted by GitBox <gi...@apache.org>.
DK101010 commented on issue #5202:
URL: https://github.com/apache/cloudstack/issues/5202#issuecomment-893211651


   @DaanHoogland japp, I'm also the wrong person to answer this question. 
   
   But we are use this bugfix incl. uri update since 3 weeks in our prod VMware env. without problems. 
   
   Perhaps it helps to estimate the impact.  
   
      


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org