adaptation: return nil as no-op adjustment. #118
Conversation
klihub
force-pushed
the
devel/use-nil-as-nop-adjustment
branch
from
f765fc8 to
362c075
Compare
klihub
force-pushed
the
devel/use-nil-as-nop-adjustment
branch
from
362c075 to
a39f4c4
Compare
mikebrow
left a comment
mikebrow
left a comment
There was a problem hiding this comment.
LGTM just have to push and cherry the nil check referenced in the PR desc.
klihub
force-pushed
the
devel/use-nil-as-nop-adjustment
branch
from
a39f4c4 to
4eab409
Compare
klihub
force-pushed
the
devel/use-nil-as-nop-adjustment
branch
from
4eab409 to
fd82b8b
Compare
Don't create an empty adjustment a priori. Only create once a plugin makes an adjustment to the container being created. In particular, this should result in a nil adjustment if no plugin touches a container. Signed-off-by: Krisztian Litkey <[email protected]>
klihub
force-pushed
the
devel/use-nil-as-nop-adjustment
branch
from
fd82b8b to
633857b
Compare
|
@chrishenzie @samuelkarp wdyt? |
|
@klihub did we merge and cherry the fix in internal/cri/nri for what happens if we return nil and no err for the adjustment? |
|
@mikebrow No, I think not yet. It'll require something like this. But I think I know what you're after. It would be much safer to (first rebase, retest and make sure that it is really all we'll need and then) merge that change in containerd (and a similar one in CRI-O, even if they are now checks for something that can't happen), and only then merge this one. Right ? |
nod.. thx! |
4 participants
With no NRI plugins registered or none of the registered plugins requesting changes to a container, we should ideally skip applying NRI adjustments on the runtime side altogether, instead of going through the motions without any effective changes. Having seen #117 (quickly fixed by #116), this is not the case.
This PR is the first part in an attempt to change that. With this PR in place, we don't create an a priori empty adjustment in preparation to process NRI requests. We only create it once a plugin makes an actual change to the container being created. Effectively, this should result in a nil adjustment reply when there are no plugins present, or none of the present plugins touches a container.
The other part is a related change to the runtime integration code to check for this and short-circuit processing.