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.

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

[PF-692] Pass workspace bucket zone/region parameter to Leonardo createRuntime… #2441

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
26 changes: 21 additions & 5 deletions src/components/NewRuntimeModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ImageDepViewer } from 'src/components/ImageDepViewer'
import { NumberInput, TextInput, ValidatedInput } from 'src/components/input'
import { withModalDrawer } from 'src/components/ModalDrawer'
import { InfoBox } from 'src/components/PopupTrigger'
import { regionInfo } from 'src/components/region-common'
import { SaveFilesHelp } from 'src/components/runtime-common'
import TitleBar from 'src/components/TitleBar'
import { cloudServices, machineTypes } from 'src/data/machines'
Expand All @@ -18,7 +19,7 @@ import Events, { extractWorkspaceDetails } from 'src/libs/events'
import {
currentRuntime, DEFAULT_DISK_SIZE, defaultDataprocMachineType, defaultGceMachineType, findMachineType, getDefaultMachineType,
persistentDiskCostMonthly,
runtimeConfigBaseCost, runtimeConfigCost
runtimeConfigBaseCost, runtimeConfigCost, DEFAULT_LOCATION, DEFAULT_LOCATION_TYPE
} from 'src/libs/runtime-utils'
import * as Style from 'src/libs/style'
import * as Utils from 'src/libs/utils'
Expand Down Expand Up @@ -187,11 +188,13 @@ export const NewRuntimeModal = withModalDrawer({ width: 675 })(class NewRuntimeM
cloudService: desiredRuntime.cloudService,
...(desiredRuntime.cloudService === cloudServices.GCE ? {
bootDiskSize: 50,
zone: desiredRuntime.zone,
machineType: desiredRuntime.machineType || defaultGceMachineType,
...(desiredRuntime.diskSize ? {
diskSize: desiredRuntime.diskSize
} : {})
} : {
region: desiredRuntime.region,
masterMachineType: desiredRuntime.masterMachineType || defaultDataprocMachineType,
masterDiskSize: desiredRuntime.masterDiskSize,
numberOfWorkers: desiredRuntime.numberOfWorkers,
Expand Down Expand Up @@ -251,7 +254,7 @@ export const NewRuntimeModal = withModalDrawer({ width: 675 })(class NewRuntimeM
withErrorReporting('Error creating cloud environment')
)(async () => {
const { onSuccess } = this.props
const { currentRuntimeDetails, currentPersistentDiskDetails } = this.state
const { currentRuntimeDetails, currentPersistentDiskDetails, bucketLocation, bucketLocationType } = this.state
const { runtime: existingRuntime, persistentDisk: existingPersistentDisk } = this.getExistingEnvironmentConfig()
const { runtime: desiredRuntime, persistentDisk: desiredPersistentDisk } = this.getDesiredEnvironmentConfig()
const shouldUpdatePersistentDisk = this.canUpdatePersistentDisk() && !_.isEqual(desiredPersistentDisk, existingPersistentDisk)
Expand All @@ -260,10 +263,12 @@ export const NewRuntimeModal = withModalDrawer({ width: 675 })(class NewRuntimeM
const shouldDeleteRuntime = existingRuntime && !this.canUpdateRuntime()
const shouldCreateRuntime = !this.canUpdateRuntime() && desiredRuntime
const { name, bucketName, googleProject } = this.getWorkspaceObj()
const { computeZone, computeRegion } = regionInfo(bucketLocation, bucketLocationType)

const runtimeConfig = desiredRuntime && {
cloudService: desiredRuntime.cloudService,
...(desiredRuntime.cloudService === cloudServices.GCE ? {
zone: computeZone,
machineType: desiredRuntime.machineType || defaultGceMachineType,
...(desiredRuntime.diskSize ? {
diskSize: desiredRuntime.diskSize
Expand All @@ -277,6 +282,7 @@ export const NewRuntimeModal = withModalDrawer({ width: 675 })(class NewRuntimeM
}
})
} : {
region: computeRegion,
masterMachineType: desiredRuntime.masterMachineType || defaultDataprocMachineType,
masterDiskSize: desiredRuntime.masterDiskSize,
numberOfWorkers: desiredRuntime.numberOfWorkers,
Expand Down Expand Up @@ -325,11 +331,12 @@ export const NewRuntimeModal = withModalDrawer({ width: 675 })(class NewRuntimeM
const {
deleteDiskSelected, selectedPersistentDiskSize, viewMode, masterMachineType,
masterDiskSize, sparkMode, numberOfWorkers, numberOfPreemptibleWorkers, workerMachineType,
workerDiskSize, jupyterUserScriptUri, selectedLeoImage, customEnvImage
workerDiskSize, jupyterUserScriptUri, selectedLeoImage, customEnvImage, bucketLocation, bucketLocationType
} = this.state
const { persistentDisk: existingPersistentDisk, runtime: existingRuntime } = this.getExistingEnvironmentConfig()
const cloudService = sparkMode ? cloudServices.DATAPROC : cloudServices.GCE
const desiredNumberOfWorkers = sparkMode === 'cluster' ? numberOfWorkers : 0
const { computeZone, computeRegion } = regionInfo(bucketLocation, bucketLocationType)
return {
runtime: Utils.cond(
[(viewMode !== 'deleteEnvironmentOptions'), () => {
Expand All @@ -338,13 +345,15 @@ export const NewRuntimeModal = withModalDrawer({ width: 675 })(class NewRuntimeM
toolDockerImage: selectedLeoImage === CUSTOM_MODE ? customEnvImage : selectedLeoImage,
...(jupyterUserScriptUri && { jupyterUserScriptUri }),
...(cloudService === cloudServices.GCE ? {
zone: computeZone,
machineType: masterMachineType || defaultGceMachineType,
...(this.shouldUsePersistentDisk() ? {
persistentDiskAttached: true
} : {
diskSize: masterDiskSize
})
} : {
region: computeRegion,
machineType: masterMachineType || defaultDataprocMachineType,
masterDiskSize,
numberOfWorkers: desiredNumberOfWorkers,
Expand All @@ -368,23 +377,26 @@ export const NewRuntimeModal = withModalDrawer({ width: 675 })(class NewRuntimeM
}

getExistingEnvironmentConfig() {
const { currentRuntimeDetails, currentPersistentDiskDetails } = this.state
const { currentRuntimeDetails, currentPersistentDiskDetails, bucketLocation, bucketLocationType } = this.state
const runtimeConfig = currentRuntimeDetails?.runtimeConfig
const cloudService = runtimeConfig?.cloudService
const numberOfWorkers = runtimeConfig?.numberOfWorkers || 0
const { computeZone, computeRegion } = regionInfo(bucketLocation, bucketLocationType)
return {
runtime: currentRuntimeDetails ? {
cloudService,
toolDockerImage: this.getImageUrl(currentRuntimeDetails),
...(currentRuntimeDetails?.jupyterUserScriptUri && { jupyterUserScriptUri: currentRuntimeDetails?.jupyterUserScriptUri }),
...(cloudService === cloudServices.GCE ? {
zone: computeZone,
machineType: runtimeConfig.machineType || defaultGceMachineType,
...(runtimeConfig.persistentDiskId ? {
persistentDiskAttached: true
} : {
diskSize: runtimeConfig.diskSize
})
} : {
region: computeRegion,
masterMachineType: runtimeConfig.masterMachineType || defaultDataprocMachineType,
masterDiskSize: runtimeConfig.masterDiskSize || 100,
numberOfWorkers,
Expand Down Expand Up @@ -476,7 +488,7 @@ export const NewRuntimeModal = withModalDrawer({ width: 675 })(class NewRuntimeM
withErrorReporting('Error loading cloud environment'),
Utils.withBusyState(v => this.setState({ loading: v }))
)(async () => {
const { googleProject } = this.getWorkspaceObj()
const { bucketName, namespace, googleProject, workspaceName } = this.getWorkspaceObj()
const currentRuntime = this.getCurrentRuntime()
const currentPersistentDisk = this.getCurrentPersistentDisk()

Expand All @@ -489,13 +501,17 @@ export const NewRuntimeModal = withModalDrawer({ width: 675 })(class NewRuntimeM
currentPersistentDisk ? Ajax().Disks.disk(currentPersistentDisk.googleProject, currentPersistentDisk.name).details() : null
])

const { location, locationType } = await Ajax().Workspaces.workspace(namespace, workspaceName).checkBucketLocation(bucketName)
Copy link
Contributor

@jdcanas jdcanas Apr 20, 2021

Choose a reason for hiding this comment

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

we may want to provide a default here in case the remote call fails (google down-time, etc), otherwise there will be some errors that cascade (thinking about the failure mode of this line in particular const { computeZone, clusterRegion } = regionInfo(bucketLocation, bucketLocationType). Unlikely that the app would actually work in the proposed scenario, but I like guarding against javascript crash scenarios wherever possible...


const imageUrl = currentRuntimeDetails ? this.getImageUrl(currentRuntimeDetails) : _.find({ id: 'terra-jupyter-gatk' }, newLeoImages).image
const foundImage = _.find({ image: imageUrl }, newLeoImages)
this.setState({
leoImages: newLeoImages, currentRuntimeDetails, currentPersistentDiskDetails,
selectedLeoImage: foundImage ? imageUrl : CUSTOM_MODE,
customEnvImage: !foundImage ? imageUrl : '',
jupyterUserScriptUri: currentRuntimeDetails?.jupyterUserScriptUri || '',
bucketLocation: location || DEFAULT_LOCATION,
bucketLocationType: locationType || DEFAULT_LOCATION_TYPE,
...this.getInitialState(currentRuntimeDetails, currentPersistentDiskDetails)
})
})
Expand Down
109 changes: 41 additions & 68 deletions src/components/region-common.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/libs/runtime-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import * as Utils from 'src/libs/utils'

export const DEFAULT_DISK_SIZE = 50

export const DEFAULT_LOCATION = 'US'
export const DEFAULT_LOCATION_TYPE = 'multi-region'

export const usableStatuses = ['Updating', 'Running']

export const defaultDataprocMachineType = 'n1-standard-2'
Expand Down