-
Notifications
You must be signed in to change notification settings - Fork 131
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
Updated Kubernetes Readme for more information about using docker images #846
Updated Kubernetes Readme for more information about using docker images #846
Conversation
b6e91fe
to
8d3a1a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good. Can you link from the top level README to this deployent example if there is a good place to suggest that people look at Kubernetes deployment examples? I'm not sure if there is a good place as this has been around for a while.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: pre-commit-checks, and 12 discussions need to be resolved
-- commits
line 2 at r1:
Updated -> Update
-- commits
line 2 at r1:
nit: Try to shorten the title
.github/styles/config/vocabularies/TraceMachina/accept.txt
line 29 at r1 (raw file):
remoteable Chromium namespace
This is breaking pre-commit hooks because the last line doesn't have a trailing newline. To fix it:
# The current status of your Pr
git status
# invoke the pre-commit hooks (only works in the nix flake)
pre-commit run -a
# view the changed files
git status
# in this case it should've modified the `accept.txt` file
git add .github/styles/config/vocabularies/TraceMachina/accept.txt
# Amend the commit
git commit --amend
deployment-examples/kubernetes/README.md
line 78 at r1 (raw file):
Use a published image
Published images can be found under the Container registry, which uses the namespace
https://ghcr.io
. The Container registry offers benefits such as granular permissions and storage optimizations for images.
I'm not sure whether this is correct. AFAIU the only thing the remote registry gives us is "prebuilt" images which effectively means that they're just faster to start than when you'd build the image youreself first.
deployment-examples/kubernetes/README.md
line 81 at r1 (raw file):
To pull an existing image, you can run: ```sh
nit: Consider surrounding code blocks with newlines to reduce the changes which will be necessary when we add markdownlint to our pre-commit hooks: https://github.com/markdownlint/markdownlint/blob/main/docs/RULES.md#md031---fenced-code-blocks-should-be-surrounded-by-blank-lines
deployment-examples/kubernetes/README.md
line 85 at r1 (raw file):
Tag an OCI image
nit: Consider surrounding headings with newlines: https://github.com/markdownlint/markdownlint/blob/main/docs/RULES.md#md022---headers-should-be-surrounded-by-blank-lines
deployment-examples/kubernetes/README.md
line 86 at r1 (raw file):
## Tag an OCI image You can tag an image by invoking `nix eval` and either specifying the commit hash you want to build or point to the current state of the upstream NativeLink main branch. For example, to tag an image with a specific commit hash, run the `nix eval` command and change `someCommit` with the commit hash you want to use:
This is not exactly correct. The nix eval
command doesn't tag an image. Instead, it derives/computes the tag that nix would generate if you were to build the image and push it to a registry.
When you do
nix run github:TraceMachina/nativelink/someCommit#image.copyToDockerDaemon
the image you get when you run docker image ls
would have the tag:
nix eval github:TraceMachina/nativelink/someCommit#image.imageTag --raw
deployment-examples/kubernetes/README.md
line 93 at r1 (raw file):
Alternatively, the tag can be derived from the upstream sources at the current state of the upstream main branch by running this command: ```sh nix eval github:TraceMachina/nativelink --raw
This nix eval
command doesn't give you the tag of the image. Instead would give you the evaluated output of the entire nativelink
flake (which probably doesn't work).
The image tags for the nativelink
container images are autogenerated from the inputs to the image
output. Since this image output is only dependent on the nativelink
executable, its hash is only derived from the rust sources, it's dependencies, and the container-specific config options. It's not derived from the entire nativelink repository. For instance, the tag wouldn't change if you modified some random non-rust file that doesn't influence the nativelink build.
Something to not confuse here is that .#image
and .#image.imageTag
are two different expressions. nix eval .#image
would try to evaluate theimage
flake output (which probably doesn't work because you can't "print" an image). nix eval .#image.imageTag
just evaluates the imageTag
field in the image
output, which is effectively just a string.
deployment-examples/kubernetes/README.md
line 107 at r1 (raw file):
> retrieve an image: > ```sh > nix eval github:TraceMachina/nativelink.image.> imageTag --raw
Probably a typo which meant to say:
nix eval github:TraceMachina/nativelink.image.imageTag --raw
deployment-examples/kubernetes/README.md
line 126 at r1 (raw file):
```sh `nix run .#image
You can't run the .#image
output, as it's just a json file which looks like this (you can verify this by running nix build
and printing the output of the result
symlink that the build command creates):
{
"version": 1,
"image-config": {
"Entrypoint": [
"/nix/store/wswj7zid10blknd6kcg4xp8w0ri81d3x-nativelink-0.3.0/bin/nativelink"
],
"Labels": {
"org.opencontainers.image.description": "An RBE compatible, high-performance cache and remote executor.",
"org.opencontainers.image.documentation": "https://github.com/TraceMachina/nativelink",
"org.opencontainers.image.licenses": "Apache-2.0",
"org.opencontainers.image.revision": "aab867a79396f1aefba7030fcd1128822107a2e0-dirty",
"org.opencontainers.image.source": "https://github.com/TraceMachina/nativelink",
"org.opencontainers.image.title": "NativeLink",
"org.opencontainers.image.vendor": "Trace Machina, Inc."
}
},
"layers": [
{
"digest": "sha256:22e91acf12aba41ba8469db4a900baa550803fa3f4289f5d997683916c920353",
"size": 29400576,
"diff_ids": "sha256:22e91acf12aba41ba8469db4a900baa550803fa3f4289f5d997683916c920353",
"paths": [
{
"path": "/nix/store/wswj7zid10blknd6kcg4xp8w0ri81d3x-nativelink-0.3.0"
}
],
"mediatype": "application/vnd.oci.image.layer.v1.tar"
}
],
"arch": "amd64"
}
The way to make this image runnable is to invoke the copyTo
or copyToRegistry
or copyToDockerDaemon
attributes on the image. See this for more information:
https://github.com/nlewo/nix2container/tree/master
Something you could nix run
is the actual nativelink executable which you can run via a raw nix run
in the cloned repository without any additional arguments (but note that this is running the executable natively, not invoking any image).
deployment-examples/kubernetes/README.md
line 129 at r1 (raw file):
Below are examples within the NativeLink repository for running an image:
- Example 1 highlights:
nit: It's probably a good idea to mention the actual documentation of copyTo
and friends: https://github.com/nlewo/nix2container
deployment-examples/kubernetes/README.md
line 138 at r1 (raw file):
docker-daemon:nativelink:''${IMAGE_TAG}
You can also refer to the SECURITY.md for more details around using OCI images.
nit: we might want to note that the published images are signed using cosign
and that verification instructions can be found in SECURITY.md
Let me make a new PR with the top-level readme changes. |
8d3a1a9
to
c88afab
Compare
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Update Kubernetes Readme is ok? |
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
This is from type of verbage came from github. |
Previously, nfarah86 (nadine farah) wrote…
This type of verbage came from github... I can amend it to include your comment. |
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
fixed |
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
fixed |
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
updated to clarify this |
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
@aaronmondal I think we should not mention this if it probably doesn't work |
Previously, nfarah86 (nadine farah) wrote…
updated |
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Done. |
c88afab
to
d050856
Compare
d050856
to
ba95b2e
Compare
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Done. |
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04, and 7 discussions need to be resolved
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Updated -> Update
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), macos-13, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), macos-13, ubuntu-20.04 / stable, ubuntu-22.04 / stable, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04
Description
Updated deployment-examples/kubernetes/README.md for more context around tagging, building, and running images.
Fixes # (issue)
The original question for this improvement popped up here: https://nativelink.slack.com/archives/C066XC8JT0X/p1711386866760809
Type of change
Please delete options that aren't relevant.
Checklist
git amend
see some docsThis change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)