Skip to content

Conversation

@rbtr
Copy link
Collaborator

@rbtr rbtr commented Jun 4, 2021

Reason for Change:

  • Updates the CNS pod statemap to be able to use the new interface ID being passed from CNI instead of keying on Kubernetes namespace/name
  • Adds a flow where CNS can rebuild its internal state from CNI instead of from the Kubernetes API
  • Adds a config flag and a CNI version check, making the above two changes explicitly opt-in

Issue Fixed:

Requirements:

Notes:

@rbtr rbtr force-pushed the rbtr/pod-interface-id branch from 0da895e to 4c20015 Compare June 4, 2021 20:44
@rbtr rbtr force-pushed the rbtr/pod-interface-id branch 4 times, most recently from c8d81a0 to 21ae876 Compare June 14, 2021 23:41
@matmerr
Copy link
Member

matmerr commented Jun 15, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr rbtr force-pushed the rbtr/pod-interface-id branch 2 times, most recently from 4dfc78e to 6f757d1 Compare June 15, 2021 20:04
@rbtr rbtr added cns Related to CNS. enhancement labels Jun 15, 2021
@rbtr rbtr force-pushed the rbtr/pod-interface-id branch 8 times, most recently from 7434f9a to 3adfec0 Compare June 20, 2021 01:13
@rbtr rbtr changed the title cns pod interface changes feat: add flow to initialize CNS from CNI Jun 20, 2021
@rbtr rbtr requested review from matmerr and neaggarwMS June 20, 2021 01:21
@rbtr rbtr self-assigned this Jun 20, 2021
@rbtr rbtr marked this pull request as ready for review June 20, 2021 01:22
@rbtr rbtr force-pushed the rbtr/pod-interface-id branch from 3adfec0 to e4bef1a Compare June 21, 2021 17:42
@neaggarwMS
Copy link
Member

neaggarwMS commented Jun 24, 2021

func TestReconcileNCWithExistingState(t *testing.T) {

Can we update this test to new struct (pass InterfaceId/ContainerId and validate that as well) #Closed


Refers to: cns/restserver/internalapi_test.go:220 in 6f27e8a. [](commit_id = 6f27e8a, deletion_comment = False)

@neaggarwMS
Copy link
Member

neaggarwMS commented Jun 24, 2021

  logger.Errorf("[releaseIPConfig] SetIPConfigAsAvailable failed to release, no allocation found for pod [%+v]", podInfo)

It is okay to get idempotent release calls, can you update this message for "Ignore release as no allocations found for this pod" #Closed


Refers to: cns/restserver/ipam.go:397 in 6f27e8a. [](commit_id = 6f27e8a, deletion_comment = False)

@neaggarwMS
Copy link
Member

neaggarwMS commented Jun 24, 2021

var (

We should either run all these test with old and new contract or move them to new (where you also pass interfaceId and infracontainer in IPConfigRequest" #Closed


Refers to: cns/restserver/ipam_test.go:16 in 6f27e8a. [](commit_id = 6f27e8a, deletion_comment = False)

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.

🕐

@neaggarwMS neaggarwMS dismissed their stale review June 24, 2021 16:25

revoking review

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.

🕐

neaggarwMS
neaggarwMS previously approved these changes Jun 24, 2021
@matmerr matmerr requested a review from thatmattlong June 28, 2021 18:16
@rbtr rbtr force-pushed the rbtr/pod-interface-id branch from 7c87944 to fbb472a Compare June 29, 2021 16:59
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:

@rbtr rbtr merged commit 7d224bf into master Jun 29, 2021
@rbtr rbtr deleted the rbtr/pod-interface-id branch June 29, 2021 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cns Related to CNS. enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants