Add a go routine to update NC host version from NMAgent periodically. #714

csfmomo · 2020-10-30T00:03:51Z

feat:

Add a go routine to update NC host version from NMAgent periodically.
If orchestrator type is CRD, update pending programming IPs as well.

Reason for Change:
NC version from DNC can be different with NC version from NMAgent when NMAgent didn't keep up the NC version change from DNC side.
This feature enable CNS sync NC version from NMAgent periodically and update related info.
fix: Bug Fixes 🐞
test: Testing 💚

[X ] adds unit tests

codecov · 2020-10-30T00:09:04Z

Codecov Report

Merging #714 (2f2cc80) into master (169d7d7) will increase coverage by 0.08%.
The diff coverage is 64.70%.

@@            Coverage Diff             @@
##           master     #714      +/-   ##
==========================================
+ Coverage   39.61%   39.69%   +0.08%     
==========================================
  Files          82       82              
  Lines       10800    10858      +58     
==========================================
+ Hits         4278     4310      +32     
- Misses       6017     6031      +14     
- Partials      505      517      +12

cns/restserver/internalapi.go

cns/restserver/ipam.go

cns/service/main.go

cns/api.go

cns/restserver/internalapi.go

cns/api.go

cns/cnsclient/cnsclient_test.go

cns/configuration/configuration.go

cns/restserver/internalapi.go

cns/restserver/internalapi_test.go

cns/restserver/internalapi.go

cns/service/main.go

csfmomo · 2020-11-18T05:10:15Z

/azp run

azure-pipelines · 2020-11-18T05:10:25Z

Azure Pipelines successfully started running 1 pipeline(s).

csfmomo · 2020-11-18T05:27:26Z

/azp run

azure-pipelines · 2020-11-18T05:27:36Z

Azure Pipelines successfully started running 1 pipeline(s).

cns/configuration/configuration.go

cns/nmagentclient/nmagentclient.go

cns/requestcontroller/kubecontroller/crdtranslator.go

neaggarwMS · 2020-11-24T19:00:14Z

cns/restserver/internalapi.go


+// SyncHostNCVersion will check NC version from NMAgent and save it as host NC version in container status.
+// If NMAgent NC version got updated, CNS will refresh the pending programming IP status.
+func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMode string, lastUpdatedHostNCTimeStamp time.Time, forceMarkIPAvailableTimeRange time.Duration) {


I highly recommend to move this to a separate file (like IPAMPoolMonitor). we only add apis called by internal cns client in this file.

This file is not for internal cns call from begining. It's AKS start using it for internal CNS call. There are 6 functions in internalapi.go, only 2 of them are called by cns client. CreateOrUpdateNetworkContainerInternal and ReconcileNCState are called by cns client. The rest of 4 are not called by cns client.

Could I remain the function here for this version?
I can do moving logic to separate file for next PR and we can move all the related logic, including what managed DNC use in a separate file.

cns/restserver/internalapi.go

cns/restserver/ipam.go

cns/restserver/restserver.go

cns/service/main.go

If orchestrator type is CRD, update pending programming IPs as well.

…tate as Avaialable instead of pending programming.

Resovle conflicts manually. Update unit test nc version value. Update unit test nc version. Add get nmagent default value back for integ testing purpose. Unit test can be break by this change. Update default new IP CNS status to available. Assign value to host version if none exist in util.go Addressed feedback and perform cluster integ test with 1 sec frequent nc version update. Need to clean logNCSnapshots when send out PR. Update nc version associate with secondary ip. Add new nmagent api test. Add versionResponseWithoutToken.Containers log Add containerId from our runner sub. Add containerId from NMAgent team. Addressed feedback and add real nmagent logic. Add timeout when query nmagent for nc version.

…ion.

This is the final version. Change the way of http get request to add context. Change channel to no buffer with same goroutine. Found always fall in ctx.Done() condition. Add channel close for get nc version list. Add milisecond unit for timeout. Testing with different context version.

… retry if no response from NMAgent. Release pending programming IP when scale down.

csfmomo · 2020-12-02T23:43:37Z

/azp run

azure-pipelines · 2020-12-02T23:43:50Z

Azure Pipelines successfully started running 1 pipeline(s).

cns/restserver/const.go

…ion inside CreateNetworkContainerRequest.

cns/configuration/configuration.go

cns/fakes/nmagentclientfake.go

cns/nmagentclient/nmagentclient.go

cns/restserver/internalapi.go

cns/service/main.go

ashvindeodhar · 2020-12-09T16:07:13Z

cns/service/main.go

 		return
 	}

+	nmaclient, err := nmagentclient.NewNMAgentClient("")


what's the use of having empty param to this?

In case we want to override nmagent client url.

cns/restserver/internalapi.go

Can't find where is this request. Sync up offline we can merege it first.

csfmomo requested a review from ramiro-gamarra October 30, 2020 00:04

csfmomo requested review from ashvindeodhar and neaggarwMS October 30, 2020 16:01

neaggarwMS requested changes Nov 2, 2020

View reviewed changes

ashvindeodhar requested changes Nov 3, 2020

View reviewed changes

csfmomo force-pushed the bgForNcV branch from 10e54d2 to 9cd0855 Compare November 5, 2020 02:00

csfmomo force-pushed the bgForNcV branch 2 times, most recently from f97a86e to bc35910 Compare November 18, 2020 05:07

csfmomo requested review from ashvindeodhar, matmerr and neaggarwMS November 18, 2020 05:34

csfmomo force-pushed the bgForNcV branch 3 times, most recently from 392f83d to 71bb051 Compare November 24, 2020 18:55

neaggarwMS requested changes Nov 24, 2020

View reviewed changes

cns/configuration/configuration.go Outdated Show resolved Hide resolved

neaggarwMS previously requested changes Nov 24, 2020

View reviewed changes

csfmomo force-pushed the bgForNcV branch 4 times, most recently from 6a6fa28 to 946cbbf Compare December 1, 2020 01:42

csfmomo added 5 commits November 30, 2020 17:49

Add a go routine to update NC host version from NMAgent periodically.

37d4948

If orchestrator type is CRD, update pending programming IPs as well.

Update NC version in test from 0 to -1, which will allow default IP s…

d9c9b95

…tate as Avaialable instead of pending programming.

Update comments.

19afda8

Add context background with timeout function for syncing node nc vers…

8dc5632

…ion.

csfmomo added 5 commits November 30, 2020 17:51

Add 5 second force update CNS pending programming IP to available logic.

3e32105

Resovle merge conflict from master.

88748fe

Resolve merge conflict.

7b4d200

Remove force update pending programming IP to available logic. Remain…

02f2800

… retry if no response from NMAgent. Release pending programming IP when scale down.

csfmomo force-pushed the bgForNcV branch 5 times, most recently from f48b691 to 4dc342e Compare December 1, 2020 20:21

matmerr reviewed Dec 7, 2020

View reviewed changes

cns/restserver/const.go Outdated Show resolved Hide resolved

Remain VMVersion, HostVersion variable name as it is and use the Vers…

d494cf6

…ion inside CreateNetworkContainerRequest.

csfmomo force-pushed the bgForNcV branch from 4dc342e to d494cf6 Compare December 7, 2020 17:55

ashvindeodhar reviewed Dec 9, 2020

View reviewed changes

csfmomo requested review from ashvindeodhar and neaggarwMS December 10, 2020 01:24

Addressed team member feedback.

2f2cc80

csfmomo force-pushed the bgForNcV branch from eadd93d to 2f2cc80 Compare December 10, 2020 18:35

ashvindeodhar approved these changes Dec 11, 2020

View reviewed changes

csfmomo merged commit 3c48a34 into Azure:master Dec 11, 2020

csfmomo deleted the bgForNcV branch December 12, 2020 05:27

Add a go routine to update NC host version from NMAgent periodically. #714

Add a go routine to update NC host version from NMAgent periodically. #714

Uh oh!

Conversation

csfmomo commented Oct 30, 2020

Uh oh!

codecov bot commented Oct 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

csfmomo commented Nov 18, 2020

Uh oh!

azure-pipelines bot commented Nov 18, 2020

Uh oh!

csfmomo commented Nov 18, 2020

Uh oh!

azure-pipelines bot commented Nov 18, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

neaggarwMS Nov 24, 2020

Choose a reason for hiding this comment

Uh oh!

csfmomo Nov 25, 2020

Choose a reason for hiding this comment

Uh oh!

csfmomo Dec 1, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

csfmomo commented Dec 2, 2020

Uh oh!

azure-pipelines bot commented Dec 2, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ashvindeodhar Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

csfmomo Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

codecov bot commented Oct 30, 2020 •

edited

Loading