Skip to content

Conversation

@mxpv
Copy link
Member

@mxpv mxpv commented Apr 15, 2021

A few minor runtime changes:

  • b83d04f adds variable names to runtime's interface definitions to ease readability.
  • 993b863 extracts out ShimStart params into a separate structure shim.StartOpts. This will make easier adding new options and keep backward compatibility. For instance, turns out we pass shim runtime options via Stdin, but don't really use those in the shim itself. I think we should expose this option in 1.6.

mxpv added 2 commits April 15, 2021 11:55
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
To ease code readability

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
@theopenlab-ci
Copy link

theopenlab-ci bot commented Apr 15, 2021

Build succeeded.

@github-actions
Copy link

Test Results

     13 files     219 suites   47m 26s ⏱️
1 028 tests    996 ✔️ 32 💤 0 ❌
2 483 runs  2 413 ✔️ 70 💤 0 ❌

Results for commit b83d04f.

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

// Tasks returns all the current tasks for the runtime.
// Any container runs at most one task at a time.
Tasks(context.Context, bool) ([]Task, error)
Tasks(ctx context.Context, all bool) ([]Task, error)
Copy link
Member

@kzys kzys Apr 15, 2021

Choose a reason for hiding this comment

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

Naming the flag all is confusing since Tasks() returns all tasks regardless of the flag. Seems the flag means "from all namespaces".

Copy link
Member

@kzys kzys left a comment

Choose a reason for hiding this comment

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

LGTM. One non-blocking comment.

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 1e5cb4e into containerd:master Apr 16, 2021
@mxpv mxpv deleted the runtime_cleanup branch April 19, 2021 18:59
@mxpv mxpv mentioned this pull request Apr 19, 2021
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.

5 participants