IP ranges in pools should only be IPv4

[david-crespo]

The IP range add/remove endpoints allow pairs of v4 or v6 addresses, and have done so since they were added in #1253 (June 2022).

omicron/common/src/address.rs

Lines 351 to 361 in 235420e

	/// An IP Range is a contiguous range of IP addresses, usually within an IP
	/// Pool.
	///
	/// The first address in the range is guaranteed to be no greater than the last
	/// address.
	#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize)]
	#[serde(untagged)]
	pub enum IpRange {
	V4(Ipv4Range),
	V6(Ipv6Range),
	}

I only just found out that while the API lets you add and remove them, IPv6 address ranges will not actually work from a networking POV — it's possible we can allocate an IPv6 address to an instance, but the instance will not be reachable through it.

The simplest thing would be to change the endpoint to take an Ipv4Range instead of an IpRange. That is probably not as a simple as it sounds — there will be a lot of fiddly changes to the tests.

omicron/nexus/src/external_api/http_entrypoints.rs

Lines 1731 to 1741 in 235420e

	/// Add range to IP pool
	#[endpoint {
	method = POST,
	path = "/v1/system/ip-pools/{pool}/ranges/add",
	tags = ["system/networking"],
	}]
	async fn ip_pool_range_add(
	rqctx: RequestContext<Arc<ServerContext>>,
	path_params: Path<params::IpPoolPath>,
	range_params: TypedBody<shared::IpRange>,
	) -> Result<HttpResponseCreated<IpPoolRange>, HttpError> {

[bnaecker]

Could you explain more about why we'd remove this? It's true that support for IPv6 has been deferred, but it's not abandoned AFAIK. One cannot add an IPv6 address to an instance at this point, since there are a number of places that check that condition and return a 400. A client gets a pretty immediate notification that this failed, rather than just an unreachable instance.

[david-crespo]

I can soften the issue title: really what I want to do is make sure the user and operator don't have an egregiously bad experience around this, and disallowing IPv6 ranges altogether is only one approach among others.

What I want to avoid is mysterious failures in this scenario:

• IP pool P1 has an IPv6 range in it (whether it also has IPv4 ranges may or may not matter)
• P1 is the default pool for silo S1
• A user in S1 tries to create an instance, automatically pulling an external IP from the default pool

What I need to find out in detail is: what happens at this point? Where does the 400 actually happen?

• Does the user get a 400 on instance create because their default pool is configured incorrectly? We know there are various ways the operator can misconfigure IP pools so that instance external IP allocation just doesn't work. Do we consider the presence of an IPv6 range in the pool at all to be such a misconfiguration?
• Do the next external IP queries know to ignore IPv6 ranges and pull from IPv4 ranges if there are any? (I don't see any evidence of this in the code, but I didn't look very long). This would mean that IPv6 being present doesn't break things, it only breaks if there are also no IPv4 ranges.
• Does the allocation of an IPv6 address work fine, and the API/console will tell the user that's their external IP, but then it just doesn't work?

Right now, the help text in the create IP range form says the IPs must be either v4 or v6, implicitly endorsing the possibility of using v6. At the very least I want to fix that. But I don't see why we would accept something in the API that we know cannot work, and it seems pretty light-weight to me to stop the operator from adding an IPv6 range at the API layer while leaving everything below that IP version agnostic (or however agnostic it currently is) until we can properly accept IPv6.

[bnaecker]

What I want to avoid is mysterious failures in this scenario:

I agree.

What I need to find out in detail is: what happens at this point? Where does the 400 actually happen?

It does not today, which is news to me. When we added all these APIs, there were several places that checked for requests for external IPv6 and returned a 400. I can't find those anymore. I'm not sure what actually happens, but I'm looking at it now.

[bnaecker]

Ok, so this is definitely not great right now. You certainly can create a pool with IPv6 ranges, and instance with an IPv6 address from that pool.

bnaecker@shale : ~/omicron $ oxide ip-pool range list --pool 2ab2c2f6-dd89-4107-a478-3ebd80827262
IpPoolRange {
    id: b34db578-c7de-4da1-9789-0ea81196cd0c,
    ip_pool_id: 2ab2c2f6-dd89-4107-a478-3ebd80827262,
    range: V6(
        Ipv6Range {
            first: fd00::1,
            last: fd00::10,
        },
    ),
    time_created: 2024-02-16T21:16:01.893937Z,
}
bnaecker@shale : ~/omicron $ oxide instance list --project test
Instance {
    description: "my inst",
    hostname: "test",
    id: 8344c09c-1ca6-43d5-aa85-6962c7ba40d0,
    memory: ByteCount(
        1073741824,
    ),
    name: Name(
        "test",
    ),
    ncpus: InstanceCpuCount(
        2,
    ),
    project_id: e49c211b-603e-49ba-95d4-455176f965fc,
    run_state: Stopped,
    time_created: 2024-02-16T21:17:18.784391Z,
    time_modified: 2024-02-16T21:17:18.784391Z,
    time_run_state_updated: 2024-02-16T21:18:40.983355Z,
}
bnaecker@shale : ~/omicron $ oxide instance external-ip list --instance 8344c09c-1ca6-43d5-aa85-6962c7ba40d0
success
ExternalIpResultsPage {
    items: [
        ExternalIp {
            ip: fd00::2,
            kind: Ephemeral,
        },
    ],
    next_page: None,
}

But you get a 500 when starting the instance:

bnaecker@shale : ~/omicron $ oxide instance start --instance 8344c09c-1ca6-43d5-aa85-6962c7ba40d0
error
Error Response: status: 500 Internal Server Error; headers: {"content-type": "application/json", "x-request-id": "56c0099b-2d58-45c6-a88c-cbccde1c07f1", "content-length": "124", "date": "Fri, 16 Feb 2024 21:18:41 GMT"}; value: Error { error_code: Some("Internal"), message: "Internal Server Error", request_id: "56c0099b-2d58-45c6-a88c-cbccde1c07f1" }

That comes from here:

{"msg":"request completed","v":0,"name":"nexus","level":30,"time":"2024-02-16T21:18:41.144205856Z","hostname":"oxz_nexus_9daee106-8cf8-4873-96ae-c6e0925b38d8","pid":2054,"uri":"//v1/instances/8344c09c-1ca6-43d5-aa85-6962c7ba40d0/start","method":"POST","req_id":"56c0099b-2d58-45c6-a88c-cbccde1c07f1","remote_addr":"192.168.1.82:39978","local_addr":"172.30.2.5:80","component":"dropshot_external","name":"9daee106-8cf8-4873-96ae-c6e0925b38d8","file":"/home/bnaecker/.cargo/git/checkouts/dropshot-a4a923d29dccc492/711a749/dropshot/src/server.rs:837","error_message_external":"Internal Server Error","error_message_internal":"saga ACTION error at node \"dpd_ensure\": ipv6 nat is not yet implemented","latency_us":330254,"response_code":"500"}

So the current blocker is that IPv6 NAT isn't implemented, here:

omicron/nexus/src/app/instance_network.rs

Lines 480 to 485 in 9e9e60c

	IpNetwork::V6(_v6net) => {
	// TODO: implement handling of v6 nat.
	return Err(Error::InternalError {
	internal_message: "ipv6 nat is not yet implemented".into(),
	});
	}

Given how much this has obviously changed since I've last looked at the code, I have no idea how many other blockers exist even if we clear this one.

I generally agree with your point @david-crespo, that the current behavior is confusing and a bad experience. I also don't particularly want to unwind support or modify tests, since we do still want to support IPv6. (I don't know how far away we are from that though.)

Rather than changing the API, tests, etc., could we instead quickly fail a request if IPv6 is used? I think doing so at the range-add API would make sense, but I'm not positive about that.

[david-crespo]

Yes, I think 400ing on IPv6 address range add would not be very invasive and it avoids the whole problem while we figure things out.