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

[WIP] Add builds of Ceph and Rook binaries and OCIs #498

Open
wants to merge 5 commits into
base: 0-update-before-ceph
Choose a base branch
from

Conversation

celskeggs
Copy link
Member

@celskeggs celskeggs commented Feb 29, 2020

In order to meet our supply chain protection standards, we have to build our own versions of Rook and Ceph.

I am not yet planning on merging this PR, because I want to make more progress on Ceph before concluding that I did this part correctly, but I would like a code review as things stand.

The testing carried out for Ceph to determine that this worked correctly was manually deploying a container running the ceph image, execing into it with kubectl, and confirming that every one of the ceph commands could be launched. (This took a lot of iteration, to make sure all of the dependencies -- both built as part of Ceph and not built as part of Ceph -- worked out.) The same testing process was performed for the Rook binaries.

See #29.


Checklist:

  • I have split up this change into one or more appropriately-delineated commits.
  • The first line of each commit is of the form "[component]: do something"
  • I have written a complete, multi-line commit message for each commit.
  • I have formatted any Go code that I have changed with gofmt.
  • I have written or updated appropriate documentation to cover this change.
  • I have confirmed that this change is covered by at least one appropriate test run by CI.
  • If my change includes new or modified functionality, I have tested that the changes work as expected.
  • I have assigned this issue to an appropriate reviewer. (Choose @celskeggs if you are not otherwise certain.)
  • I consider my PR complete and ready to be merged without my further input, assuming that it passes CI and code review.
  • My changes have passed CI, including an automatic Jenkins deploy.
  • My changes have passed code review.

Copy link
Member

@cryslith cryslith left a comment

Choose a reason for hiding this comment

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

nice!

platform/ceph/deps.bzl Outdated Show resolved Hide resolved
# remove symlinks that create cycles (which break glob)
"find -name '.qa' -delete",
# remove filenames with ":" in them (which are disallowed in filegroups)
"rm src/test/common/test_blkdev_sys_block/sys/dev/block/8:0 src/test/common/test_blkdev_sys_block/sys/dev/block/9:0",
Copy link
Member

Choose a reason for hiding this comment

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

any reason not to use another find command for this? or at least to verify that there aren't any unexpected filenames with that character?

Copy link
Member Author

@celskeggs celskeggs Mar 3, 2020

Choose a reason for hiding this comment

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

There definitely might be some unexpected names that happen to include colons, but verifying that there aren't any further names like this is a great idea.

platform/ceph/deps_early.bzl Show resolved Hide resolved
build-chroot/packages.list Show resolved Hide resolved
platform/ceph/BUILD.bazel Show resolved Hide resolved
platform/ceph/BUILD.bazel Outdated Show resolved Hide resolved
@cryslith cryslith removed their assignment Mar 3, 2020
@celskeggs celskeggs self-assigned this Mar 3, 2020
# OOM killer gets fed up with Bazel, but I can't come up with a good
# way around that, and hopefully this code is correct enough that it
# never happens.
"make -j$(expr $(expr 1 + $(cat /proc/cpuinfo | cut -f 1 | grep -E '^processor$' | wc -l)) / 2)",
Copy link
Member

Choose a reason for hiding this comment

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

  • I don't think you need the nested $(expr ...) here, expr understands parentheses just fine.

  • use </proc/cpuinfo ..., not cat /proc/cpuinfo | ...

  • There are a couple ways to detect the inner command failing in bash, such as setting a variable and using an explicit test. But at this level of complexity it might be appropriate to refactor this into a separate script.

Copy link
Member Author

Choose a reason for hiding this comment

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

  • Will fix
  • Will fix
  • The make_commands parameter is badly implemented, and there appears to be no way to embed a reference to an external file in it, hence inlining the logic that really deserves a separate script. I will try to update the logic to be smarter, just inline.

@celskeggs celskeggs changed the title [WIP] ceph: add basic build of binaries and OCI [WIP] Add builds of Ceph and Rook binaries and OCIs Apr 3, 2020
@celskeggs celskeggs assigned cryslith and unassigned celskeggs Apr 3, 2020
@celskeggs celskeggs requested a review from cryslith April 3, 2020 03:30
platform/ceph/BUILD.bazel Show resolved Hide resolved
# which requires working pip in a virtualenv, which is not working for
# some reason relating to DNS resolution.
# (and it's too much hell to make it functional for me to want to deal
# with this mess right now.)
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a fine justification without this acerbic parenthetical :)

@cryslith cryslith assigned celskeggs and unassigned cryslith Apr 12, 2020
It takes so long to build Ceph that Jenkins times out. Increase the
amount of time we have to run our build so that this doesn't happen.
Ceph wants LVM on the server hosts. Make sure we include it, then.
The previous workspace format name corresponds to the import style used
by gazelle, but this particular etcd dependency isn't appropriate for
use in imports. Since rook is going to require an appropriate etcd
dependency, change this dependency to use a different name that won't
conflict with the importable one.
@celskeggs celskeggs force-pushed the 29-setup-ceph branch 3 times, most recently from cd6b40a to 9415874 Compare May 15, 2020 22:03
@celskeggs celskeggs changed the base branch from master to 0-update-before-ceph May 15, 2020 22:03
In order to meet our supply chain protection standards, we have to
build our own version of Ceph to use with Rook. This commit brings in
rules_foreign_cc, a set of bazel rules to more easily run cmake builds,
and then uses those rules to build Ceph using cmake.

Of course, we end up needing to patch both Ceph and rules_foreign_cc to
make them work correctly together, and we end up needing to install
many more packages into the build chroot.
In order to manage our disk cluster, we need a build of rook. Our
software supply chain standards mean that we have to build it
ourselves.

We also need to pull in numerous Rook dependencies, update the version
of at least one dependency, build tini, and package up Rook in a usable
OCI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants