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

feat: add a new shim for lunatic #122

Merged
merged 4 commits into from
Aug 23, 2023
Merged

feat: add a new shim for lunatic #122

merged 4 commits into from
Aug 23, 2023

Conversation

0xE282B0
Copy link
Contributor

Shim for Lunatic "an Erlang-inspired runtime for WebAssembly".

@0xE282B0
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Liquid Reply"

}
}

impl Executor for LunaticExecutor {
Copy link
Member

Choose a reason for hiding this comment

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

I like that it's already using libcontainer!!

@Mossaka
Copy link
Member

Mossaka commented Aug 16, 2023

Ready for review?

@0xE282B0
Copy link
Contributor Author

@Mossaka, I wanted to use LibcontainerInstance but can't make it build.

When I enable the libcontainer feature on containerd-shim-wasm it adds libcontainer/default features. With that I need to add th packages libdbus-1-dev and libsystemd0 finally I get the error:

/usr/local/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: 
    warning: libpthread.so.0, needed by /usr/lib/x86_64-linux-gnu/libdbus-1.so, not found (try using -rpath or -rpath-link)

and

/usr/local/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: /usr/lib/x86_64-linux-gnu/libdbus-1.so: undefined reference to `sd_listen_fds@LIBSYSTEMD_209'
/usr/local/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: /usr/lib/x86_64-linux-gnu/libdbus-1.so: undefined reference to `sd_is_socket@LIBSYSTEMD_209'
/usr/local/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: /usr/lib/x86_64-linux-gnu/libdbus-1.so: undefined reference to `pthread_mutexattr_settype@GLIBC_2.2.5'
/usr/local/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: /usr/lib/x86_64-linux-gnu/libdbus-1.so: undefined reference to `pthread_mutexattr_init@GLIBC_2.2.5'
/usr/local/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: /usr/lib/x86_64-linux-gnu/libdbus-1.so: undefined reference to `pthread_mutexattr_destroy@GLIBC_2.2.5'
/usr/local/bin/../lib/gcc/x86_64-linux-musl/9.2.0/../../../../x86_64-linux-musl/bin/ld: /usr/lib/x86_64-linux-gnu/libdbus-1.so: undefined reference to `pthread_condattr_setclock@GLIBC_2.3.3'

Does the libcontainer feater really need to depend on systemd and dbus or am I doing something completely wrong?

@0xE282B0
Copy link
Contributor Author

When I remove the default features on libcontainer cross compilation works:
containerd/runwasi@00dca5c

@Mossaka, do we need systemd in libcontainer? Is there a way to cross compile with systemd/dbus dependencies?

@0xE282B0 0xE282B0 reopened this Aug 17, 2023
@Mossaka
Copy link
Member

Mossaka commented Aug 17, 2023

@0xE282B0 can you try "cgroupsv2" feature?

@Mossaka, do we need systemd in libcontainer?

No, musl doesn't have systemd.

@Mossaka
Copy link
Member

Mossaka commented Aug 17, 2023

@0xE282B0 I have a PR containerd/runwasi#246 in runwasi to fix this issue. Once it's merged, you can use cgroupsv2 feature to remove dependencies on systemd and libseccump. This should make it compilable against musl.

@jprendes
Copy link
Contributor

FYI, you can build with the systemd feature in alpine, but:

  • you have to vendor dbus, see here and here
  • if you want to statically link libseccomp you need a few env-vars, see here

@jprendes
Copy link
Contributor

The systemd feature in youki doesn't link agains libsystemd. It links agains dbus instead.
The dbus crate can be vendored without involving systemd. I do not know if that actually does something in systems without systemd

@Mossaka
Copy link
Member

Mossaka commented Aug 18, 2023

@0xE282B0 I pushed a new commit that updates to the latest main branch of runwasi. It should build now.

@0xE282B0
Copy link
Contributor Author

@jprendes I think for now it's ok to build the shim without dbus, but statically linking libseccomp is interesting.
@Mossaka Thanks for updating the shim already! I was afraid I would have to wait an hour to see if the workflow was complete 😅

Test just fails with 0/3 nodes available: 3 node(s) had untolerated taint {node.kubernetes.io/disk-pressure: }
I set this now to "Ready for review".

@0xE282B0 0xE282B0 marked this pull request as ready for review August 18, 2023 06:23
Copy link
Member

@Mossaka Mossaka left a comment

Choose a reason for hiding this comment

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

LGTM!

I am going to test this locally and then if everything runs fine, will approve this PR.

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
@Mossaka
Copy link
Member

Mossaka commented Aug 18, 2023

I am running into this problem from the containerd log:

[ERROR] executable "/" does not have the correct permission set
[ERROR] failed to initialize container process: invalid executable: /

I think the issue was the libcontainer requires the entrypoint to be an executable, which we tried to push off. See youki-dev/youki#2220.

In the meantime, you probably want to do something like the spin shim is doing described in the #120.

@Mossaka
Copy link
Member

Mossaka commented Aug 18, 2023

I just pushed a new commit to make lunatic wasm binaries executable.

make integration-tests is working on my local!! 🎉

@Mossaka
Copy link
Member

Mossaka commented Aug 18, 2023

Actually, all the pods are running fine, but the response I got is a bit off. Is this expected?

curl http://localhost:8082/lunatic
<h1>404: Not found</h1>

curl http://localhost:8082/lunatic/hello/     
Hello :)

@0xE282B0
Copy link
Contributor Author

0xE282B0 commented Aug 18, 2023

The path is like spin and wws and the text is just the submillisecond example. What would you expect?

Ah, the integration test URL did not match.

@Mossaka
Copy link
Member

Mossaka commented Aug 18, 2023

The path is like spin and wws and the text is just the submillisecond example. What would you expect?

Thanks for the clarification! I thought we are running the images/lunatic example which confuses me. Now that makes sense.

Ah, the integration test URL did not match.

Yes, I just pushed a commit to fix it.

@Mossaka
Copy link
Member

Mossaka commented Aug 18, 2023

Could you please sign this commit ea3fd9c?

@Mossaka
Copy link
Member

Mossaka commented Aug 21, 2023

@0xE282B0 in PR #125, I am trying to fix the CI with splitting build and integration tests into two jobs and thus reduce the disk pressure issue this PR is facing.

@Mossaka
Copy link
Member

Mossaka commented Aug 22, 2023

Need a rebase to main branch

0xE282B0 and others added 2 commits August 22, 2023 22:19
update to the latest main branch of runwasi

Signed-off-by: jiaxiao zhou <[email protected]>

delete command in workload.yaml and make lunatic executable

Signed-off-by: jiaxiao zhou <[email protected]>

tests: fix the integration tests for lunatic

this commits fix the integration tests for lunatic shim by
adding hello to the end of the url for curling.

It also removes unused code in mod.rs

Signed-off-by: jiaxiao zhou <[email protected]>
Signed-off-by: jiaxiao zhou <[email protected]>
Signed-off-by: jiaxiao zhou <[email protected]>
@Mossaka
Copy link
Member

Mossaka commented Aug 23, 2023

Merging it in!! Thanks @0xE282B0 again for contributing. 🥂

@Mossaka Mossaka merged commit f66a8f3 into deislabs:main Aug 23, 2023
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