Skip to content

[release/1.7] Require plugins to succeed after registering readiness#9165

Merged
fuweid merged 2 commits intocontainerd:release/1.7from
dmcgowan:backport-1.7-readiness-hang-fix
Sep 30, 2023
Merged

[release/1.7] Require plugins to succeed after registering readiness#9165
fuweid merged 2 commits intocontainerd:release/1.7from
dmcgowan:backport-1.7-readiness-hang-fix

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan commented Sep 29, 2023

Fixes hang when plugin which registers readiness fail. Additional fix in CRI to register readiness later on in its initialization.

Backport #9153 #9164

@AkihiroSuda
Copy link
Copy Markdown
Member

Could you swap the commit order to help bisecting?

AkihiroSuda and others added 2 commits September 29, 2023 20:44
`NewCRIService()` may easily fail and its error has to be ignored
unless the CRI plugin is in the `required_plugins` list.

Now this has to be called before `RegisterReadiness()`, as
PR 9153 "Require plugins to succeed after registering readiness"
was merged on 2023-09-29.

Fix issue 9163: `[Regression in main (2023-09-29)]: containerd-rootless.sh doesn't start up`

Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit 5365f4b)
Signed-off-by: Derek McGowan <[email protected]>
When readiness is registered on initialization, the plugin must not
fail. When such a plugin fails, containerd will hang on the readiness
condition.

Signed-off-by: Derek McGowan <[email protected]>
(cherry picked from commit e725440)
Signed-off-by: Derek McGowan <[email protected]>
@dmcgowan dmcgowan force-pushed the backport-1.7-readiness-hang-fix branch from b8c0638 to a83c668 Compare September 30, 2023 03:53
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@fuweid fuweid merged commit 790f6d9 into containerd:release/1.7 Sep 30, 2023
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