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

Unify services and datasets, external and internal DNS are datasets too #3390

Merged
merged 16 commits into from
Jun 29, 2023

Conversation

smklein
Copy link
Collaborator

@smklein smklein commented Jun 21, 2023

Prelude

This combines:

Into one PR. Trying to split them out resulted in intermediate states that were not worthwhile debugging - I'd rather invest the energy into fixing this "more-desirable end-state".

What does this PR do

  • (Internal and External) DNS services are explicitly backed by datasets, making it possible to "delete and re-initialize" arbitrary zones without data loss. This is necessary for correct behavior in cold boot.
  • Merges the concept of "datasets" into "services".
    • Previously, "datasets" were considered services managing data, and "services" were considered services without datasets - the two were totally disjoint. This was a bit arbitrary, and made it more complicated to conform sleds during service management.
    • As one example of an inconsistency, "requesting services" was vectorized ("ask for all services that should be on the sled"), "requesting datasets" was not (it was an ask-for-one-at-a-time API).
    • Now, "services" are universal, and they may optionally manage datasets. This unifies pathways a bit, and simplifies the API.

Fixes #3018
Part of #2978

illumos-utils/src/zpool.rs Show resolved Hide resolved
sled-agent/src/bootstrap/agent.rs Show resolved Hide resolved
sled-agent/src/http_entrypoints.rs Show resolved Hide resolved
sled-agent/src/params.rs Show resolved Hide resolved
sled-agent/src/rack_setup/service.rs Outdated Show resolved Hide resolved
sled-agent/src/sled_agent.rs Show resolved Hide resolved
@smklein smklein marked this pull request as ready for review June 22, 2023 21:41
@smklein
Copy link
Collaborator Author

smklein commented Jun 27, 2023

Rebased around the service bundle changes - this is still ready for review!

common/src/sql/dbinit.sql Outdated Show resolved Hide resolved
smf/external-dns/config.toml Outdated Show resolved Hide resolved
smf/internal-dns/config.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@bnaecker bnaecker left a comment

Choose a reason for hiding this comment

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

Looks good to me, a few nits, but nice work overall!

illumos-utils/src/zpool.rs Show resolved Hide resolved
sled-agent/src/rack_setup/service.rs Outdated Show resolved Hide resolved
sled-agent/src/sled_agent.rs Outdated Show resolved Hide resolved
@smklein smklein enabled auto-merge (squash) June 28, 2023 22:35
@smklein smklein merged commit cdf91f2 into main Jun 29, 2023
@smklein smklein deleted the unify branch June 29, 2023 00:36
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.

DNS zones should provision storage in a dataset explicitly
4 participants