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

Send hcsshim's options struct when running with hcsshim #4364

Merged
merged 1 commit into from
Oct 22, 2023

Conversation

TBBle
Copy link
Collaborator

@TBBle TBBle commented Oct 22, 2023

containerd-shim-runhcs-v1 does not know how to parse the default containerd runtimeoptions.v1.Options structure, and fails at start-up.

Ref: microsoft/hcsshim#1941

The same logic is used in containerd/cri, and I'm not aware of any other better source for that constant string at the moment than just locally defining it. I suggested in microsoft/hcsshim#1941 that it be exported from the same package as the containerd.runhcs.v1.Options structure.

containerd-shim-runhcs-v1 does not know how to parse the default
containerd runtimeoptions.v1.Options structure, and panics at startup.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Copy link
Collaborator

@gabriel-samfira gabriel-samfira left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @TBBle !

Copy link
Member

@jedevc jedevc left a comment

Choose a reason for hiding this comment

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

Thanks @TBBle!

Sorry, I remember when working on this I omitted including the hcs shim options - I hadn't realized that would impact this work sorry!

@jedevc jedevc merged commit 3fbc634 into moby:master Oct 22, 2023
55 of 56 checks passed
@TBBle TBBle deleted the send-hcsshim-options-to-hcsshim branch October 22, 2023 22:42
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