Skip to content

Conversation

@matmerr
Copy link
Member

@matmerr matmerr commented Aug 13, 2020

Reason for Change:

IPAM delete in CNS was not passing pod info, therefore ID in state was never found

Issue Fixed:

Requirements:

Notes:

@codecov
Copy link

codecov bot commented Aug 13, 2020

Codecov Report

Merging #649 into master will decrease coverage by 0.05%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master     #649      +/-   ##
==========================================
- Coverage   42.08%   42.03%   -0.06%     
==========================================
  Files          71       71              
  Lines       10271    10271              
==========================================
- Hits         4323     4317       -6     
- Misses       5473     5476       +3     
- Partials      475      478       +3     

}

func (service *HTTPRestService) validateIpConfigRequest(ipConfigRequest cns.GetIPConfigRequest) (int, string) {
func (service *HTTPRestService) validateIpConfigRequest(ipConfigRequest cns.GetIPConfigRequest) (*cns.KubernetesPodInfo, int, string) {
Copy link
Collaborator

@thatmattlong thatmattlong Aug 13, 2020

Choose a reason for hiding this comment

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

I think it would be safer to return a cns.KubernetesPodInfo instead of a pointer. Unless I'm missing something here I don't see the value in using a pointer since we return a pointer to an empty struct instead of nil on error, anyway

neaggarwMS
neaggarwMS previously approved these changes Aug 13, 2020
Copy link
Collaborator

@thatmattlong thatmattlong left a comment

Choose a reason for hiding this comment

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

I think removing the pointer return type from cns/restserver/util.go's validateIpConfigRequest would make things a bit simpler, but otherwise lgtm

@matmerr matmerr merged commit df9c086 into Azure:master Aug 14, 2020
neaggarwMS pushed a commit to neaggarwMS/azure-container-networking that referenced this pull request Nov 13, 2020
* fix: present podinfo to delete

* fix: return empty struct instead of pointer
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