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

Merge service binaries into a single binary. #57

Merged
merged 11 commits into from
Oct 27, 2020
Merged

Merge service binaries into a single binary. #57

merged 11 commits into from
Oct 27, 2020

Conversation

arsing
Copy link
Member

@arsing arsing commented Oct 25, 2020

aziot-{cert,identity,key}d are now symlinks to a new aziotd binary.
This new binary looks at its argv[0] to figure out which service it's being
invoked as, and runs the corresponding service's code.

One advantage of this approach is that the common code for
reading config env vars, binding the service connector and
creating an HTTP server can be made common to all services.

The second advantage is that the three services can now share a single copy of
tokio, hyper, etc dependencies, that previously were statically linked into
each individual binary. This has savings for both disk space as well as
memory usage (from sharing code pages).

Making the startup sequence code common also means all services now
consistently use env_logger to set up logging. keyd and certd have been
updated to use the log crate instead of {e,}println!

Other changes:

  • Add support for reading multiple config files. The base file is still
    config.toml. In addition any files found under config.d will be applied
    as patches to the base config.

    This will be needed in the near future when iotedge init needs to add
    additional preloaded keys and certs to keyd's and certd's config.
    The same or a similar mechanism will also be useful for third-party packages
    to register principals with identityd.

  • Make naming of servers vs services vs APIs clearer.

    hyper owns the server. The server uses instances of a service to
    handle requests. The service implements the HTTP routes using an inner API.

  • Fix some tabs that rustfmt did not convert.

  • Don't run cargo fmt -- --check with --quiet regardless of the verbose flag,
    because it doesn't print errors if --quiet is specified.

  • Separate the rlib components of aziot-keys into aziot-keys-common.
    This is to avoid building aziot-keys as an rlib, because it triggers
    a cargo bug that causes it to be rebuilt over and over again even if it
    isn't modified.

    There were three things causing it to be built as an rlib:

    • Tests. The test target now excludes aziot-keys.

    • aziot used the PreloadedKeyLocation type from the crate. That type is
      now in the aziot-keys-common crate.

    • aziotd had it as a dependency to force it to be built first, because it
      has extern C fns that link to it. This is now enforced by the makefile
      instead.

  • Fix aziot init tests to be able to run in release mode, by moving
    the current-user-is-root check to the outer run fn. Also fix some tabs that
    rustfmt didn't convert to spaces in that file.

aziot-{cert,identity,key}d are now symlinks to a new aziotd binary.
This new binary looks at its `argv[0]` to figure out which service it's being
invoked as, and runs the corresponding service's code.

One advantage of this approach is that the common code for
reading config env vars, binding the service connector and
creating an HTTP server can be made common to all services.

The second advantage is that the three services can now share a single copy of
tokio, hyper, etc dependencies, that previously were statically linked into
each individual binary. This has savings for both disk space as well as
memory usage (from sharing code pages).

Making the startup sequence code common also means all services now
consistently use `env_logger` to set up logging. keyd and certd have been
updated to use the `log` crate instead of `{e,}println!`

Other changes:

- Add support for reading multiple config files. The base file is still
  config.toml. In addition any files found under `config.d` will be applied
  as patches to the base config.

  This will be needed in the near future when `iotedge init` needs to add
  additional preloaded keys and certs to keyd's and certd's config.
  The same or a similar mechanism will also be useful for third-party packages
  to register principals with identityd.

- Make naming of servers vs services vs APIs clearer.

  hyper owns the server. The server uses instances of a service to
  handle requests. The service implements the HTTP routes using an inner API.

- Fix some tabs that rustfmt did not convert.

- Don't run `cargo fmt -- --check` with `--quiet` regardless of the verbose flag,
  because it doesn't print errors if `--quiet` is specified.

- Separate the rlib components of aziot-keys into aziot-keys-common.
  This is to avoid building aziot-keys as an rlib, because it triggers
  a cargo bug that causes it to be rebuilt over and over again even if it
  isn't modified.

  There were three things causing it to be built as an rlib:

  - Tests. The test target now excludes aziot-keys.

  - aziot used the `PreloadedKeyLocation` type from the crate. That type is
    now in the aziot-keys-common crate.

  - aziotd had it as a dependency to force it to be built first, because it
    has extern C fns that link to it. This is now enforced by the makefile
    instead.
Makefile Show resolved Hide resolved
cert/aziot-certd/Cargo.toml Show resolved Hide resolved
cert/aziot-certd/src/http/mod.rs Outdated Show resolved Hide resolved
aziotd/src/main.rs Outdated Show resolved Hide resolved
aziotd/src/main.rs Show resolved Hide resolved
aziotd/src/main.rs Outdated Show resolved Hide resolved
aziotd/src/main.rs Outdated Show resolved Hide resolved
docs/dev/running/aziot-certd.md Outdated Show resolved Hide resolved
aziot/src/init.rs Outdated Show resolved Hide resolved
aziotd/src/main.rs Outdated Show resolved Hide resolved
aziotd/src/main.rs Outdated Show resolved Hide resolved
docs/dev/running/aziot-certd.md Show resolved Hide resolved
Copy link
Contributor

@massand massand left a comment

Choose a reason for hiding this comment

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

:shipit:

@kodiakhq kodiakhq bot merged commit 5e2070f into Azure:main Oct 27, 2020
@arsing arsing deleted the busybox branch October 27, 2020 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants