vpc/server: Fix network statistics for vpc by ustcweizhou · Pull Request #3944 · apache/cloudstack

ustcweizhou · 2020-03-05T21:51:03Z

Description

This contains 3 main changes
(1) add NETWORK_STATS_ethX for all nics with public ips in VPC VRs (current: NETWORK_STATS_eth1)
(2) DO NOT create records in user_statistics for each VPC tier (only one record per public nic per VPC VR)
(3) send NetworkUsageCommand before unplugging a NIC with public IPs from VPC VR

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)

Screenshots (if appropriate):

How Has This Been Tested?

yadvr · 2020-03-06T06:32:25Z

Is this in progress or ready for review @ustcweizhou ?

weizhouapache · 2020-03-06T08:02:41Z

Is this in progress or ready for review @ustcweizhou ?

@rhtyd it is ready for review

travis failed, I will check it.

DaanHoogland · 2020-03-06T08:22:09Z

Is this in progress or ready for review @ustcweizhou ?

@rhtyd it is ready for review

travis failed, I will check it.

don't bother @weizhouapache those seem to be the usual log too long errors. Travis stops at 4GB or so.

weizhouapache · 2020-03-06T09:18:43Z

Is this in progress or ready for review @ustcweizhou ?

@rhtyd it is ready for review
travis failed, I will check it.

don't bother @weizhouapache those seem to be the usual log too long errors. Travis stops at 4GB or so.

thanks @DaanHoogland for your info.

yadvr · 2020-06-12T05:02:38Z

@blueorangutan package

blueorangutan · 2020-06-12T05:03:13Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-06-12T05:47:16Z

Packaging result: ✔centos7 ✔debian. JID-1356

yadvr · 2020-06-13T00:43:50Z

@blueorangutan package

blueorangutan · 2020-06-13T00:45:11Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-06-13T01:46:34Z

Packaging result: ✖centos7 ✖debian. JID-1368

yadvr · 2020-07-02T09:26:54Z

@blueorangutan package

blueorangutan · 2020-07-02T09:28:15Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-07-02T10:30:23Z

Packaging result: ✔centos7 ✔debian. JID-1508

yadvr · 2020-07-02T10:46:43Z

@blueorangutan test

blueorangutan · 2020-07-02T10:47:48Z

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

yadvr · 2020-07-04T05:29:09Z

@blueorangutan test

blueorangutan · 2020-07-04T05:30:12Z

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

blueorangutan · 2020-07-04T21:14:35Z

Trillian test result (tid-1976)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 49953 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3944-t1976-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Smoke tests completed. 82 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_add_delete_kubernetes_supported_version	`Error`	1807.31	test_kubernetes_supported_versions.py

yadvr

LGTM, did not test it

DaanHoogland

code looks good and i like the simplification, but ...
@PaulAngus @andrijapanicsb do we know people that use per tier billing somehow? this will impede those.

DaanHoogland · 2020-11-17T18:51:09Z

@weizhouapache converted to draft so it is clear 4.15 is not waiting for it. please change back if you disagree

yadvr · 2020-12-08T10:47:14Z

@ustcweizhou @weizhouapache can you comment/explore the cases where this could add regression for usage/billing?

weizhouapache · 2020-12-08T13:43:01Z

@DaanHoogland @rhtyd
this fixes bugs in network statistics for vpc.
cloudstack should collect network statistics from the public interfaces, not from eth1 which is default public nic for vpc but there might be more public nics, not from vpc tiers.
in a word, there is not per tier billing. there are only statistics for all public interfaces in VR for isolated network and vpc, and for all nics on shared network.

DaanHoogland · 2020-12-09T14:01:42Z

@weizhouapache the changes seem logical to me and given your description above seem to do what you intended, but we have no testdescription and no integration tests to guarantee against regressions. I'm a bit reluctant to include this just before release. can we with just the current set of smoke tests? cc @rhtyd @sureshanaparti are there any others we could ask for testing this functionality?

weizhouapache · 2020-12-09T20:23:17Z

@weizhouapache the changes seem logical to me and given your description above seem to do what you intended, but we have no testdescription and no integration tests to guarantee against regressions. I'm a bit reluctant to include this just before release. can we with just the current set of smoke tests? cc @rhtyd @sureshanaparti are there any others we could ask for testing this functionality?

@DaanHoogland let's move this to 4.15.1.0 (and 4.14.1.0)

shwstppr · 2021-02-22T10:08:17Z

@weizhouapache conflicts here

This contains 3 main changes (1) add NETWORK_STATS_ethX for all nics with public ips in VPC VRs (current: NETWORK_STATS_eth1) (2) DO NOT create records in user_statistics for each VPC tier (only one record per public nic per VPC VR) (3) send NetworkUsageCommand before unplugging a NIC with public IPs from VPC VR

shwstppr · 2021-02-22T15:53:41Z

@blueorangutan package

blueorangutan · 2021-02-22T15:55:15Z

@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-02-23T06:07:29Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2798

DaanHoogland · 2021-02-23T09:24:57Z

@blueorangutan test

blueorangutan · 2021-02-23T09:26:02Z

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

blueorangutan · 2021-02-23T18:50:40Z

Trillian test result (tid-3601)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31877 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3944-t3601-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Smoke tests completed. 86 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland · 2021-02-24T11:19:37Z

@rhtyd @weizhouapache no discussion has happened, do we dare merge this?

weizhouapache · 2021-03-24T08:34:45Z

@rhtyd @weizhouapache no discussion has happened, do we dare merge this?

@DaanHoogland @rhtyd
it is ok for me to merge it :-D

we have this patch on our productions. looks good until now.

yadvr

Proxying my lgtm based on Daan's and Wei's comments/review.

svenvogel added component:vpc type:bug labels Mar 6, 2020

DaanHoogland added this to the 4.14.1.0 milestone May 7, 2020

yadvr changed the base branch from master to 4.14 June 13, 2020 00:43

yadvr closed this Jul 2, 2020

yadvr reopened this Jul 2, 2020

yadvr approved these changes Jul 7, 2020

View reviewed changes

yadvr requested review from DaanHoogland and nvazquez July 7, 2020 07:16

DaanHoogland reviewed Jul 28, 2020

View reviewed changes

yadvr requested review from PaulAngus and borisstoyanov August 4, 2020 05:56

DaanHoogland requested a review from yadvr October 20, 2020 09:48

DaanHoogland marked this pull request as draft November 17, 2020 18:50

ustcweizhou force-pushed the 4.14-fix-vpc-statistics branch from 09cec4b to b3403e2 Compare December 8, 2020 13:18

weizhouapache marked this pull request as ready for review December 8, 2020 13:22

DaanHoogland modified the milestones: 4.14.1.0, 4.14.2.0 Dec 10, 2020

yadvr modified the milestones: 4.14.2.0, 4.15.1.0 Jan 27, 2021

yadvr changed the base branch from 4.14 to 4.15 January 27, 2021 14:00

ustcweizhou force-pushed the 4.14-fix-vpc-statistics branch from b3403e2 to 6e83c77 Compare February 22, 2021 13:14

apache deleted a comment from blueorangutan Feb 23, 2021

DaanHoogland approved these changes Mar 24, 2021

View reviewed changes

yadvr approved these changes Apr 1, 2021

View reviewed changes

yadvr merged commit 63c91c1 into apache:4.15 Apr 1, 2021

Conversation

ustcweizhou commented Mar 5, 2020

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

yadvr commented Mar 6, 2020

Uh oh!

weizhouapache commented Mar 6, 2020

Uh oh!

DaanHoogland commented Mar 6, 2020

Uh oh!

weizhouapache commented Mar 6, 2020

Uh oh!

yadvr commented Jun 12, 2020

Uh oh!

blueorangutan commented Jun 12, 2020

Uh oh!

blueorangutan commented Jun 12, 2020

Uh oh!

yadvr commented Jun 13, 2020

Uh oh!

blueorangutan commented Jun 13, 2020

Uh oh!

blueorangutan commented Jun 13, 2020

Uh oh!

yadvr commented Jul 2, 2020

Uh oh!

blueorangutan commented Jul 2, 2020

Uh oh!

blueorangutan commented Jul 2, 2020

Uh oh!

yadvr commented Jul 2, 2020

Uh oh!

blueorangutan commented Jul 2, 2020

Uh oh!

yadvr commented Jul 4, 2020

Uh oh!

blueorangutan commented Jul 4, 2020

Uh oh!

blueorangutan commented Jul 4, 2020

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Nov 17, 2020

Uh oh!

yadvr commented Dec 8, 2020

Uh oh!

weizhouapache commented Dec 8, 2020

Uh oh!

DaanHoogland commented Dec 9, 2020

Uh oh!

weizhouapache commented Dec 9, 2020

Uh oh!

shwstppr commented Feb 22, 2021

Uh oh!

shwstppr commented Feb 22, 2021

Uh oh!

blueorangutan commented Feb 22, 2021

Uh oh!

blueorangutan commented Feb 23, 2021

Uh oh!

DaanHoogland commented Feb 23, 2021

Uh oh!

blueorangutan commented Feb 23, 2021

Uh oh!

blueorangutan commented Feb 23, 2021

Uh oh!

DaanHoogland commented Feb 24, 2021

Uh oh!

weizhouapache commented Mar 24, 2021

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!