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.

Sign up for GitHub

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

Jump to bottom

[wip] Sketching out API, DB models for affinity and anti-affinity #7076

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[wip] Sketching out API, DB models for affinity and anti-affinity #7076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions common/src/api/external/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,31 @@ pub enum InstanceAutoRestartPolicy {
BestEffort,
}

// AFFINITY GROUPS

#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, JsonSchema)]
#[serde(rename_all = "snake_case")]
pub enum AffinityPolicy {
Allow,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know I'm calling this "Allow" instead of "Alert", but without a fleshed-out alerting system, I wanted to keep this honest.

We can add "Alert" later, when it can actually be implemented.

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense to me. Perhaps we should document the behavior here?

Fail,
}

#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, JsonSchema)]
#[serde(rename_all = "snake_case")]
pub enum AffinityDistance {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could rename this to "failure domain", if that seems useful?

Copy link
Member

Choose a reason for hiding this comment

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

I like "failure domain", seems more semantically meaningful IMO.

Sled,
}

#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
pub enum AffinityGroupMember {
Instance(Uuid),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know this is an "enum of one", but RFD 522 discusses having anti-affinity groups which contain "either instances or affinity groups". I figured I'd just define these as "members" to be flexible for future work

}

#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
pub enum AntiAffinityGroupMember {
Instance(Uuid),
}

// DISKS

/// View of a Disk
Expand Down
85 changes: 85 additions & 0 deletions nexus/db-model/src/affinity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/5.0/.

// Copyright 2024 Oxide Computer Company

//! Database representation of affinity and anti-affinity groups

use super::impl_enum_type;
use crate::schema::affinity_group;
use crate::schema::affinity_group_instance_membership;
use crate::schema::anti_affinity_group;
use crate::schema::anti_affinity_group_instance_membership;
use crate::typed_uuid::DbTypedUuid;
use chrono::{DateTime, Utc};
use omicron_uuid_kinds::AffinityGroupKind;
use omicron_uuid_kinds::AntiAffinityGroupKind;
use omicron_uuid_kinds::InstanceKind;

impl_enum_type!(
#[derive(SqlType, Debug)]
#[diesel(postgres_type(name = "affinity_policy", schema = "public"))]
pub struct AffinityPolicyEnum;

#[derive(Clone, Copy, Debug, AsExpression, FromSqlRow, PartialEq)]
#[diesel(sql_type = AffinityPolicyEnum)]
pub enum AffinityPolicy;

// Enum values
Fail => b"fail"
Allow => b"allow"
);

impl_enum_type!(
#[derive(SqlType, Debug)]
#[diesel(postgres_type(name = "affinity_distance", schema = "public"))]
pub struct AffinityDistanceEnum;

#[derive(Clone, Copy, Debug, AsExpression, FromSqlRow, PartialEq)]
#[diesel(sql_type = AffinityDistanceEnum)]
pub enum AffinityDistance;

// Enum values
Sled => b"sled"
);

#[derive(Queryable, Insertable, Clone, Debug, Selectable)]
#[diesel(table_name = affinity_group)]
pub struct AffinityGroup {
pub id: DbTypedUuid<AffinityGroupKind>,
pub name: String,
pub description: String,
pub time_created: DateTime<Utc>,
pub time_modified: DateTime<Utc>,
pub time_deleted: Option<DateTime<Utc>>,
pub policy: AffinityPolicy,
pub distance: AffinityDistance,
}

#[derive(Queryable, Insertable, Clone, Debug, Selectable)]
#[diesel(table_name = anti_affinity_group)]
pub struct AntiAffinityGroup {
pub id: DbTypedUuid<AntiAffinityGroupKind>,
pub name: String,
pub description: String,
pub time_created: DateTime<Utc>,
pub time_modified: DateTime<Utc>,
pub time_deleted: Option<DateTime<Utc>>,
pub policy: AffinityPolicy,
pub distance: AffinityDistance,
}
Comment on lines +47 to +71
Copy link
Member

Choose a reason for hiding this comment

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

Part of me wonders whether it makes sense to use the same model to represent these, since they share all the same fields...but I think the type-level distinction between what's an affinity group and what's an anti-affinity group will probably prevent bugs in the future, and using different typed UUIDs for the IDs is definitely the right move. Potentially shared bits like the policy and distance fields could be factored out later if there was a need to have functions that can operate on either, but at present it seems like there probably isn't!


#[derive(Queryable, Insertable, Clone, Debug, Selectable)]
#[diesel(table_name = affinity_group_instance_membership)]
pub struct AffinityGroupInstanceMembership {
pub group_id: DbTypedUuid<AffinityGroupKind>,
pub instance_id: DbTypedUuid<InstanceKind>,
}

#[derive(Queryable, Insertable, Clone, Debug, Selectable)]
#[diesel(table_name = anti_affinity_group_instance_membership)]
pub struct AntiAffinityGroupInstanceMembership {
pub group_id: DbTypedUuid<AntiAffinityGroupKind>,
pub instance_id: DbTypedUuid<InstanceKind>,
}
Comment on lines +73 to +85
Copy link
Member

Choose a reason for hiding this comment

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

I note that these lack created/deleted timestamps, implying that:

  1. we intend to hard-delete rather than soft-delete them, and,
  2. we don't presently record when instances were added to affinity/anti-affinity groups, so we can't present that in UIs in the future.

I'm not sure if either of these matter to us, but I figured I'd comment on it.

2 changes: 2 additions & 0 deletions nexus/db-model/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ extern crate diesel;
extern crate newtype_derive;

mod address_lot;
mod affinity;
mod allow_list;
mod bfd;
mod bgp;
Expand Down Expand Up @@ -128,6 +129,7 @@ mod db {
pub use self::macaddr::*;
pub use self::unsigned::*;
pub use address_lot::*;
pub use affinity::*;
pub use allow_list::*;
pub use bfd::*;
pub use bgp::*;
Expand Down
40 changes: 40 additions & 0 deletions nexus/db-model/src/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,46 @@ table! {
}
}

table! {
affinity_group (id) {
id -> Uuid,
name -> Text,
description -> Text,
time_created -> Timestamptz,
time_modified -> Timestamptz,
time_deleted -> Nullable<Timestamptz>,
policy -> crate::AffinityPolicyEnum,
distance -> crate::AffinityDistanceEnum,
}
}

table! {
anti_affinity_group (id) {
id -> Uuid,
name -> Text,
description -> Text,
time_created -> Timestamptz,
time_modified -> Timestamptz,
time_deleted -> Nullable<Timestamptz>,
policy -> crate::AffinityPolicyEnum,
distance -> crate::AffinityDistanceEnum,
}
}

table! {
affinity_group_instance_membership (group_id, instance_id) {
group_id -> Uuid,
instance_id -> Uuid,
}
}

table! {
anti_affinity_group_instance_membership (group_id, instance_id) {
group_id -> Uuid,
instance_id -> Uuid,
}
}
Comment on lines +497 to +509
Copy link
Member

Choose a reason for hiding this comment

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

I presume we'll want to add joinable!/allow_tables_to_appear_in_same_query! for joining the affinity/anti-affinity group tables on the instances table?


table! {
metric_producer (id) {
id -> Uuid,
Expand Down
144 changes: 144 additions & 0 deletions nexus/external-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,150 @@ pub trait NexusExternalApi {
disk_to_detach: TypedBody<params::DiskPath>,
) -> Result<HttpResponseAccepted<Disk>, HttpError>;

// Affinity Groups

/// List affinity groups
#[endpoint {
method = GET,
path = "/v1/affinity-groups",
tags = ["affinity"],
}]
async fn affinity_group_list(
rqctx: RequestContext<Self::Context>,
query_params: Query<PaginatedByNameOrId<params::ProjectSelector>>,
) -> Result<HttpResponseOk<ResultsPage<views::AffinityGroup>>, HttpError>;

/// List members of an affinity group
#[endpoint {
method = GET,
path = "/v1/affinity-groups/{affinity_group}",
tags = ["affinity"],
}]
async fn affinity_group_member_list(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::OptionalProjectSelector>,
path_params: Path<params::AffinityGroupPath>,
) -> Result<HttpResponseOk<ResultsPage<AffinityGroupMember>>, HttpError>;
Comment on lines +1265 to +1275
Copy link
Member

Choose a reason for hiding this comment

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

This returns a ResultsPage but appears to be missing pagination params --- should it be paginated? Or am I missing something?


/// Add a member to an affinity group
#[endpoint {
method = POST,
path = "/v1/affinity-groups/{affinity_group}/{member}",
tags = ["affinity"],
}]
async fn affinity_group_member_add(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::OptionalProjectSelector>,
path_params: Path<params::AffinityGroupMemberPath>,
) -> Result<HttpResponseUpdatedNoContent, HttpError>;

/// Remove a member from an affinity group
#[endpoint {
method = DELETE,
path = "/v1/affinity-groups/{affinity_group}/{member}",
tags = ["affinity"],
}]
async fn affinity_group_member_delete(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::OptionalProjectSelector>,
path_params: Path<params::AffinityGroupMemberPath>,
) -> Result<HttpResponseDeleted, HttpError>;

/// Create an affinity group
#[endpoint {
method = POST,
path = "/v1/affinity-groups",
tags = ["affinity"],
}]
async fn affinity_group_create(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::ProjectSelector>,
new_affinity_group_params: TypedBody<params::AffinityGroupCreate>,
) -> Result<HttpResponseCreated<views::AffinityGroup>, HttpError>;

/// Delete an affinity group
#[endpoint {
method = DELETE,
path = "/v1/affinity-groups/{affinity_group}",
tags = ["affinity"],
}]
async fn affinity_group_delete(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::OptionalProjectSelector>,
path_params: Path<params::AffinityGroupPath>,
) -> Result<HttpResponseDeleted, HttpError>;

/// List anti-affinity groups
#[endpoint {
method = GET,
path = "/v1/anti-affinity-groups",
tags = ["affinity"],
}]
async fn anti_affinity_group_list(
rqctx: RequestContext<Self::Context>,
query_params: Query<PaginatedByNameOrId<params::ProjectSelector>>,
) -> Result<HttpResponseOk<ResultsPage<views::AntiAffinityGroup>>, HttpError>;

/// List members of an anti-affinity group
#[endpoint {
method = GET,
path = "/v1/anti-affinity-groups/{anti_affinity_group}",
tags = ["affinity"],
}]
async fn anti_affinity_group_member_list(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::OptionalProjectSelector>,
path_params: Path<params::AntiAffinityGroupPath>,
) -> Result<HttpResponseOk<ResultsPage<AntiAffinityGroupMember>>, HttpError>;
Comment on lines +1336 to +1346
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, should this be paginated?


/// Add a member to an anti-affinity group
#[endpoint {
method = POST,
path = "/v1/anti-affinity-groups/{anti_affinity_group}/{member}",
tags = ["affinity"],
}]
async fn anti_affinity_group_member_add(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::OptionalProjectSelector>,
path_params: Path<params::AntiAffinityGroupMemberPath>,
) -> Result<HttpResponseUpdatedNoContent, HttpError>;

/// Remove a member from an anti-affinity group
#[endpoint {
method = DELETE,
path = "/v1/anti-affinity-groups/{anti_affinity_group}/{member}",
tags = ["affinity"],
}]
async fn anti_affinity_group_member_delete(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::OptionalProjectSelector>,
path_params: Path<params::AntiAffinityGroupMemberPath>,
) -> Result<HttpResponseDeleted, HttpError>;

/// Create an anti-affinity group
#[endpoint {
method = POST,
path = "/v1/anti-affinity-groups",
tags = ["affinity"],
}]
async fn anti_affinity_group_create(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::ProjectSelector>,
new_affinity_group_params: TypedBody<params::AntiAffinityGroupCreate>,
) -> Result<HttpResponseCreated<views::AntiAffinityGroup>, HttpError>;

/// Delete an anti-affinity group
#[endpoint {
method = DELETE,
path = "/v1/anti_affinity-groups/{anti_affinity_group}",
tags = ["affinity"],
}]
async fn anti_affinity_group_delete(
rqctx: RequestContext<Self::Context>,
query_params: Query<params::OptionalProjectSelector>,
path_params: Path<params::AntiAffinityGroupPath>,
) -> Result<HttpResponseDeleted, HttpError>;

// Certificates

/// List certificates for external endpoints
Expand Down
Loading
Loading