Skip to content

Conversation

@matmerr
Copy link
Member

@matmerr matmerr commented Sep 17, 2020

Reason for Change:

Fixes an issue where if the CRD is nil, then don't pass to IPAM pool monitor

Issue Fixed:

Requirements:

Notes:

@codecov
Copy link

codecov bot commented Sep 17, 2020

Codecov Report

Merging #673 into master will decrease coverage by 0.94%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #673      +/-   ##
==========================================
- Coverage   42.63%   41.69%   -0.95%     
==========================================
  Files          72       72              
  Lines       10392     9483     -909     
==========================================
- Hits         4431     3954     -477     
+ Misses       5482     5057     -425     
+ Partials      479      472       -7     

@matmerr matmerr force-pushed the openaicnsinvestigation branch from 8379eff to 316006f Compare September 17, 2020 21:59
logger.Errorf("[cns-rc] Error creating or updating IPAM Pool Monitor: %v", err)
// requeue
return UnexpectedError
if scalar != nil && spec != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not be even calling CreateNC call if scaler/spec are null

}

// If there are no NCs, pass nil to CNSClient
if len(nodeNetConfig.Status.NetworkContainers) == 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should only be called if NodeNetworkConfig is null

Copy link
Contributor

@pjohnst5 pjohnst5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, talked about this in teams chat, and this is good in case the CRD is defined on the cluster, but the NNC instance of the crd isn't there yet, I missed that

@matmerr matmerr merged commit 251e820 into Azure:master Sep 21, 2020
neaggarwMS pushed a commit to neaggarwMS/azure-container-networking that referenced this pull request Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants