Possibility to choose the source NAT IP address on a isolated network or VPC by DaanHoogland · Pull Request #6442 · apache/cloudstack

DaanHoogland · 2022-06-07T13:44:09Z

Description

This PR intends to allow users to choose an IP address for the VR of their network from the pool of available addresses.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
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

Screenshots (if appropriate):

How Has This Been Tested?

codecov · 2022-08-29T15:29:31Z

Codecov Report

Merging #6442 (6dbcee2) into main (941cc83) will increase coverage by 0.32%.
The diff coverage is 32.46%.

@@             Coverage Diff              @@
##               main    #6442      +/-   ##
============================================
+ Coverage     12.69%   13.01%   +0.32%     
- Complexity     8674     9172     +498     
============================================
  Files          2729     2728       -1     
  Lines        256579   262916    +6337     
  Branches      39987    42677    +2690     
============================================
+ Hits          32575    34222    +1647     
- Misses       219859   224397    +4538     
- Partials       4145     4297     +152

Impacted Files	Coverage Δ
.../main/java/com/cloud/network/IpAddressManager.java	`100.00% <ø> (ø)`
...n/java/com/cloud/network/dao/IPAddressDaoImpl.java	`31.22% <0.00%> (ø)`
...src/main/java/com/cloud/api/ApiResponseHelper.java	`4.96% <0.00%> (+1.13%)`	⬆️
...com/cloud/network/NetworkMigrationManagerImpl.java	`0.28% <ø> (ø)`
.../cloud/configuration/ConfigurationManagerImpl.java	`16.10% <5.55%> (+0.85%)`	⬆️
...ain/java/com/cloud/network/vpc/VpcManagerImpl.java	`9.37% <25.00%> (+0.70%)`	⬆️
...ain/java/com/cloud/network/NetworkServiceImpl.java	`12.91% <38.21%> (+1.50%)`	⬆️
...n/java/com/cloud/network/IpAddressManagerImpl.java	`2.90% <100.00%> (+0.77%)`	⬆️

... and 144 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

blueorangutan · 2023-05-02T10:04:33Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6010

blueorangutan · 2023-05-02T11:53:01Z

Trillian Build Failed (tid-6494)

blueorangutan · 2023-05-02T12:54:50Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6012

blueorangutan · 2023-05-02T15:58:36Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6013

.github/workflows/ci.yml

api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java

kiranchavala · 2023-05-03T09:52:35Z

@blueorangutan package

blueorangutan · 2023-05-03T09:54:03Z

@kiranchavala a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2023-05-03T10:48:13Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6017

blueorangutan · 2023-05-08T11:00:57Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6042

blueorangutan · 2023-05-08T13:13:53Z

Trillian Build Failed (tid-6520)

kiranchavala

LGTM, tested the feature and all the scenarios

Configurable SNAT IP for VR (test cases).xlsx

kiranchavala · 2023-06-01T10:32:59Z

@blueorangutan package

blueorangutan · 2023-06-01T10:34:03Z

@kiranchavala a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2023-06-01T11:39:12Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6197

DaanHoogland · 2023-06-06T11:01:04Z

@weizhouapache @GutoVeronezi @alexandremattioli , I am assuming your comments have all been addressed?

weizhouapache · 2023-06-06T11:31:22Z

@weizhouapache @GutoVeronezi @alexandremattioli , I am assuming your comments have all been addressed?

@DaanHoogland
lgtm

DaanHoogland · 2023-06-09T11:10:10Z

@blueorangutan test

blueorangutan · 2023-06-09T11:12:04Z

@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

weizhouapache · 2023-06-09T17:47:36Z

@DaanHoogland
I tested this feature, overall it looks nice !

two issues

update network with a public IP did nothing, if the IP is not acquired before. Would be good to display an error message ?
the network router is destroyed and then recreated, which caused 43 seconds downtime (static nat). this can be improved I think.

DaanHoogland · 2023-06-09T19:36:44Z

update network with a public IP did nothing, if the IP is not acquired before. Would be good to display an error message ?

that seems a bug. It should throw an exception in this case.

blueorangutan · 2023-06-09T23:36:37Z

[SF] Trillian test result (tid-6713)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43176 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6442-t6713-kvm-centos7.zip
Smoke tests completed. 111 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

BryanMLima · 2024-04-22T19:41:44Z

ui/src/views/network/CreateIsolatedNetworkForm.vue

+          <a-form-item
+            ref="networkdomain"
+            name="networkdomain"
+            v-if="!isObjectEmpty(selectedNetworkOffering) && !selectedNetworkOffering.forvpc">
+            <template #label>
+              <tooltip-label :title="$t('label.networkdomain')" :tooltip="apiParams.networkdomain.description"/>
+            </template>
+            <a-input
+             v-model:value="form.networkdomain"
+              :placeholder="apiParams.networkdomain.description"/>
+          </a-form-item>
+          <a-form-item
+            ref="account"
+            name="account"
+            v-if="accountVisible">
+            <template #label>
+              <tooltip-label :title="$t('label.account')" :tooltip="apiParams.account.description"/>
+            </template>
+            <a-input
+             v-model:value="form.account"
+              :placeholder="apiParams.account.description"/>
+          </a-form-item>


@DaanHoogland, this code seems duplicated, considering that the network domain and account fields are already present in the form. Is this on purpose?

While working on PR #8919, there is no option to create a network for a project, so I was thinking of adding the Ownership component to this view as well and removing this duplicate code.

Sounds great @BryanMLima , I think this was not intentional. I just wonder about this dialog containing a "network domain" twice, while it should be a "cloudstack domain" and a real network domain. So a bit of semantic hacking needed ;) Thanks.

boring-cyborg bot added component:api component:networking labels Jun 7, 2022

apache deleted a comment from blueorangutan Jun 8, 2022

apache deleted a comment from blueorangutan Jun 9, 2022

DaanHoogland force-pushed the selectPublicIp branch from 129cc99 to c342e16 Compare July 8, 2022 10:49

boring-cyborg bot added component:UI component:vpc labels Jul 8, 2022

apache deleted a comment from blueorangutan Jul 8, 2022

boring-cyborg bot added component:kubernetes component:management-server labels Jul 8, 2022

DaanHoogland force-pushed the selectPublicIp branch 2 times, most recently from 22dbbfd to fe5cd64 Compare July 11, 2022 12:44

github-actions bot added status:has-conflicts and removed status:has-conflicts labels Aug 29, 2022

github-actions bot added the status:has-conflicts label Sep 10, 2022

DaanHoogland force-pushed the selectPublicIp branch from 75f9e50 to f2263c9 Compare November 29, 2022 16:00

github-actions bot removed the status:has-conflicts label Nov 29, 2022

boring-cyborg bot added component:marvin Python Warning... Python code Ahead! labels Nov 30, 2022

DaanHoogland force-pushed the selectPublicIp branch from 328b947 to f2263c9 Compare November 30, 2022 08:55

github-actions bot added the status:has-conflicts label Dec 5, 2022

DaanHoogland force-pushed the selectPublicIp branch from f2263c9 to 083a594 Compare December 12, 2022 09:45

fix warning message and some minor issues

54e8e10

harikrishna-patnala reviewed May 3, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java Show resolved Hide resolved

DaanHoogland added 5 commits May 3, 2023 08:59

integration test

c3bc8f9

javadoc

406a94a

bit of ui pollish

0b8e080

spello

9c6b92d

spello

e8ebd71

extra restart check

6dbcee2

kiranchavala approved these changes May 9, 2023

View reviewed changes

BryanMLima reviewed Apr 22, 2024

View reviewed changes

BryanMLima mentioned this pull request Apr 23, 2024

[UI] Add option to specify account/project while deploying VMs and creating networks #8919

Merged

13 tasks

Conversation

DaanHoogland commented Jun 7, 2022

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

codecov bot commented Aug 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented May 2, 2023

Uh oh!

blueorangutan commented May 2, 2023

Uh oh!

blueorangutan commented May 2, 2023

Uh oh!

blueorangutan commented May 2, 2023

Uh oh!

Uh oh!

Uh oh!

kiranchavala commented May 3, 2023

Uh oh!

blueorangutan commented May 3, 2023

Uh oh!

blueorangutan commented May 3, 2023

Uh oh!

blueorangutan commented May 8, 2023

Uh oh!

blueorangutan commented May 8, 2023

Uh oh!

kiranchavala left a comment

Choose a reason for hiding this comment

Uh oh!

kiranchavala commented Jun 1, 2023

Uh oh!

blueorangutan commented Jun 1, 2023

Uh oh!

blueorangutan commented Jun 1, 2023

Uh oh!

DaanHoogland commented Jun 6, 2023

Uh oh!

weizhouapache commented Jun 6, 2023

Uh oh!

DaanHoogland commented Jun 9, 2023

Uh oh!

blueorangutan commented Jun 9, 2023

Uh oh!

weizhouapache commented Jun 9, 2023

Uh oh!

DaanHoogland commented Jun 9, 2023

Uh oh!

blueorangutan commented Jun 9, 2023

Uh oh!

BryanMLima Apr 22, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Apr 23, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

codecov bot commented Aug 29, 2022 •

edited

Loading