Skip to content

Conversation

@matmerr
Copy link
Member

@matmerr matmerr commented Jun 30, 2021

Reason for Change:

When reconciling with CNI, it may be the case that CNI state hasn't been created yet. Don't block consumers of that state when empty.

Issue Fixed:

Requirements:

Notes:

@matmerr matmerr force-pushed the cnifiledirectory branch 2 times, most recently from 4781851 to 40fc997 Compare June 30, 2021 21:01
@matmerr matmerr changed the title fix: [CNI] move store and lockfile paths to isolated directory fix: [CNI] handle getting endpoints when state file is empty Jun 30, 2021
@matmerr matmerr force-pushed the cnifiledirectory branch from 40fc997 to 97991e8 Compare June 30, 2021 21:05
neaggarwMS
neaggarwMS previously approved these changes Jun 30, 2021
Copy link
Member

@neaggarwMS neaggarwMS left a comment

Choose a reason for hiding this comment

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

:shipit:

store/json.go Outdated

if len(b) == 0 {
log.Printf("Unable to read file %s, was empty", kvs.fileName)
return nil
Copy link
Member

Choose a reason for hiding this comment

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

i would suggest return ErrEmptyFile and handle this in restore function (network/manager.go and ipam/manager.go) as like ErrKeyNotFound

Copy link
Member

Choose a reason for hiding this comment

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

@tamilmani1989 , this store file is also read by CNS to save CNS state, if are returning error then we need to handle it in there as well.

tamilmani1989
tamilmani1989 previously approved these changes Jun 30, 2021
Copy link
Member

@tamilmani1989 tamilmani1989 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@neaggarwMS neaggarwMS left a comment

Choose a reason for hiding this comment

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

:shipit:

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.

4 participants