Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use docker/CRI to discover pods at node init #1118

Merged
merged 2 commits into from
Aug 6, 2020
Merged

Use docker/CRI to discover pods at node init #1118

merged 2 commits into from
Aug 6, 2020

Conversation

fawadkhaliq
Copy link

Issue #711

Description of changes:
ipamd nodeInit relies on pod discovery watcher to get the list of running pods. The watcher and ipamd are launched in parallel. This can lead to a race condition where ipamd getLocalPodsWithRetry kicks in before the watcher is done with listing all the running pods on a node. If this happens, ipamd may end up allocating in-use IP addr to new pods

Changes

  • Changed ipamd to query docker/CRI to discover the list of node local pods instead Kubernetes API
  • Add support in the cri.go to get IP address of a pod and added IP field to cri.SandboxInfo
  • Datastore inputs changed from k8sapi.K8SPodInfo to cri.SandboxInfo

Alternative considered

  • Instead of using the watcher, make a blocking list pods call to the Kubernetes API server directly. This approach does not guarantee accurate state. On an overloaded system, API server might lag behind the node local state returning. While this approach would solve the original race condition, it can still return a different view of the running pods (eventually consistent)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

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

Nice, LGTM!

@mogren mogren merged commit 5932d79 into aws:release-1.6.4 Aug 6, 2020
@@ -386,9 +386,8 @@ func (c *IPAMContext) nodeInit() error {
}
localPods, err := c.getLocalPodsWithRetry()
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO - in 1.7 we might want to rename localPods to something like localRunningSandboxes

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