Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

Expose environment variables for configuring an additional runtime handler #1069

Merged
merged 1 commit into from
Mar 14, 2019
Merged

Expose environment variables for configuring an additional runtime handler #1069

merged 1 commit into from
Mar 14, 2019

Conversation

tallclair
Copy link
Contributor

This is a prerequisite to testing a non-default runtime handler in the Kubernetes containerd E2E tests.

@k8s-ci-robot
Copy link

Hi @tallclair. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

This was referenced Feb 28, 2019
Merged
Merged
@mikebrow
Copy link
Member

mikebrow commented Mar 1, 2019 •
edited
Loading

Looks like the DCO signature validation failed. Maybe commit description ran over.. messing with the signature block? I didn't pull the PR down to check.

@mikebrow
Copy link
Member

mikebrow commented Mar 1, 2019

/ok-to-test

@tallclair tallclair force-pushed the runtimehandler-setup branch from 02e8b9f to dd93df6 Compare March 4, 2019 18:01
@tallclair
Copy link
Contributor Author

Updated the commit message, in case that help.s

@mikebrow
Copy link
Member

mikebrow commented Mar 4, 2019 •
edited
Loading

kk just need you to add a doc signature.. one of these: Signed-off-by: Mike Brown <brownwm@us.ibm.com>

assuming you have your name and email configured in git config...:
commit -s --amend

@tallclair tallclair force-pushed the runtimehandler-setup branch from dd93df6 to 69ea030 Compare March 4, 2019 19:11
@tallclair
Copy link
Contributor Author

Oh yeah, sorry, done.

mikebrow
mikebrow suggested changes Mar 4, 2019
View reviewed changes
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.

This format has been deprecated.. Might want to move up to the runtimes format. Here's an example for kata.

https://github.com/kata-containers/documentation/blob/master/how-to/containerd-kata.md#kata-containers-as-a-runtimeclass

@mikebrow
Copy link
Member

mikebrow commented Mar 4, 2019

Unless this is for an older release, using the old untrusted_workload_runtime pattern that came pre runtimes?

@Random-Liu Random-Liu added this to the v1.3 milestone Mar 4, 2019
@tallclair tallclair force-pushed the runtimehandler-setup branch from 69ea030 to 0a0b047 Compare March 6, 2019 18:06
@tallclair
Copy link
Contributor Author

Thanks, I think I've fixed it - LMK if this isn't what you meant.

@tallclair
Copy link
Contributor Author

For reference, the other 2 PRs for enabling the test are:

@tallclair
Copy link
Contributor Author

/retest

mikebrow
mikebrow approved these changes Mar 7, 2019
View reviewed changes
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

@Random-Liu
Copy link
Member

/test pull-cri-containerd-node-e2e

@tallclair
Copy link
Contributor Author

Gave up on specifying specific options, and pushed the whole options block into a single env. kubernetes/test-infra#11572 has the updated test configuration.

mikebrow
mikebrow approved these changes Mar 12, 2019
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

/LGTM

Just need to sign the 2nd commit :)

@tallclair
Expose vars to configure an additional runtime handler
d7c5b24 
Expose environment variables in the GCE containerd configuration
script for configuring an additional runtime handler. This unblocks
E2E testing of custom runtime handlers.

Signed-off-by: Tim Allclair <tallclair@google.com>
@tallclair tallclair force-pushed the runtimehandler-setup branch from c52d00c to d7c5b24 Compare March 12, 2019 21:44
@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 12, 2019
@tallclair
Copy link
Contributor Author

squashed commits

mikebrow
mikebrow approved these changes Mar 12, 2019
View reviewed changes
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

@Random-Liu
Copy link
Member

/lgtm

@Random-Liu Random-Liu merged commit 9c9bf1d into containerd:master Mar 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mikebrow mikebrow mikebrow approved these changes

Assignees

@Random-Liu Random-Liu

@mikebrow mikebrow

Labels
lgtm ok-to-test size/S
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.
4 participants
@tallclair @k8s-ci-robot @mikebrow @Random-Liu