If OXIDE_HOST is not set, we choose a random host from the user's config

[jgallagher]

In

oxide.rs/cli/src/context.rs

Lines 49 to 60 in f0ba6a2

	(Err(_), Ok(token)) => {
	let Some((host, _)) = config.hosts.hosts.iter().next() else {
	return Ok(None);
	};
	(host.clone(), token)
	}
	(Err(_), Err(_)) => {
	let Some((host, host_entry)) = config.hosts.hosts.iter().next() else {
	return Ok(None);
	};
	(host.clone(), host_entry.token.clone())
	}

, if OXIDE_HOST is not set, we choose a host by taking the "next" house out of a hashmap, which results in effectively a random host being chosen on each invocation (if the user has multiple host entries). This can create confusing results where the same command may succeed in one invocation and fail in the next, if the command was intended to be used with one of the specific hosts.

[jgallagher]

Closing as a dup of #302.

[karencfv]

Related: #6

[ahl]

Agreed that this behavior is lousy. What do we want to do here? I don't love $OXIDE_HOST as a primary interface

[karencfv]

Having the environment variables take precedence over the config file is not uncommon (AWS CLI does this as well).

It would be nice to be able to set a host as default though. My thinking is that having profiles (#6) is the best way to go long term, but perhaps not something that can just be whipped up and shipped quickly. Perhaps having the user have to choose a host from the list through a flag if they are logged in to more than one silo can be a start?

$ oxide some-command
Error: You are logged in to more than one host. Use the --host flag to select a host from your config file.

$ oxide some-command --host <host>
success

[ahl]

Having the environment variables take precedence over the config file is not uncommon (AWS CLI does this as well).

Yes! I'm just saying that it's lousy as the primary mechanism.

[askfongjojo]

Bumping this since our own folks run into this a lot and we can expect fleet admin who operates in a multi-tenant environment may hit the issue as well.
Note: We didn't think this was high priority previously because the most likely situation for this to happen was during initial setup - when the fleet admin has to make CLI requests against the recovery silo and then the one user silo they would create and keep using. At this time, we simply tell the user to set environment variables or manually edit the hosts.toml file to get rid of the recovery silo entry.

There are some suggestions on the desired behavior:

1. Error out - inform user that the endpoint/token setting is ambiguous, and that they can either set the environment variables or remove entries from the toml. (For the latter, we can provide a new CLI command to manage host entries so they don't mess up the config file due to typos.)
2. Prompt user to pass a --host parameter if there are multiple hosts in the toml file, as Karen already mentioned above. It may become annoying enough that user will eventually do one of the two things mentioned in 1.
3. Support the setting of a default host entry, e.g. by including a flag in each of the toml entries to keep track of the default.
4. Implement support named "profiles" #6.

[david-crespo]

Erroring out in case of ambiguity is something we could do immediately while deciding between fancier solutions.

[askfongjojo]

We have a customer who will be using multiple silos to partition dev and prod workloads. The change will likely cause many end-users to have multiple silo endpoint/tokens in their host config file. This CLI issue is going to be hit a lot more frequently (right now, it affects just the fleet admin user so we've been prioritizing it low).

@ahl - We need to bump up the priority of resolving this issue somehow.

[ahl]

Confirmed; I have a plan that I'll prototype ASAP for feedback.

[augustuswm]

Independent of the any end user configuration I think we could resolve the non-determinism by using a BTreeMap instead of HashMap when defining the Hosts struct. This would at least ensure that consecutive calls to the cli target the same host. Any reason not to make this change:

oxide.rs/sdk/src/config.rs

Lines 62 to 65 in 3ac6e32

	pub struct Hosts {
	#[serde(flatten)]
	pub hosts: HashMap<String, Host>,
	}

[david-crespo]

Only reason not to is if @ahl is planning on fixing this in the next few days anyway.

[ahl]

I believe this is addressed by #727. Please open a new issue if that's not the case.