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

cri-o, buildah: add wrappers #86637

Merged
merged 5 commits into from May 25, 2020
Merged

cri-o, buildah: add wrappers #86637

merged 5 commits into from May 25, 2020

Conversation

@zowoq
Copy link
Contributor

zowoq commented May 3, 2020

Opened as a draft, waiting for other changes to land in master.

Having a wrapper might be preferred but for now I'd rather that we merge this and reevaluate if we end up adding more packages.

Ended up being easier to add the wrapper.

cc @NixOS/podman

@zowoq zowoq marked this pull request as draft May 3, 2020
@zowoq zowoq force-pushed the zowoq:crio-wrapper branch 2 times, most recently from daaced1 to 5310841 May 3, 2020
@zowoq zowoq force-pushed the zowoq:crio-wrapper branch from 5310841 to 22405a4 May 5, 2020
@zowoq zowoq changed the title cri-o: wrap packages nixos/cri-o: set paths May 5, 2020
@zowoq zowoq marked this pull request as ready for review May 5, 2020
Copy link
Member

saschagrunert left a comment

LGTM

@zowoq zowoq marked this pull request as draft May 5, 2020
@zowoq zowoq mentioned this pull request May 5, 2020
6 of 10 tasks complete
@zowoq zowoq force-pushed the zowoq:crio-wrapper branch from 22405a4 to 9a41262 May 7, 2020
@zowoq zowoq force-pushed the zowoq:crio-wrapper branch from 9a41262 to 1f6c72c May 11, 2020
@zowoq zowoq force-pushed the zowoq:crio-wrapper branch from 1f6c72c to 640c9ea May 12, 2020
@zowoq zowoq changed the title nixos/cri-o: set paths cri-o, buildah: add wrappers May 12, 2020
@zowoq zowoq removed the request for review from nlewo May 17, 2020
@zowoq zowoq force-pushed the zowoq:crio-wrapper branch from 640c9ea to 74c8170 May 17, 2020
@zowoq zowoq marked this pull request as ready for review May 17, 2020
@zowoq
Copy link
Contributor Author

zowoq commented May 17, 2020

Needs to be rebased after #86290 is merged.

Done.

[crio.runtime]
conmon = "${pkgs.conmon}/bin/conmon"

This comment has been minimized.

Copy link
@zowoq

zowoq May 17, 2020

Author Contributor

@saschagrunert Apologies for asking you to keep this in #86290 and then removing it here anyway, the wrapper end up being the best approach.

This comment has been minimized.

Copy link
@saschagrunert

saschagrunert May 18, 2020

Member

Yeah no worries ❤️

@ofborg ofborg bot requested a review from kalbasit May 17, 2020
@zowoq zowoq force-pushed the zowoq:crio-wrapper branch from 74c8170 to 6ffb0a0 May 17, 2020
@Profpatsch
Copy link
Member

Profpatsch commented May 25, 2020

I merged #86290 so this should be good to go?

@zowoq zowoq force-pushed the zowoq:crio-wrapper branch from 6ffb0a0 to 4c2765e May 25, 2020
@zowoq
Copy link
Contributor Author

zowoq commented May 25, 2020

I merged #86290 so this should be good to go?

Yes, thanks @Profpatsch

mkdir -p $out/bin
ln -s ${buildah-unwrapped}/share $out/share
makeWrapper ${buildah-unwrapped}/bin/buildah $out/bin/buildah \
--prefix PATH : ${binPath}

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch May 25, 2020

Member

iirc you don’t need a separate derivation to wrap the executable, you can just run makeWrapper in the postInstall phase of the original derivation.

This comment has been minimized.

Copy link
@zowoq

zowoq May 25, 2020

Author Contributor

This and the top-level -unwrapped are duplicated from podman.

For now I'd like to keep them as duplicates so they are easier to merge into one wrapper. I'm looking at doing that after adding some tests.

@@ -1369,7 +1369,8 @@ in

btfs = callPackage ../os-specific/linux/btfs { };

buildah = callPackage ../development/tools/buildah { };
buildah = callPackage ../development/tools/buildah/wrapper.nix { };
buildah-unwrapped = callPackage ../development/tools/buildah { };

This comment has been minimized.

Copy link
@Profpatsch

Profpatsch May 25, 2020

Member

Do we need to expose the unwrapped version?

, utillinux # nsenter
, cni-plugins # not added to path
, iptables
}:

This comment has been minimized.

Copy link
@saschagrunert

saschagrunert May 25, 2020

Member

We still need socatfor port forwarding purposes.

This comment has been minimized.

Copy link
@zowoq

zowoq May 25, 2020

Author Contributor

Fixed.

@zowoq zowoq force-pushed the zowoq:crio-wrapper branch from 4c2765e to 13bcb8d May 25, 2020
@ofborg ofborg bot requested review from saschagrunert and Profpatsch May 25, 2020
Copy link
Member

saschagrunert left a comment

LGTM

@Profpatsch Profpatsch merged commit 37a87c3 into NixOS:master May 25, 2020
17 checks passed
17 checks passed
buildah, buildah.passthru.tests, cri-o, cri-o.passthru.tests, podman, podman.passthru.tests on x86_64-darwin No attempt
Details
Evaluation Performance Report Evaluator Performance Report
Details
buildah, buildah.passthru.tests, cri-o, cri-o.passthru.tests, podman, podman.passthru.tests on aarch64-linux Success
Details
buildah, buildah.passthru.tests, cri-o, cri-o.passthru.tests, podman, podman.passthru.tests on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="13bcb8d"; rev="13bcb8d2bb2067b51f342ada39d92dcfaaf58ed3"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="13bcb8d"; rev="13bcb8d2bb2067b51f342ada39d92dcfaaf58ed3"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="13bcb8d"; rev="13bcb8d2bb2067b51f342ada39d92dcfaaf58ed3"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="13bcb8d"; rev="13bcb8d2bb2067b51f342ada39d92dcfaaf58ed3"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="13bcb8d"; rev="13bcb8d2bb2067b51f342ada39d92dcfaaf58ed3"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="13bcb8d"; rev="13bcb8d2bb2067b51f342ada39d92dcfaaf58ed3"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="13bcb8d"; rev="13bcb8d2bb2067b51f342ada39d92dcfaaf58ed3"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@zowoq zowoq deleted the zowoq:crio-wrapper branch May 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.