Instance creation page doesn't allow attaching floating IPs if none existed when opened

[pietroalbini]

I needed to create an instance with a floating IP, so I opened the instance creation page in colo. I filled in all of the details, and then realized I forgot to create the floating IP ahead of time. I opened a new tab, created the IP, and then went back to the already opened instance creation page. Unfortunately the "Add floating IP" button was disabled, since when the page was loaded there were indeed no floating IPs. I had to reload the page (and lose the other settings I entered) to be able to add the IP at creation time.

[david-crespo]

Thanks for the report. In general, we could stand to have a lot more polling (#2867). But this is an especially interesting case because it is nested inside of a modal. Usually when I think about places to add polling, I'm thinking about top-level list views.

For you, this problem manifested as the button being disabled, but I'm pretty sure that even if, e.g., you already had one floating IP but you wanted to add another one, that also wouldn't have worked — you would have been able to open the list but it would be missing the newly created one because it wouldn't have refetched the floating IPs from the API.

This is because the query where we fetch the floating IPs is always active, regardless of whether the advanced accordion is open or the attach modal is open. It does not refetch when the modal opens and closes (though this would not help if the button is disabled to begin with).

console/app/forms/instance-create.tsx

Lines 616 to 649 in 1f2019c

	const AdvancedAccordion = ({
	control,
	isSubmitting,
	siloPools,
	}: {
	control: Control<InstanceCreateInput>
	isSubmitting: boolean
	siloPools: Array<SiloIpPool>
	}) => {
	// we track this state manually for the sole reason that we need to be able to
	// tell, inside AccordionItem, when an accordion is opened so we can scroll its
	// contents into view
	const [openItems, setOpenItems] = useState<string[]>([])
	const [floatingIpModalOpen, setFloatingIpModalOpen] = useState(false)
	const [selectedFloatingIp, setSelectedFloatingIp] = useState<FloatingIp | undefined>()
	const externalIps = useController({ control, name: 'externalIps' })
	const ephemeralIp = externalIps.field.value?.find((ip) => ip.type === 'ephemeral')
	const assignEphemeralIp = !!ephemeralIp
	const selectedPool = ephemeralIp && 'pool' in ephemeralIp ? ephemeralIp.pool : undefined
	const defaultPool = siloPools.find((pool) => pool.isDefault)?.name
	const attachedFloatingIps = (externalIps.field.value || []).filter(isFloating)
	const instanceName = useWatch({ control, name: 'name' })
	const { project } = useProjectSelector()
	const { data: floatingIpList } = usePrefetchedApiQuery('floatingIpList', {
	query: { project, limit: ALL_ISH },
	})
	// Filter out the IPs that are already attached to an instance
	const attachableFloatingIps = useMemo(
	() => floatingIpList.items.filter((ip) => !ip.instanceId),
	[floatingIpList]
	)

console/app/forms/instance-create.tsx

Lines 796 to 831 in 1f2019c

	<Modal
	isOpen={floatingIpModalOpen}
	onDismiss={closeFloatingIpModal}
	title="Attach floating IP"
	>
	<Modal.Body>
	<Modal.Section>
	<Message variant="info" content={selectedFloatingIpMessage} />
	<form>
	<Listbox
	name="floatingIp"
	items={availableFloatingIps.map((i) => ({
	value: i.name,
	label: <FloatingIpLabel ip={i} />,
	selectedLabel: `${i.name} (${i.ip})`,
	}))}
	label="Floating IP"
	onChange={(name) => {
	setSelectedFloatingIp(
	availableFloatingIps.find((i) => i.name === name)
	)
	}}
	required
	placeholder="Select a floating IP"
	selected={selectedFloatingIp?.name || ''}
	/>
	</form>
	</Modal.Section>
	</Modal.Body>
	<Modal.Footer
	actionText="Attach"
	disabled={!selectedFloatingIp}
	onAction={attachFloatingIp}
	onDismiss={closeFloatingIpModal}
	></Modal.Footer>
	</Modal>

Some possible mediocre local fixes:

• Move the modal rendering into its own component that actually mounts and unmounts when the button is clicked
• Add a second instance of the same query inside the modal

However! The simplest and most comprehensive fix is probably to take out refetchOnWindowFocus: false here, which means all queries would refetch when you come back from the other tab. Magic!

console/app/api/client.ts

Lines 90 to 98 in 1f2019c

	export const queryClient = new QueryClient({
	defaultOptions: {
	queries: {
	retry: false,
	staleTime: 2000,
	refetchOnWindowFocus: false,
	},
	},
	})

This line was put in 3 years ago, see #1232 (review). It was originally (Jan 2022) only on the /v1/me fetch but we moved it to be a global default. Wise comment from @zephraph 😁:

we should think more holistically about this from a design perspective. Having things pop out from under you is a bad experience but having to refresh over and over to see changes is also not great.

The original refetchOnWindowFocus: false for the session me fetch specifically was put in here and this is the reason: if your session happened to expire while you were gone, you would immediately get a jarring login redirect when you come back and all the queries refetch.

console/app/api/hooks.ts

Lines 49 to 57 in 1f2019c

	// if logged out, hit /login to trigger login redirect
	// Exception: 401 on password login POST needs to be handled in-page
	if (result.response.status === 401 && method !== 'loginLocal') {
	// TODO-usability: for background requests, a redirect to login without
	// warning could come as a surprise to the user, especially because
	// sometimes background requests are not directly triggered by a user
	// action, e.g., polling or refetching when window regains focus
	navToLogin({ includeCurrent: true })
	}

I think the best way to fix this is #1616, so that instead of getting that redirect, you would stay in the same spot but get a modal saying you've been logged out, click here to log back in. That may be harder to do than it sounds, though, because if you refetch a query and it comes back as an error, the component rendering that response (e.g., a list of floating IPs) now has nothing to render. We would have to make the queries smart, having them hold onto the last good response if there is one.

[pietroalbini]

Having background polling and things showing up automatically would indeed be amazing. For the record even a "refresh" icon next to the button would also work for me, even as a temporary fix.