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/08 13:52:35 UTC
[GitHub] [cloudstack] DK101010 opened a new pull request #5194: adapt condition to use the correct letter for pvlan types
DK101010 opened a new pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194
### Description
In the past, the broadcast uri was changed to an "i" for isolated, as well as c for community.
see https://github.com/apache/cloudstack/pull/4040
However, the ingest logic has not been adjusted, which means it's no longer possible to ingest a VM with a community
network.
For this reason I've adapted the condition and add correct pvlan type.
### **Important**
**All existing community network broadcast uri's in cloudstack before 4.15.1.0 must be updated manually or new created
because all community networks contains the letter i in the broadcast uri.**
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [x] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
Tested in own VMware env.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-883005801
@DK101010 can you please include a DB upgrade for older pvlans as mentioned in #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] shwstppr commented on a change in pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#discussion_r667642172
##########
File path: server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
##########
@@ -617,8 +617,8 @@ private void checkUnmanagedNicAndNetworkForImport(UnmanagedInstanceTO.Nic nic, N
}
if (nic.getVlan() != null && nic.getVlan() != 0 && nic.getPvlan() != null && nic.getPvlan() != 0 &&
(Strings.isNullOrEmpty(network.getBroadcastUri().toString()) ||
- !networkBroadcastUri.equals(String.format("pvlan://%d-i%d", nic.getVlan(), nic.getPvlan())))) {
- throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("PVLAN of network(ID: %s) %s is found different from the VLAN of nic(ID: %s) pvlan://%d-i%d during VM import", network.getUuid(), networkBroadcastUri, nic.getNicId(), nic.getVlan(), nic.getPvlan()));
+ !networkBroadcastUri.equals(String.format("pvlan://%d-%s%d", nic.getVlan(), nic.getPvlanType().charAt(0), nic.getPvlan())))) {
Review comment:
@DK101010 will case of pvlan type matter? Does `nic.getPvlanType()` always return lowercase string?
--
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 a change in pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#discussion_r668595190
##########
File path: server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
##########
@@ -615,10 +615,11 @@ private void checkUnmanagedNicAndNetworkForImport(UnmanagedInstanceTO.Nic nic, N
!networkBroadcastUri.equals(String.format("vlan://%d", nic.getVlan())))) {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("VLAN of network(ID: %s) %s is found different from the VLAN of nic(ID: %s) vlan://%d during VM import", network.getUuid(), networkBroadcastUri, nic.getNicId(), nic.getVlan()));
}
+ char pvLanType = nic.getPvlanType() == null ? '\0' : nic.getPvlanType().toLowerCase().charAt(0);
Review comment:
```suggestion
String pvLanType = nic.getPvlanType() == null ? "" : new String(nic.getPvlanType().toLowerCase().charAt(0));
```
i don't trust placing a null-char in the middle of a string. maybe I should google a bit for this, but I'd rather insert a string on %s
--
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] shwstppr commented on a change in pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#discussion_r669469938
##########
File path: server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
##########
@@ -615,10 +615,11 @@ private void checkUnmanagedNicAndNetworkForImport(UnmanagedInstanceTO.Nic nic, N
!networkBroadcastUri.equals(String.format("vlan://%d", nic.getVlan())))) {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("VLAN of network(ID: %s) %s is found different from the VLAN of nic(ID: %s) vlan://%d during VM import", network.getUuid(), networkBroadcastUri, nic.getNicId(), nic.getVlan()));
}
+ String pvLanType = nic.getPvlanType() == null ? "" : new String(nic.getPvlanType().toLowerCase().charAt(0));
Review comment:
Build failing,
```suggestion
String pvLanType = nic.getPvlanType() == null ? "" : nic.getPvlanType().toLowerCase().substring(0, 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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-880490287
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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 a change in pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#discussion_r667654618
##########
File path: server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
##########
@@ -617,8 +617,8 @@ private void checkUnmanagedNicAndNetworkForImport(UnmanagedInstanceTO.Nic nic, N
}
if (nic.getVlan() != null && nic.getVlan() != 0 && nic.getPvlan() != null && nic.getPvlan() != 0 &&
(Strings.isNullOrEmpty(network.getBroadcastUri().toString()) ||
- !networkBroadcastUri.equals(String.format("pvlan://%d-i%d", nic.getVlan(), nic.getPvlan())))) {
- throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("PVLAN of network(ID: %s) %s is found different from the VLAN of nic(ID: %s) pvlan://%d-i%d during VM import", network.getUuid(), networkBroadcastUri, nic.getNicId(), nic.getVlan(), nic.getPvlan()));
+ !networkBroadcastUri.equals(String.format("pvlan://%d-%s%d", nic.getVlan(), nic.getPvlanType().charAt(0), nic.getPvlan())))) {
Review comment:
Hi @shwstppr yes, the types community and isolated are lowercase. I'm not quite sure if this will continue in the future. But I also didn't want to create pointless source code. That's why I left out for 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] DK101010 commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
DK101010 commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-883921362
> @DK101010 can you please include a DB upgrade for older pvlans as mentioned in #5202?
Hi @nvazquez I have currently no clue how the cs upgrade works and have a couple of other stuff to do. Therefore I'm not sure if I get finished this until 1.15.2 release.
--
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 edited a comment on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez edited a comment on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-884116562
@DK101010 when upgrading from version X to Y, CloudStack will apply all the upgrade scripts from versions between X and Y (e.g. from 4.15.0 to 4.16.0 has to apply DB upgrades from intermediate version 4.15.1)
The upgrade scripts are executed in this way:
- Prepare scripts: `schema-XtoY.sql` where X and Y are versions
- Data migration scripts (if any, provided in the java classes `UpgradeXtoY.java`)
- Cleanup scripts: `schema-XtoY-cleanup.sql`
In short, as this PR is going into 4.16 you can place the DB update SQL for older PVLANs in the file `schema-41510to41600.sql` (some examples in the same file: https://github.com/apache/cloudstack/blob/main/engine/schema/src/main/resources/META-INF/db/schema-41510to41600.sql#L396-L407)
Happy to help, please ping me if you need help with that
P.S: on the issue #5202 the SQL update is proposed:
````
UPDATE cloud.networks
SET broadcast_uri = REPLACE(broadcast_uri, '-i', '-c')
WHERE removed is null and broadcast_uri like '%pvlan%';
````
--
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 merged pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez merged pull request #5194:
URL: https://github.com/apache/cloudstack/pull/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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-902290666
<b>Trillian test result (tid-1723)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38084 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5194-t1723-vmware-67u3.zip
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-901612612
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-902634847
Ping @davidjumani can you review this - I think you worked on pvlan related feature so you may have some ideas to review/test this? cc @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] nvazquez commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-883005801
@DK101010 can you please include a DB upgrade for older pvlans as mentioned in #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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-884116562
@DK101010 when upgrading from version X to Y, CloudStack will apply all the upgrade scripts from versions between X and Y (e.g. from 4.15.0 to 4.16.0 has to apply DB upgrades from intermediate version 4.15.1)
The upgrade scripts are executed in this way:
- Prepare scripts: `schema-XtoY.sql` where X and Y are versions
- Data migration scripts (if any, provided in the java classes `UpgradeXtoY.java`)
- Cleanup scripts: 'schema-XtoY-cleanup.sql`
In short, as this PR in going into 4.16 you can place the DB update SQL for older PVLANs in the file `schema-41510to41600.sql` (some examples in the same file: https://github.com/apache/cloudstack/blob/main/engine/schema/src/main/resources/META-INF/db/schema-41510to41600.sql#L396-L407)
Happy to help, please ping me if you need help with that
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
mib1185 commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-884461329
> P.S: on the issue #5202 the SQL update is proposed:
>
> ```
> UPDATE cloud.networks
> SET broadcast_uri = REPLACE(broadcast_uri, '-i', '-c')
> WHERE removed is null and broadcast_uri like '%pvlan%';
> ```
@nvazquez Please don't oversee the remarks before and after the SQL in #5202:
> In our case - since all pvlan are of type community - we simple updated all entries in DB with following SQL statement:
> 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.
--
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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-880541764
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 554
--
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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-901835077
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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] shwstppr commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-879721404
@blueorangutan package
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-895046107
@nvazquez @shwstppr @davidjumani - can you discuss and check with @DK101010 if it's possible to support backward compatibility or automatic fix post-upgrade?
--
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 a change in pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#discussion_r668533864
##########
File path: server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
##########
@@ -617,8 +617,8 @@ private void checkUnmanagedNicAndNetworkForImport(UnmanagedInstanceTO.Nic nic, N
}
if (nic.getVlan() != null && nic.getVlan() != 0 && nic.getPvlan() != null && nic.getPvlan() != 0 &&
(Strings.isNullOrEmpty(network.getBroadcastUri().toString()) ||
- !networkBroadcastUri.equals(String.format("pvlan://%d-i%d", nic.getVlan(), nic.getPvlan())))) {
- throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("PVLAN of network(ID: %s) %s is found different from the VLAN of nic(ID: %s) pvlan://%d-i%d during VM import", network.getUuid(), networkBroadcastUri, nic.getNicId(), nic.getVlan(), nic.getPvlan()));
+ !networkBroadcastUri.equals(String.format("pvlan://%d-%s%d", nic.getVlan(), nic.getPvlanType().charAt(0), nic.getPvlan())))) {
Review comment:
living dangerously, huh? as it is used twice anyway, better prepare it in a var above.
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-901834897
@blueorangutan test centos7 vmware-67u3
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-884524905
Thanks @mib1185 good remarks, I'll check further 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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-880505656
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 553
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
DK101010 commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-880430878
> Other than the compilation issue, code changes are correct based on the premise that broadcast URI for PVLAN will be based on its type and the first character of the type string.
>
> @DK101010 since we do not have any smoke to test VM import, it will be great if you can add API call/output for testing this change
Do you want this ?
``` console
import unmanagedinstance clusterid=b6dc2477-f3ec-43e2-a405-3605e29aaa42 name=VM-ac60ee4e-7231-4057-8ff6-40f0c9eacd04 displayname=VM-ac60ee4e-7231-4057-8ff6-40f0c9eacd04 serviceofferingid=35d68014-0257-4775-b256-49a009aa5750
````
```JSON
{
"accountid": "226c8611-ff24-11ea-9d07-005056827a97",
"cmd": "org.apache.cloudstack.api.command.admin.vm.ImportUnmanagedInstanceCmd",
"completed": "2021-07-15T07:18:39+0100",
"created": "2021-07-15T07:18:36+0100",
"jobid": "c07d365c-7057-4124-bf6b-ce4efbe2ca1b",
"jobprocstatus": 0,
"jobresult": {
"virtualmachine": {
"account": "admin",
"affinitygroup": [],
"cpunumber": 1,
"cpuspeed": 0,
"created": "2021-07-15T07:18:38+0100",
"details": {
"dataDiskController": "osdefault",
"deployvm": "true",
"nicAdapter": "E1000",
"rootDiskController": "pvscsi"
},
"displayname": "VM-ac60ee4e-7231-4057-8ff6-40f0c9eacd04",
"displayvm": true,
"domain": "ROOT",
"domainid": "226c46c2-ff24-11ea-9d07-005056827a97",
"guestosid": "224af937-ff24-11ea-9d07-005056827a97",
"haenable": false,
"hostid": "73676017-b962-4942-bb4c-cf297634b212",
"hostname": "iosvemt01.esx-tst.os.itelligence.de",
"hypervisor": "VMware",
"id": "93309b0b-8af7-4ad2-85ea-a7e492b9af2e",
"instancename": "VM-ac60ee4e-7231-4057-8ff6-40f0c9eacd04",
"isdynamicallyscalable": false,
"memory": 1024,
"name": "VM-ac60ee4e-7231-4057-8ff6-40f0c9eacd04",
"nic": [
{
"broadcasturi": "vlan://2",
"extradhcpoption": [],
"id": "3d4d8f5e-2ce6-4575-91c8-9d367520d97c",
"isdefault": true,
"isolationuri": "vlan://2",
"macaddress": "02:00:07:e5:00:60",
"networkid": "85d27cc3-9184-4d7f-87c4-e5b08b2eb2cc",
"networkname": "Guest",
"secondaryip": [],
"traffictype": "Guest",
"type": "L2"
}
],
"osdisplayname": "Other Ubuntu (64-bit)",
"ostypeid": "224af937-ff24-11ea-9d07-005056827a97",
"passwordenabled": false,
"rootdeviceid": 0,
"rootdevicetype": "ROOT",
"securitygroup": [],
"serviceofferingid": "35d68014-0257-4775-b256-49a009aa5750",
"serviceofferingname": "testCPU0Mhz",
"state": "Running",
"tags": [],
"templatedisplaytext": "VM Import Default Template",
"templateid": "59faead6-743e-442f-a964-ffb31da3e785",
"templatename": "system-default-vm-import-dummy-template.iso",
"userid": "22712228-ff24-11ea-9d07-005056827a97",
"username": "admin",
"zoneid": "de1c4b6f-a58b-46e6-9418-1fd55a9f022c",
"zonename": "eu-de-vct5"
}
},
"jobresultcode": 0,
"jobresulttype": "object",
"jobstatus": 1,
"userid": "22712228-ff24-11ea-9d07-005056827a97"
}
```
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-902668248
@rhtyd as discussed in #5202 this PR is good to go, but needs a separate PR to address the upgrade for older pvlans (DB upgrade will not work)
--
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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-880517932
@DK101010 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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] shwstppr commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-880489758
@blueorangutan package
@DK101010 yes, will be great to have a list and import API call/output added in the PR's description. Though I feel the test you added above is not for PVLAN broadcast URI type.
--
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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-879726127
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian. SL-JID 544
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-901612151
@blueorangutan package
--
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 edited a comment on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez edited a comment on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-884116562
@DK101010 when upgrading from version X to Y, CloudStack will apply all the upgrade scripts from versions between X and Y (e.g. from 4.15.0 to 4.16.0 has to apply DB upgrades from intermediate version 4.15.1)
The upgrade scripts are executed in this way:
- Prepare scripts: `schema-XtoY.sql` where X and Y are versions
- Data migration scripts (if any, provided in the java classes `UpgradeXtoY.java`)
- Cleanup scripts: `schema-XtoY-cleanup.sql`
In short, as this PR in going into 4.16 you can place the DB update SQL for older PVLANs in the file `schema-41510to41600.sql` (some examples in the same file: https://github.com/apache/cloudstack/blob/main/engine/schema/src/main/resources/META-INF/db/schema-41510to41600.sql#L396-L407)
Happy to help, please ping me if you need help with that
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-883005801
@DK101010 can you please include a DB upgrade for older pvlans as mentioned in #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] DK101010 commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
DK101010 commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-880517541
> @blueorangutan package
>
> @DK101010 yes, will be great to have a list and import API call/output added in the PR's description. Though I feel the test you added above is not for PVLAN broadcast URI type.
@shwstppr you are completely right, wrong vm
```
import unmanagedinstance clusterid=b6dc2477-f3ec-43e2-a405-3605e29aaa42 name=ubuvtst6 displayname=ubuvtst6 projectid=fc61ebe1-6290-4100-98ca-18fb3d950af2 serviceofferingid=15b5681f-08c2-417a-a9ba-4480c229ec03
```
```json
{
"accountid": "226c8611-ff24-11ea-9d07-005056827a97",
"cmd": "org.apache.cloudstack.api.command.admin.vm.ImportUnmanagedInstanceCmd",
"completed": "2021-07-15T09:42:19+0100",
"created": "2021-07-15T09:42:05+0100",
"jobid": "189390c5-6401-4e62-830f-7ba2b96c174f",
"jobprocstatus": 0,
"jobresult": {
"virtualmachine": {
"affinitygroup": [],
"cpunumber": 1,
"cpuspeed": 0,
"created": "2021-07-15T09:42:19+0100",
"details": {
"cpuNumber": "1",
"dataDiskController": "osdefault",
"deployvm": "true",
"memory": "1024",
"nicAdapter": "Vmxnet3",
"rootDiskController": "pvscsi"
},
"displayname": "ubuvtst6",
"displayvm": true,
"domain": "itelligence",
"domainid": "05dfd7d7-5b11-4f3e-94e4-9569c95f75ee",
"guestosid": "224af937-ff24-11ea-9d07-005056827a97",
"haenable": false,
"hostid": "73676017-b962-4942-bb4c-cf297634b212",
"hostname": "iosvemt01.esx-tst.os.itelligence.de",
"hypervisor": "VMware",
"id": "c13c12de-aa87-413c-8fe3-895a7094af60",
"instancename": "ubuvtst6",
"isdynamicallyscalable": false,
"memory": 1024,
"name": "ubuvtst6",
"nic": [
{
"broadcasturi": "vlan://289",
"extradhcpoption": [],
"id": "c5f110b3-cdc1-492d-be66-c313861942df",
"isdefault": true,
"isolationuri": "vlan://289",
"macaddress": "00:50:56:92:12:41",
"networkid": "c0816fba-bac0-46a7-b749-cc2c36e97f60",
"networkname": "Frontend_289",
"secondaryip": [],
"traffictype": "Guest",
"type": "L2"
},
{
"broadcasturi": "pvlan://63-c289",
"extradhcpoption": [],
"id": "46820c91-c310-4b1e-9383-7a8036e97a8f",
"isdefault": false,
"isolationuri": "pvlan://63-c289",
"macaddress": "02:00:39:f3:00:01",
"networkid": "baad9713-c8a8-4f5f-8ea5-546190db4306",
"networkname": "Backup_63-289",
"secondaryip": [],
"traffictype": "Guest",
"type": "L2"
}
],
"osdisplayname": "Other Ubuntu (64-bit)",
"ostypeid": "224af937-ff24-11ea-9d07-005056827a97",
"passwordenabled": false,
"project": "dev-01",
"projectid": "fc61ebe1-6290-4100-98ca-18fb3d950af2",
"rootdeviceid": 0,
"rootdevicetype": "ROOT",
"securitygroup": [],
"serviceofferingid": "15b5681f-08c2-417a-a9ba-4480c229ec03",
"serviceofferingname": "custom_std-ha-std_fat-fp-std",
"state": "Running",
"tags": [],
"templatedisplaytext": "VM Import Default Template",
"templateid": "59faead6-743e-442f-a964-ffb31da3e785",
"templatename": "system-default-vm-import-dummy-template.iso",
"userid": "22712228-ff24-11ea-9d07-005056827a97",
"username": "admin",
"zoneid": "de1c4b6f-a58b-46e6-9418-1fd55a9f022c",
"zonename": "eu-de-vct5"
}
},
"jobresultcode": 0,
"jobresulttype": "object",
"jobstatus": 1,
"userid": "22712228-ff24-11ea-9d07-005056827a97"
}
```
--
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 edited a comment on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
mib1185 edited a comment on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-884461329
> P.S: on the issue #5202 the SQL update is proposed:
>
> ```
> UPDATE cloud.networks
> SET broadcast_uri = REPLACE(broadcast_uri, '-i', '-c')
> WHERE removed is null and broadcast_uri like '%pvlan%';
> ```
@nvazquez Please don't oversee the remarks before and after the SQL in #5202:
> In our case - since all pvlan are of type community - we simple updated all entries in DB with following SQL statement:
> 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.
Those I guess it is unfortunately not done by a simple sql update script :thinking:
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-900122650
@rhtyd That's being tracked here 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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-901624113
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 926
--
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 pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-891539102
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] mib1185 commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
mib1185 commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-884528649
There can only be one isolated secondary vlan per each primary vlan, but multiple community secondary vlan.
Furthermore, promiscuous typed networks needs also to be checked 🤔
--
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] blueorangutan commented on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-879722032
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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 edited a comment on pull request #5194: adapt condition to use the correct letter for pvlan types
Posted by GitBox <gi...@apache.org>.
nvazquez edited a comment on pull request #5194:
URL: https://github.com/apache/cloudstack/pull/5194#issuecomment-884116562
@DK101010 when upgrading from version X to Y, CloudStack will apply all the upgrade scripts from versions between X and Y (e.g. from 4.15.0 to 4.16.0 has to apply DB upgrades from intermediate version 4.15.1)
The upgrade scripts are executed in this way:
- Prepare scripts: `schema-XtoY.sql` where X and Y are versions
- Data migration scripts (if any, provided in the java classes `UpgradeXtoY.java`)
- Cleanup scripts: `schema-XtoY-cleanup.sql`
In short, as this PR is going into 4.16 you can place the DB update SQL for older PVLANs in the file `schema-41510to41600.sql` (some examples in the same file: https://github.com/apache/cloudstack/blob/main/engine/schema/src/main/resources/META-INF/db/schema-41510to41600.sql#L396-L407)
Happy to help, please ping me if you need help with that
--
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