Skip to content

Conversation

@andrewjstone
Copy link
Contributor

The new zone type reflects the zone with which we'll deploy clickhouse server nodes in a replicated setup. We decided on using a new zone type rather than adding a boolean field to the existing Clickhouse zone type that is used for single server deployments in last Tuesday's (Aug 6, 2024) update huddle.

This is the fist part of the work to be done for replicated clickhouse deployments that are automated via reconfigurator. As such, the actual zone deployment is left as a todo.

The new zone type reflects the zone with which we'll deploy clickhouse
server nodes in a replicated setup. We decided on using a new zone type
rather than adding a boolean field to the existing `Clickhouse` zone
type that is used for single server deployments in last Tuesday's (Aug
6, 2024) update huddle.

This is the fist part of the work to be done for replicated clickhouse
deployments that are automated via reconfigurator. As such, the actual
zone deployment is left as a `todo`.
Copy link
Contributor

@karencfv karencfv 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!

Just a heads up that this is going to have conflicts with #6144

@andrewjstone andrewjstone enabled auto-merge (squash) August 13, 2024 17:42
@andrewjstone andrewjstone merged commit d25b102 into main Aug 13, 2024
@andrewjstone andrewjstone deleted the add-clickhouse-server-zone branch August 13, 2024 17:45
ZoneKind::BoundaryNtp | ZoneKind::InternalNtp => Self::NTP_PREFIX,
ZoneKind::Clickhouse => "clickhouse",
ZoneKind::ClickhouseKeeper => "clickhouse-keeper",
ZoneKind::ClickhouseServer => "clickhouse_server",
Copy link
Contributor

Choose a reason for hiding this comment

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

oh... this isn't going to work, needs a dash. (Should add a test for this!)

Copy link
Contributor

Choose a reason for hiding this comment

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

sunshowers added a commit that referenced this pull request Aug 13, 2024
)

Followup from #6297 -- `name_prefix` requires dashes.
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.

4 participants