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

Suggested Ceph package improvements #147801

Open
24 tasks
nh2 opened this issue Nov 29, 2021 · 15 comments
Open
24 tasks

Suggested Ceph package improvements #147801

nh2 opened this issue Nov 29, 2021 · 15 comments
Assignees

Comments

@nh2
Copy link
Contributor

nh2 commented Nov 29, 2021

As known from #42830 (comment), my company is using our own Ceph derivation instead of the nixpkgs one. After some of my contributions from that link were kindly incorporated by @krav, I'd like to spend some time to further improve the current nixpkgs Ceph package, so that I can switch to using that.

Before I start putting time into it, I'd like to share with other ceph package maintainers/contributors the current list of items I'd like to address.

Please let me know if you find anything problematic, or if you can answer some of these easily:

List:

  • mount -t ceph (kernel mounter) has a problem where it loses its pkgs.kmod (it needs modprobe on PATH)
    • The nix ceph package overrides the hardcode of /sbin/modprobe to just mount here.
    • But when mount -t ceph is invoked with proper PATH, it invokes mount.ceph without any environment variables, thus losing PATH (that happens here, calling system() here). I suspect this is because mount is a SUID program and clears its PATH for security, but I couldn't find a reference for that fact. We may need to explain that somewhere, and perhaps make modprobe available in general if we want mount -t ceph to work, or undo the overriding to call modprobe from PATH and put in a full path to "${pkgs.kmod}/bin/modprobe" (which will imply more frequent ceph package rebuilds).
  • ceph-fuse needs to be wrapped with pkgs.utillinux because it needs mount on PATH
  • ceph-fuse needs to be wrapped with pkgs.fuse because it needs fusermount on PATH
  • Split dependencies to multiline, so that patches are less likely to merge-conflict
  • sphinx should not be a runtime dependency (it currently is due to being bunched up in ceph-python-env)
  • Add ncurses to buildInputs,
    see e.g. https://github.com/ceph/ceph/blob/785edd08a00c359397e69e808350874886c3908c/doc/cephfs/cephfs-top.rst#L8
  • Add more justification comments to cmakeFlags
  • Why is cunit not in nativeBuildInputs? I suspect it should.
  • What's the comment # ceph 14 for?
  • (ensureNewerSourcesHook { year = "1980"; }) needs comment
  • substituteInPlace src/common/module.c should semantically better be in postPatch instead of preConfigure
  • substituteInPlace src/common/module.c should link to Don't hardcode executable paths in module.c ceph/ceph#20938
  • "-DWITH_SYSTEMD=OFF" needs comment
  • "-DWITH_TESTS=OFF" needs comment
  • check if these need to be added to Python env, compared to my current derivation (maybe they are no longer necessary in the current ceph version):
    • requests
    • jinja2
    • werkzeug
  • Comment why export PYTHONPATH is needed technically in addition to pythonPath =.
    Seee ceph: Bring back ceph-volume #78243 (comment)
  • Add @srhb back to maintainers, I think she was accidentally removed in 0dea5df because she was in only 1 of the 2 lists that were refactored into a variable.
  • Split up the build into 2 parts: The C++ part (e.g. ceph-unwrapped), and a symlinked derivation on top of it that has all the Python wrapping.
    This is so that one can very quickly iterate on Python wrapping, e.g. changing Python dependencies, without a multi-hour-rebuild.
    Check what @krav meant in Upgrade to Ceph 13 #42830 (comment):

    couldn't figure out how do debug yours with nix-shell on account of the python wrapping
    E.g. what exactly does "debug" refer to here?

  • Fix ceph health functionality not working due to problem with sudo: exit status: 1:
    Suggested Ceph package improvements #147801 (comment)
    # ceph device scrape-health-metrics
    # ceph device get-health-metrics TOSHIBA_MG08ACA16TEY_71B0A033FVNG
    {
        "20211231-005226": {
            "dev": "/dev/sdg",
            "error": "smartctl failed",
            "nvme_smart_health_information_add_log_error": "nvme returned an error: sudo: exit status: 1",
            "nvme_smart_health_information_add_log_error_code": -22,
            "nvme_vendor": "toshiba_mg08aca16tey",
            "smartctl_error_code": -22,
            "smartctl_output": "smartctl returned an error (1): stderr:\nsudo: exit status: 1\nstdout:\n"
        }
    }
    
  • Also linking here the custom NixOS service modules we wrote: https://gist.github.com/nh2/13425a1f18b4c1ce82edb63c10b163c9

Further issuess I found while testing Ceph 16 from nixos-21.11:

  • ceph-volume lvm zap errors due to lack of lsblk (from util-linux). It needs to be added to the wrapping.
  • Same for lvs (from lvm2)
@nh2 nh2 self-assigned this Nov 29, 2021
@nh2
Copy link
Contributor Author

nh2 commented Nov 29, 2021

For reference, this is my current derivation I'm comparing to, building 13.2.2 on nixos-21.11 (a newer version of #42830 (comment)):

{ stdenv, lib, fetchurl, fetchpatch, cmake, pkgconfig, makeWrapper
, python3
, python3Packages
, cunit
, lz4
, oathToolkit
, libuuid
, udev, libaio, utillinux, keyutils, fuse, libxfs
, leveldb, snappy, curl
, nss
, ncurses
, expat, boost, gperftools, gperf, yasm, rdma-core, kmod, cryptsetup, lvm2, coreutils, bzip2
}:

let
  version = "13.2.2";
  ceph-unwrapped = stdenv.mkDerivation {
    name = "ceph-unwrapped-${version}";

    src = fetchurl {
      url = "https://download.ceph.com/tarballs/ceph_${version}.orig.tar.gz";
      sha256 = "0h483n9iy0fkbqrhf7k0dzspwdpcaswkjwmc5n5c600fr6s1v9pk";
    };

    buildInputs = [
      udev
      libaio
      utillinux
      keyutils

      libuuid
      lz4
      leveldb
      snappy
      curl
      nss
      ncurses
      expat
      boost
      gperftools
      gperf
      fuse
      libxfs
      rdma-core
      oathToolkit
      bzip2
    ];

    nativeBuildInputs = [
      cmake
      cunit
      python3Packages.sphinx
      python3Packages.cython
      python3Packages.virtualenv
      python3Packages.pip
      yasm
      pkgconfig
      makeWrapper
    ];

    patches = [
      # TODO: remove when https://github.com/ceph/ceph/pull/21289 is merged
      ./ceph-volume-allow-loop.patch
      # TODO: remove when https://github.com/ceph/ceph/pull/20938 is merged
      ./dont-hardcode-bin-paths.patch
      (fetchpatch {
        name = "ceph-remove-subinterpreter-check.patch";
        url = "https://github.com/ceph/ceph/compare/v13.2.2...feb258244bacca0ffdcc7b6f562b2929d2e432b6.patch";
        sha256 = "0d9v3sqxr3zwlvqv1n3nllq3sdsq1xqrwlr9aai4f80zvllh3p6k";
      })
      # TODO: remove when over > 15.1.0
      ./fix-missing-include.patch
      (fetchpatch {
        name = "ceph-fix-python-const-char.patch";
        url = "https://github.com/ceph/ceph/commit/b29c65623f508082ded87af6f8d068ce8882f936.patch";
        sha256 = "sha256:0qggdqjivy9kbs7my38bjxifc2i8n34gh1hxi2nya18294si7cqk";
      })
      ./fix-include-assert.patch
      # Backport of fix for log spam every time `ceph` is invoked, with this warning:
      #     DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.10 it will stop working
      (fetchpatch {
        name = "ceph-Fix-python-import-deprecation-warnings.patch";
        # Commit-pinned equivalent of: https://github.com/nh2/ceph/compare/ceph:v13.2.2...13.2.2-python-warnings.patch
        url = "https://github.com/nh2/ceph/compare/ceph:v13.2.2...9be54a0bc3c340d87e51540b4f6d38df9e2c1f13.patch";
        sha256 = "1pfby711nhla64crhrpa0ayq795hi4y1qghswb20l7w12z3nxsrr";
      })
    ];

    preConfigure = ''
      pushd systemd
      # Checking systemd units are unchanged:
      echo "Actual systemd file hashes:"
      sha256sum *.target *.service *.service.in | tee actual-systemd-hashes
      echo "Diff of actual systemd file hashes with expected ones:"
      diff -u ${./expected-systemd-hashes} actual-systemd-hashes
      if [ $? != 0 ]; then
        echo "Ceph's systemd files have changed. Please ensure that the corresponding units in ceph.nix of the ceph NixOS service are up to date, and bump ./expected-systemd-hashes."
        exit 1
      fi
      popd

      patchShebangs .
    '';

    # Flags from https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=ceph-git#n142
    cmakeFlags = [
      "-DCMAKE_BUILD_TYPE=RelWithDebInfo"
      "-DWITH_SYSTEM_BOOST=ON"
      "-DWITH_PYTHON3=ON"
      "-DWITH_SYSTEMD=OFF" # We need to make custom Nix units anyway
      "-DWITH_EMBEDDED=OFF"
      "-DWITH_OPENLDAP=OFF"
      "-DWITH_LTTNG=OFF"
      "-DWITH_BABELTRACE=OFF"
      "-DWITH_TESTS=OFF"

      # Can't build this for now because we get a build error from it:
      #     Traceback (most recent call last):
      #      File "/tmp/nix-build-ceph-unwrapped-13.2.0.drv-7/ceph-13.2.0/build/src/pybind/mgr/dashboard/node-env/bin/pip", line 7, in <module>
      #        from pip._internal import main
      #     ModuleNotFoundError: No module named 'pip._internal'
      "-DWITH_MGR_DASHBOARD_FRONTEND=OFF"
      # Can't build this for now because the vendored `spdk` build complains:
      #     /tmp/nix-build-ceph-unwrapped-13.2.0.drv-1/ceph-13.2.0/src/spdk/include/spdk_internal/lvolstore.h:41:10: fatal error: uuid/uuid.h: No such file or directory
      # We haven't figured out yet what's the problem here.
      "-DWITH_SPDK=OFF"
      "-DXFS_INCLUDE_DIR=${libxfs}/include"

      # This is needed otherwise Ceph will try to use sys_siglist which is now deprecated.
      "-DWITH_REENTRANT_STRSIGNAL=ON"
    ];

    # Set the LD_LIBRARY_PATH, otherwise Cython can't find the ceph libraries during compilation
    # We also need to include our install dir in PYTHONPATH otherwise pip will refuse to install ceph-disk.
    preBuild = ''
      export LD_LIBRARY_PATH=$PWD/lib:$LD_LIBRARY_PATH
      export PYTHONPATH=$(toPythonPath $out):$PYTHONPATH
    '';

    enableParallelBuilding = true;
  };

# do the binary wrapping in a separate derivation so that we don't need to rebuild ceph if only this changes
in
let
  # See https://github.com/ceph/ceph/blob/v13.2.2/src/pybind/mgr/dashboard/requirements.txt and perhaps others.
  # Update the URL to newer versions on upgrades.
  pythonEnv = python3.withPackages (pkgs: with pkgs; [
      python
      flask
      prettytable
      requests
      (cherrypy.overrideDerivation (old: { doInstallCheck = false; }))
      jinja2
      pecan
      pyopenssl
      setuptools
      werkzeug
      Mako
      bcrypt
    ]);
in
stdenv.mkDerivation {
  name = "ceph-${version}";

  buildInputs = [ ceph-unwrapped ];
  nativeBuildInputs = [ makeWrapper python3Packages.python ];

  buildCommand = let
    extraPythonPaths = with python3Packages;
      map
        (path: "$(toPythonPath ${path})")
        [ "$out"
          # Python dependencies from: https://git.archlinux.org/svntogit/community.git/tree/trunk/PKGBUILD?h=packages/ceph#n142
       ];
    in ''
      set -eo pipefail
      cp -rvs ${ceph-unwrapped} --no-preserve=mode $out

      # Some executables in `bin` call out to Python; wrap them all with PYTHONPATH so that this works.
      for script in $out/bin/*; do
        echo "Adding Python paths to $script"
        wrapProgram $script --suffix-each PYTHONPATH : "$(toPythonPath $out):$(toPythonPath ${pythonEnv})" \
          --suffix PATH : "$out/bin"
      done

      wrapProgram $out/bin/mount.ceph --suffix PATH : ${kmod}/bin
      wrapProgram $out/bin/ceph-volume --suffix PATH : "${lvm2}/bin:${utillinux}/bin:${coreutils}/bin:${cryptsetup}/bin"
  '';
}

/

@Artturin
Copy link
Member

Artturin commented Dec 2, 2021

btw when leveldb is updated to 1.23 we will need this patch https://src.fedoraproject.org/rpms/leveldb/blob/rawhide/f/0006-revert-no-rtti.patch

https://tracker.ceph.com/issues/53060#note-1

@poelzi
Copy link
Member

poelzi commented Dec 8, 2021

We definitly should build a proper python-rados package instead of shipping the python libs with the ceph package

lukegb added a commit that referenced this issue Dec 27, 2021
LevelDB 1.23 forces -fno-rtti in their CMakeLists.txt, which breaks
downstream projects (e.g. Ceph).

See
#147801 (comment)
for some discussion about this.

OpenSUSE, Fedora, and Arch have all re-enabled RTTI in their packaging
of LevelDB as a result.
@nh2
Copy link
Contributor Author

nh2 commented Dec 30, 2021

I found another problem, added to issue description:

  • mount -t ceph (kernel mounter) has a problem where it loses its pkgs.kmod (it needs modprobe on PATH)
    • The nix ceph package overrides the hardcode of /sbin/modprobe to just mount here.
    • But when mount -t ceph is invoked with proper PATH, it invokes mount.ceph without any environment variables, thus losing PATH (that happens here, calling system() here). I suspect this is because mount is a SUID program and clears its PATH for security, but I couldn't find a reference for that fact. We may need to explain that somewhere, and perhaps make modprobe available in general if we want mount -t ceph to work, or undo the overriding to call modprobe from PATH and put in a full path to "${pkgs.kmod}/bin/modprobe" (which will imply more frequent ceph package rebuilds).

Example issue:

# mount -t ceph 10.1.3.4:/ /mycephmount -o name=admin,secretfile=/etc/ceph/ceph.mount.secret
sh: line 1: modprobe: command not found
failed to load ceph kernel module (127)

When that happens, mount hangs for a long time and is unkillable, namely in this syscall (as shown by strace):

mount("10.1.3.4:/", "/mycephmount", "ceph", 0, "name=admin,key=client.admin"

@nh2
Copy link
Contributor Author

nh2 commented Dec 31, 2021

And another one:

  • Fix ceph health functionality not working due to problem with sudo: exit status: 1:
    # ceph device scrape-health-metrics
    # ceph device get-health-metrics TOSHIBA_MG08ACA16TEY_71B0A033FVNG
    {
        "20211231-005226": {
            "dev": "/dev/sdg",
            "error": "smartctl failed",
            "nvme_smart_health_information_add_log_error": "nvme returned an error: sudo: exit status: 1",
            "nvme_smart_health_information_add_log_error_code": -22,
            "nvme_vendor": "toshiba_mg08aca16tey",
            "smartctl_error_code": -22,
            "smartctl_output": "smartctl returned an error (1): stderr:\nsudo: exit status: 1\nstdout:\n"
        }
    }
    

Edit: I found the solution.

In my own ceph service declaration, I'm using:

let
    # Utilities called by Ceph device health scraping, see:
    # https://docs.ceph.com/en/latest/rados/operations/devices/#enabling-monitoring
    # As per https://github.com/ceph/ceph-container/pull/1490/commits/c49e821599965ae92a88b2c78077ee03c4405895,
    # both the OSDs and the `mon` need this.
    # Ceph calls these utilities with `sudo`. That requires sudoers entries.
    # Sudoers entries require absolute path; that exact (nix store) path needs to
    # be used by Ceph, so it needs to be given to the systemd unit via `path`.
    # This is why we pair each `sudoersExtraRule` with the `package` to put onto
    # that `path`.
    #
    # Entries are based on:
    # https://github.com/ceph/ceph/blob/a2f5a3c1dbfa4dce41e25da4f029a8fdb8c8d864/sudoers.d/ceph-smartctl
    cephMonitoringSudoersCommandsAndPackages = [
      {
        package = pkgs.smartmontools;
        sudoersExtraRule = { # entry for `security.sudo.extraRules`
          users = [ config.users.users.ceph.name ];
          commands = [{
            command = "${lib.getBin pkgs.smartmontools}/bin/smartctl -x --json=o /dev/*";
            options = [ "NOPASSWD" ];
          }];
        };
      }
      {
        package = pkgs.nvme-cli;
        sudoersExtraRule = { # entry for `security.sudo.extraRules`
          users = [ config.users.users.ceph.name ];
          commands = [{
            command = "${lib.getBin pkgs.nvme-cli}/bin/nvme * smart-log-add --json /dev/*";
            options = [ "NOPASSWD" ];
          }];
        };
      }
    ];

    cephDeviceHealthMonitoringPathsOrPackages = with pkgs; [
      # Contains `sudo`. Ceph wraps this around the other health check programs.
      # Cannot use `pkgs.sudo` because that one is not SUID, see:
      # https://discourse.nixos.org/t/sudo-uid-issues/9133
      "/run/wrappers" # `systemd.services.<name>.path` adds the `bin/` subdir of this
    ] ++ map ({ package, ... }: package) cephMonitoringSudoersCommandsAndPackages;

in {
# ...

    # Allow ceph daemons (which run as user ceph) to collect device health metrics.
    security.sudo.extraRules =
      map ({ sudoersExtraRule, ... }: sudoersExtraRule) cephMonitoringSudoersCommandsAndPackages;

# ... in the declaration of the Ceph OSD services.systemd... unit:

      path = cephDeviceHealthMonitoringPathsOrPackages;

# ... in the declaration of the Ceph MON services.systemd... unit:

      path = cephDeviceHealthMonitoringPathsOrPackages;

}

nh2 pushed a commit to nh2/nixpkgs that referenced this issue Jan 2, 2022
LevelDB 1.23 forces -fno-rtti in their CMakeLists.txt, which breaks
downstream projects (e.g. Ceph).

See
NixOS#147801 (comment)
for some discussion about this.

OpenSUSE, Fedora, and Arch have all re-enabled RTTI in their packaging
of LevelDB as a result.

(cherry picked from commit 5d2fefe)

Cherry-Picked-By: Niklas Hambuechen <mail@nh2.me>
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 31, 2022
@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/while-mounting-ceph-filesystem-got-a-modprobe-not-found/23465/1

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 24, 2022
@ca5ua1
Copy link

ca5ua1 commented Jul 8, 2023

Can't use ceph-fuse. Here is example of output #46529 (comment) :(

@mweinelt
Copy link
Member

mweinelt commented Jul 8, 2023

@nh2 There currently isn't any active maintainer for the package since srhb dropped maintainership. I'm somewhat willing to review bits and pieces, so feel free to get started with all your ideas. If we considered to use ceph as a long-term solution, it would likely need to be in a better shape than today.

@ca5ua1
Copy link

ca5ua1 commented Jul 8, 2023

I'm pretty new to nix but I can help since I have working ceph and cephfs
Any safe operations on me.

Mounting with mount.ceph works as expected (thou no tests. Only successful remote files reading)

sudo mount.ceph shared@.cephfs=/services/shared /mnt/cephtest -o conf=/tmp/ceph.conf

@srhb
Copy link
Contributor

srhb commented Jul 10, 2023

Similarly to Casul51, I can also help (but I still don't have bandwidth for full-blown maintainership, sorry)

@ca5ua1
Copy link

ca5ua1 commented Jul 12, 2023

I'm pretty new to nix but I can help since I have working ceph and cephfs Any safe operations on me.

Mounting with mount.ceph works as expected (thou no tests. Only successful remote files reading)

sudo mount.ceph shared@.cephfs=/services/shared /mnt/cephtest -o conf=/tmp/ceph.conf

In (at least privileged) containers throws error but mounts anyway

sh: line 1: modprobe: command not found
failed to load ceph kernel module (127)

@ca5ua1
Copy link

ca5ua1 commented Jul 13, 2023

By the way I've managed get working ceph on privileged container

   fileSystems."/storage/shared" = {
    device = "user_that_you_created@ceph_fsid.cephfs_name=/directory/that/you/shared/for/the/user";
      fsType = "ceph";
      options = [
        "mon_addr=192.168.0.100"
        "conf=/var/keys/ceph_shared"
      ];
    };

/var/keys/ceph_shared contains fsid and mon_hosts from ceph and key for user. More info here https://docs.ceph.com/en/latest/rados/operations/user-management/#adding-a-user
and https://docs.ceph.com/en/latest/cephfs/mount-using-kernel-driver/

Gonna try on unprivileged later. I guess I tried that but it didn't work so why I started using privileged

@mweinelt
Copy link
Member

@nh2 Are you still planning to tackle a few of these issues?

@nh2
Copy link
Contributor Author

nh2 commented Dec 15, 2023

@nh2 Are you still planning to tackle a few of these issues?

Generally yes, but currently I am very short on time.

We use Ceph in production though so I'll have an interest in making it work.

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/idea-mkderivation-auto-generate-unwrapped-package-right-after-build/37901/4

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

No branches or pull requests

7 participants