Skip to content
New issue

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

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

Already on GitHub? # to your account

ocicni: pass a Pod UID down to CNI plugins as K8S_POD_UID #91

Merged
merged 1 commit into from
Jun 11, 2021

Conversation

dcbw
Copy link
Collaborator

@dcbw dcbw commented Jun 11, 2021

If a pod is deleted from the Kube API while a SetUpPod() call
is ongoing it would be nice if the CNI plugin could easily
figure that out and exit early. Plugins can watch the Kube API
for pod events, but there is a race where the pod could have
been deleted + recreated before the plugin is executed and
sets up the watches.

Since each new pod object will have a different UID, pass
the UID we get from the runtime down to the CNI plugins so
they can compare the UID they receive from ocicni with one
they read from the Kube API. If the two UIDs are different,
that means the pod was deleted + recreated before the plugin
ran, and the plugin may wish to exit early.

@trozet @mrunalp @mccv1r0

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 11, 2021

@dcbw: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 11, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 11, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dcbw

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 11, 2021
If a pod is deleted from the Kube API while a SetUpPod() call
is ongoing it would be nice if the CNI plugin could easily
figure that out and exit early. Plugins can watch the Kube API
for pod events, but there is a race where the pod could have
been deleted + recreated before the plugin is executed and
sets up the watches.

Since each new pod object will have a different UID, pass
the UID we get from the runtime down to the CNI plugins so
they can compare the UID they receive from ocicni with one
they read from the Kube API. If the two UIDs are different,
that means the pod was deleted + recreated before or during
the plugin execution, and the plugin may wish to exit early
since any information it read from the Kube API and used to
configure sandbox resources may be out-of-date.

Signed-off-by: Dan Williams <dcbw@redhat.com>
@coveralls
Copy link

coveralls commented Jun 11, 2021

Pull Request Test Coverage Report for Build 927024188

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 65.833%

Totals Coverage Status
Change from base Build 899841198: 0.05%
Covered Lines: 474
Relevant Lines: 720

💛 - Coveralls

@dcbw dcbw force-pushed the pass-uid-to-plugins branch from a75bfed to 1ea8db5 Compare June 11, 2021 01:17
@dcbw dcbw removed the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Jun 11, 2021
Name string
// Namespace is the namespace of the sandbox.
// Namespace is the namespace of the pod.
Namespace string
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Isn't Namespace still the name of the sandbox?

@mccv1r0
Copy link
Contributor

mccv1r0 commented Jun 11, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2021
@openshift-merge-robot openshift-merge-robot merged commit 95ad096 into cri-o:master Jun 11, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants