Consider merging affinity / anti-affinity endpoints in the external API

[benjaminleonard]

I wonder if we can simplify the API a little, which would also make it easier for the end-user to work with by merging affinity and anti-affinity endpoints.

We'd only really need to include a group type on the create action, and a type on the affinity group list and fetch. Everything else is the same from the users perspective.

cc @smklein

[smklein]

This separation was intentional, for what it's worth - there are follow-up PRs where the APIs between "affinity groups" and "anti-affinity groups" diverge more significantly.

As one example: #7572

Here, it is possible to add "affinity groups" as members within "anti-affinity groups". This is a way of saying "I want this bundle of VMMs to be anti-affine from everything else, even if I want that bundle to stick together".

There is no equivalent of this in the opposite direction -- "affinity groups" cannot themselves consist of "anti-affinity groups".

[smklein]

... but, I suppose it's worth asking -- do you think it's more or less confusing to have them overlap "most" of them time and diverge internally sometimes? or for them to be two totally distinct APIs?

[benjaminleonard]

... but, I suppose it's worth asking -- do you think it's more or less confusing to have them overlap "most" of them time and diverge internally sometimes? or for them to be two totally distinct APIs?

That's a good question! Is there a world in which the external APIs overlap completely and we validate the differences. E.g. rejecting affinity groups as members within anti-affinity groups.

[smklein]

I would prefer to define valid operations within the type system, where I can.

Following-up on my example, on the input path:

• Affinity Groups: Can add instances
  

  omicron/nexus/external-api/src/lib.rs

  Lines 1316 to 1326 in 804b2ee

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

• Anti-Affinity Groups: Can add instances or affinity groups
  

  omicron/nexus/external-api/src/lib.rs

  Lines 1424 to 1434 in 804b2ee

  	/// Add a member to an anti-affinity group (where that member is an instance)
  	#[endpoint {
  	method = POST,
  	path = "/v1/anti-affinity-groups/{anti_affinity_group}/members/instance/{instance}",
  	tags = ["affinity"],
  	}]
  	async fn anti_affinity_group_member_instance_add(
  	rqctx: RequestContext<Self::Context>,
  	query_params: Query<params::OptionalProjectSelector>,
  	path_params: Path<params::AntiAffinityInstanceGroupMemberPath>,
  	) -> Result<HttpResponseCreated<AntiAffinityGroupMember>, HttpError>;

  
  

  omicron/nexus/external-api/src/lib.rs

  Lines 1460 to 1470 in 804b2ee

  	/// Add a member to an anti-affinity group (where that member is an affinity group)
  	#[endpoint {
  	method = POST,
  	path = "/v1/anti-affinity-groups/{anti_affinity_group}/members/affinity-group/{affinity_group}",
  	tags = ["affinity"],
  	}]
  	async fn anti_affinity_group_member_affinity_group_add(
  	rqctx: RequestContext<Self::Context>,
  	query_params: Query<params::OptionalProjectSelector>,
  	path_params: Path<params::AntiAffinityAffinityGroupMemberPath>,
  	) -> Result<HttpResponseCreated<AntiAffinityGroupMember>, HttpError>;

And on the output path: "Which memberships are valid" is strongly typed

omicron/common/src/api/external/mod.rs

Lines 1317 to 1326 in 804b2ee

	/// A member of an Affinity Group
	///
	/// Membership in a group is not exclusive - members may belong to multiple
	/// affinity / anti-affinity groups.
	#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
	#[serde(tag = "type", content = "value", rename_all = "snake_case")]
	pub enum AffinityGroupMember {
	/// An instance belonging to this group, identified by UUID.
	Instance(InstanceUuid),
	}

omicron/common/src/api/external/mod.rs

Lines 1336 to 1348 in 804b2ee

	/// A member of an Anti-Affinity Group
	///
	/// Membership in a group is not exclusive - members may belong to multiple
	/// affinity / anti-affinity groups.
	#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
	#[serde(tag = "type", content = "value", rename_all = "snake_case")]
	pub enum AntiAffinityGroupMember {
	/// An affinity group belonging to this group, identified by UUID.
	AffinityGroup(AffinityGroupUuid),
	/// An instance belonging to this group, identified by UUID.
	Instance(InstanceUuid),
	}

These are different!

I want to understand a little better -- what is the painful part about having both in the API, from a CLI/console perspective? I understand that there is duplication here between both pathways, but my rationale for separating the types is that it would identify the objects as distinct, and let them grow independently, like the above example shows.

[benjaminleonard]

Thanks for explaining that, that makes sense. To be honest this is partly a design challenge for the docs to display longer lists of endpoints in an intuitive way. In that case I think we could probably close this.

[smklein]

Thanks for explaining that, that makes sense. To be honest this is partly a design challenge for the docs to display longer lists of endpoints in an intuitive way. In that case I think we could probably close this.

Ack, thanks for bringing it up. I'm happy to tag all of these endpoints as "related to affinity" for grouping, or re-open this if it feels particularly bad during integration regardless.