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

Typo fix: add missing `make` command #53

Merged
merged 1 commit into from Jun 11, 2018

Conversation

Projects
None yet
2 participants
@t3hmrman
Copy link
Contributor

t3hmrman commented May 27, 2018

No description provided.

@grahamwhaley
Copy link
Contributor

grahamwhaley left a comment

lgtm
The CI failed - I suspect for two reaons:

  1. I think the commit needs to conform to the patch format as detailed in: https://github.com/clearcontainers/runtime/blob/master/CONTRIBUTING.md#patch-format - that is, you'll need to add an Issue and reference it in a Fixes line, and sign off your commit.

Also, the CI seems to have gotten some odd build error:

15:02:11 /workdir/linux /workdir
15:02:12 scripts/kconfig/conf  --silentoldconfig Kconfig
15:02:13 Makefile:943: *** "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel".  Stop.

/cc @jcvenegas @chavafg for thoughts on that.

btw @t3hmrman , have you considered https://github.com/kata-containers/, which is where the primary and leading edge development has shifted to (and with the 1.0 release last week, it is probably the best place to head...)

@t3hmrman

This comment has been minimized.

Copy link
Contributor Author

t3hmrman commented May 29, 2018

About the two things:

  1. Ahhh OK, I will go ahead and make an issue and fix up the patch format!

  2. That actually has to do with newer kernels requiring the libelf dev (?) -- I've been running into this a lot, it turns out for newer versions you need to dnf install elfutils-libelf-devel, I made the change locally in the Dockerfile but forgot about it.

Also @grahamwhaley , yeah, I'm actually trying to get kata-containers running on Container Linux (CoreOS) -- it's been an long ride but basically I've statically compiled kata-runtime, kata-proxy, kata-shim, and a minimal build of qemu-system-x86_64 since it doesn't come with Container Linux releases currently.

After the static building I was trying to get it running under containerd's untrusted workloads (new in v1.1.0), then I realized I didn't have an image to feed to kata just sitting around and couldn't find one... so I found this project.

@t3hmrman

This comment has been minimized.

Copy link
Contributor Author

t3hmrman commented May 29, 2018

I've added the Dockerfile change I was making locally so maybe the Jenkins build will pass now, while I look at the patch format

[EDIT] - Looking at the patch format docs does it actually apply to Github PRs? I assume the sign off footer should go into the squashed commit?

@grahamwhaley

This comment has been minimized.

Copy link
Contributor

grahamwhaley commented May 29, 2018

Yeah, the sign off needs to be in the commits (each commit).
You only need one Fixes in a commit message per PR
Oh, btw, kata has its own osbuilder as well, that also does initrd images:
https://github.com/kata-containers/osbuilder

@t3hmrman

This comment has been minimized.

Copy link
Contributor Author

t3hmrman commented May 29, 2018

Thanks for clarifying -- if the build passes I'll go ahead and squash the commits and fix the commit message.

Thanks for the link to the other osbuilder, I'll make sure to use that in the future!

@t3hmrman t3hmrman force-pushed the t3hmrman:patch-1 branch from c736f07 to 05cce33 May 30, 2018

@t3hmrman

This comment has been minimized.

Copy link
Contributor Author

t3hmrman commented May 30, 2018

Hey @grahamwhaley I've fixed up the commit mesage, please let me know if anything else needs to be changed.

build: Fix Dockerfile & Makefile
Dockerfile was missing a required dep for newer kernels, Makefile was missing a `make`

Fixes: 54

Signed-off-by: t3hmrman <t3hmrman@gmail.com>

@t3hmrman t3hmrman force-pushed the t3hmrman:patch-1 branch from 05cce33 to cd80d8b May 31, 2018

@t3hmrman

This comment has been minimized.

Copy link
Contributor Author

t3hmrman commented May 31, 2018

Just realized I filed the ticket in the wrong place (clearcontainers/runtime), #54 is the right one, so I updated the commit message to reflect it

@grahamwhaley

This comment has been minimized.

Copy link
Contributor

grahamwhaley commented Jun 11, 2018

Sorry for the delay there @t3hmrman - my email system started to 'eat' emails for some reason, and I didn't see this. All green ... merging!

@grahamwhaley grahamwhaley merged commit 1c79623 into clearcontainers:master Jun 11, 2018

1 check passed

jenkins-ci-ubuntu-17-10 Build finished.
Details
@t3hmrman

This comment has been minimized.

Copy link
Contributor Author

t3hmrman commented Jun 12, 2018

No problem @grahamwhaley , glad I contribute a tiny bit to help -- thanks for all the work on the project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.