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

Remove stage 2 and replace w/ a static docker registry implementation in rust #948

Merged
merged 58 commits into from
Nov 14, 2022

Conversation

Noxsios
Copy link
Contributor

@Noxsios Noxsios commented Oct 27, 2022

Description

Replaces the 2-stage injector systems with a single Rust binary that:

  1. assembles the configmap chunks
  2. extracts the archive
  3. validates the payload shasum
  4. serves the image via an embedded OCI distribution endpoint in Rust

Related Issue

Fixes #708

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist before merging

  • Tests have been added/updated as necessary (add the needs-tests label)
  • Documentation has been updated as necessary (add the needs-docs label)
  • An ADR has been written as necessary (add the needs-adr label) [ 1 2 3 ]
  • (Optional) Changes have been linted locally with golangci-lint. (NOTE: We haven't turned on lint checks in the pipeline yet so linting may be hard if it shows a lot of lint errors in places that weren't touched by changes. Thus, linting is optional right now.)

@jeff-mccoy jeff-mccoy added needs-adr needs-docs PR Label - Docs required to merge labels Oct 27, 2022
Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

Small stuff looking through the code

src/injector/stage1/Cargo.toml Outdated Show resolved Hide resolved
src/injector/stage1/Cargo.toml Outdated Show resolved Hide resolved
src/injector/stage1/release.sh Outdated Show resolved Hide resolved
src/injector/stage1/src/main.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@jeff-mccoy jeff-mccoy left a comment

Choose a reason for hiding this comment

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

We won't be able to merge this until it is off of nightly, have you tried evaluating with MUSL or do you have a better idea when they will call this magic binary shrinking fun stable?

@Noxsios
Copy link
Contributor Author

Noxsios commented Oct 28, 2022

To replicate (currently) working w/ a local built binary:

cd src/injector/stage1
./release.sh
cd ../../../
make init-package
make build-cli-mac-apple ARCH=arm64
cd build
# <blah blah> = ./zarf-mac-apple version
zarf tools archiver decompress zarf-init-arm64-v0.22.2-<blah blah>.tar.zst init
cp ../src/injector/stage1/target/aarch64-unknown-linux-musl/release/zarf-injector init/components/zarf-injector/files/0
rm zarf-init-arm64-*
zarf tools archiver compress init/. zarf-init-arm64-<blah blah>.tar.zst
./zarf-mac-apple init -l=trace

@Noxsios
Copy link
Contributor Author

Noxsios commented Oct 28, 2022

Current sizes w/ stable optimizations:

x86_64 binary size: 873k
aarch64 binary size: 750k

src/injector/stage1/src/main.rs Outdated Show resolved Hide resolved
@Madeline-UX
Copy link
Contributor

@Noxsios can you point me to the docs you created for this? I can support you by helping review those.

@jeff-mccoy
Copy link
Contributor

@Noxsios as you test the local distros, please drop a screenshot in this PR and note the distribution, thanks.

@Noxsios
Copy link
Contributor Author

Noxsios commented Nov 7, 2022

@Noxsios as you test the local distros, please drop a screenshot in this PR and note the distribution, thanks.

❌ Microk8s: Unable to deploy due to not supporting http docker registries. Regular zarf init from master branch also failed, so this is more of zarf not working on Microk8s and less the changes in this PR.

📍 TKG: TKG community edition got nuked. Will attempt and update w/ "free" TKG download.

❌ AKS: AKS is very broken.

✅ Docker Desktop K8s: working flawlessly.

@jeff-mccoy
Copy link
Contributor

Confirmed the injector functioned properly in EKS in addition to local clusters already tested.
Screenshot 2022-11-13 at 10 32 45 PM

Copy link
Contributor

@jeff-mccoy jeff-mccoy left a comment

Choose a reason for hiding this comment

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

Overall lgtm, already made most of the changes I wanted to see in the other PR that was merged into this one. Only area I'd like to leave as a future thing to look at is the FormatCraneOCILayout function. Think there's some things we can do to clean that up, especially when it comes to manually defining the struct vs using an existing library we likely already import (go-containertools or registry).

@jeff-mccoy jeff-mccoy merged commit d3e7c16 into main Nov 14, 2022
@jeff-mccoy jeff-mccoy deleted the rust-stage-2 branch November 14, 2022 04:41
Noxsios added a commit that referenced this pull request Mar 8, 2023
…ap count (#948)

Replaces the 2-stage injector systems with a single Rust binary that:
1. assembles the configmap chunks
2. extracts the archive
3. validates the payload shasum
4. serves the image via an embedded OCI distribution endpoint in Rust

Fixes #708 

Co-authored-by: Megamind <[email protected]>
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.

Creating a Zarf package fails on zarf-injector step
5 participants