Skip to content
This repository has been archived by the owner on Jan 28, 2020. It is now read-only.

42-coreos.preset: Enable crio by default #6

Merged
merged 1 commit into from
Jul 13, 2018

Conversation

ashcrow
Copy link
Member

@ashcrow ashcrow commented Jul 12, 2018

Signed-off-by: Steve Milner <smilner@redhat.com>
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 12, 2018
@ashcrow
Copy link
Member Author

ashcrow commented Jul 12, 2018

/cc @jlebon @cgwalters @miabbott @arithx

@openshift-ci-robot
Copy link

@ashcrow: GitHub didn't allow me to request PR reviews from the following users: miabbott.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @jlebon @cgwalters @miabbott @arithx

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.

@cgwalters
Copy link
Member

So I have to admit I had not actually played with cri-o really before. I have some questions...

Out of curiosity is there a reason it's not socket activated?

Why does the cli we ship emit a deprecation warning in the default config?

# crictl  images
W0712 21:42:58.686957    2041 util_unix.go:75] Using "/var/run/crio/crio.sock" as endpoint is deprecated, please consider using full url format "unix:///var/run/crio/crio.sock".
IMAGE               TAG                 IMAGE ID            SIZE

Third: I'd been hoping to not enable docker.service by default actually. Having the service started by default means that if you do want to configure it post-startup you need to turn it off. Of course Ignition fixes this problem, but OTOH we're not today using Ignition for Vagrant.

@ashcrow
Copy link
Member Author

ashcrow commented Jul 13, 2018

Out of curiosity is there a reason it's not socket activated?
Why does the cli we ship emit a deprecation warning in the default config?

I'm not sure why, but @mrunalp may be able to answer those questions.

Third: I'd been hoping to not enable docker.service by default actually.

I'm with you on that. In fact I'm hoping we can drop the package in the compose soon.

Having the service started by default means that if you do want to configure it post-startup you need to turn it off. Of course Ignition fixes this problem, but OTOH we're not today using Ignition for Vagrant.

That's fair. A different way we could do this is have kola testing on crio start it up before running tests. But that feels a bit off to me as well.

Thoughts?

@miabbott
Copy link
Member

miabbott commented Jul 13, 2018

That's fair. A different way we could do this is have kola testing on crio start it up before running tests. But that feels a bit off to me as well.

I did that when testing your PR...it's a one-line change that would allow us to get some basic coverage on crio. I don't think it would be terrible in the short term to get some early coverage.

@miabbott
Copy link
Member

I think we can enable crio by default via these changes, get the answers for @cgwalters questions, and adjust in the future.

Soooo, LGTM

@cgwalters
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2018
@openshift-merge-robot openshift-merge-robot merged commit 139e51c into openshift:master Jul 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants