Skip to content

Commit

Permalink
PR Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
grolu committed Aug 31, 2022
1 parent 545528c commit 23a3e0b
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 44 deletions.
2 changes: 1 addition & 1 deletion frontend/src/components/ShootWorkers/MachineImage.vue
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export default {
},
computed: {
machineImageItems () {
const machineImages = this.machineImages.slice()
const machineImages = [...this.machineImages]
if (this.notInList) {
machineImages.push({
...this.worker.machine.image,
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/ShootWorkers/MachineType.vue
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export default {
},
computed: {
machineTypeItems () {
const machineTypes = this.machineTypes.slice()
const machineTypes = [...this.machineTypes]
if (this.notInList) {
machineTypes.push({
name: this.worker.machine.type
Expand Down
16 changes: 9 additions & 7 deletions frontend/src/components/ShootWorkers/WorkerGroup.vue
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ export default {
},
workerGroupDescriptions () {
const description = []
if (this.workerGroup.machine.architecture) {
const machineArchitecture = this.workerGroup.machine.architecture
if (machineArchitecture) {
description.push({
title: 'Machine Architecture',
value: this.workerGroup.machine.architecture
value: machineArchitecture
})
}

Expand All @@ -97,13 +98,14 @@ export default {
}
description.push(this.machineImageDescription)

if (this.workerGroup.cri) {
const cri = this.workerGroup.cri
if (cri) {
let value
if (this.workerGroup.cri.containerRuntimes && this.workerGroup.cri.containerRuntimes.length) {
const containerRuntimes = map(this.workerGroup.cri.containerRuntimes, 'type')
value = `${this.workerGroup.cri.name} (${join(containerRuntimes, ', ')})`
if (cri.containerRuntimes?.length) {
const containerRuntimes = map(cri.containerRuntimes, 'type')
value = `${cri.name} (${join(containerRuntimes, ', ')})`
} else {
value = this.workerGroup.cri.name
value = cri.name
}
description.push({
title: 'Container Runtime',
Expand Down
70 changes: 45 additions & 25 deletions frontend/src/components/ShootWorkers/WorkerInputGeneric.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ SPDX-License-Identifier: Apache-2.0
color="primary"
item-color="primary"
:items="machineArchitectures"
:error-messages="getErrorMessages('worker.machine.architecture')"
@input="onInputMachineArchitecture"
@blur="$v.worker.machine.architecture.$touch()"
v-model="worker.machine.architecture"
:error-messages="getErrorMessages('machineArchitectureInternal')"
@blur="$v.machineArchitectureInternal.$touch()"
v-model="machineArchitectureInternal"
label="Architecture">
</v-select>
</div>
Expand Down Expand Up @@ -155,6 +154,7 @@ import difference from 'lodash/difference'
import get from 'lodash/get'
import set from 'lodash/set'
import head from 'lodash/head'
import pick from 'lodash/pick'
import { required, maxLength, minValue, requiredIf } from 'vuelidate/lib/validators'
import { getValidationErrors, parseSize } from '@/utils'
import { uniqueWorkerName, resourceName, noStartEndHyphen, numberOrPercentage } from '@/utils/validators'
Expand All @@ -176,18 +176,16 @@ const validationErrors = {
},
maxSurge: {
numberOrPercentage: 'Invalid value'
},
machine: {
architecture: {
required: 'Machine Architecture is required'
}
}
},
selectedZones: {
required: 'Zone is required'
},
volumeSizeInternal: {
minVolumeSize: 'Invalid volume size'
},
machineArchitectureInternal: {
required: 'Machine Architecture is required'
}
}

Expand Down Expand Up @@ -277,11 +275,6 @@ export default {
},
maxSurge: {
numberOrPercentage
},
machine: {
architecture: {
required
}
}
},
selectedZones: {
Expand All @@ -299,17 +292,33 @@ export default {
}
return minValue(this.minimumVolumeSize)(parseSize(value))
}
},
machineArchitectureInternal: {
required
}
}
},
machineTypes () {
return this.machineTypesByCloudProfileNameAndRegionAndZonesAndArchitecture({ cloudProfileName: this.cloudProfileName, region: this.region, zones: this.worker.zones, architecture: this.worker.machine.architecture })
return this.machineTypesByCloudProfileNameAndRegionAndZonesAndArchitecture({
cloudProfileName: this.cloudProfileName,
region: this.region,
zones: this.worker.zones,
architecture: this.worker.machine.architecture
})
},
machineArchitectures () {
return this.machineArchitecturesByCloudProfileNameAndRegionAndZones({ cloudProfileName: this.cloudProfileName, region: this.region, zones: this.worker.zones })
return this.machineArchitecturesByCloudProfileNameAndRegionAndZones({
cloudProfileName: this.cloudProfileName,
region: this.region,
zones: this.worker.zones
})
},
volumeTypes () {
return this.volumeTypesByCloudProfileNameAndRegionAndZones({ cloudProfileName: this.cloudProfileName, region: this.region, zones: this.worker.zones })
return this.volumeTypesByCloudProfileNameAndRegionAndZones({
cloudProfileName: this.cloudProfileName,
region: this.region,
zones: this.worker.zones
})
},
volumeInCloudProfile () {
return !isEmpty(this.volumeTypes)
Expand All @@ -327,9 +336,9 @@ export default {
return get(this.selectedMachineType, 'storage.type') !== 'fixed'
},
machineImages () {
return filter(this.machineImagesByCloudProfileName(this.cloudProfileName), ({ isExpired, architectures }) => {
return !isExpired && includes(architectures, this.worker.machine.architecture)
})
const machineImages = this.machineImagesByCloudProfileName(this.cloudProfileName)
const architecture = this.worker.machine.architecture
return filter(machineImages, ({ isExpired, architectures }) => !isExpired && includes(architectures, architecture))
},
minimumVolumeSize () {
const minimumVolumeSize = parseSize(this.minimumVolumeSizeByCloudProfileNameAndRegion({ cloudProfileName: this.cloudProfileName, region: this.region }))
Expand Down Expand Up @@ -425,6 +434,17 @@ export default {
},
machineImageCri () {
return get(this.selectedMachineImage, 'cri')
},
machineArchitectureInternal: {
get () {
return this.worker.machine.architecture
},
set (architecture) {
this.worker.machine.architecture = architecture

// Reset machine type and image to default as they won't be supported by new architecture
this.resetWorkerMachine()
}
}
},
methods: {
Expand Down Expand Up @@ -477,11 +497,6 @@ export default {
this.$v.selectedZones.$touch()
this.validateInput()
},
onInputMachineArchitecture () {
// Reset machine type and image to default as they won't be supported by new architecture
this.worker.machine.type = get(head(this.machineTypes), 'name')
this.worker.machine.image = head(this.machineImages)
},
onMachineTypeValid ({ valid }) {
if (this.machineTypeValid !== valid) {
this.machineTypeValid = valid
Expand Down Expand Up @@ -527,6 +542,11 @@ export default {
// storage can be defined, set volumeSizeInternal (=displayed size in size-input) to default storage size
this.volumeSizeInternal = storage.size
}
},
resetWorkerMachine () {
this.worker.machine.type = get(head(this.machineTypes), 'name')
const machineImage = head(this.machineImages)
this.worker.machine.image = pick(machineImage, ['name', 'version'])
}
},
mounted () {
Expand Down
29 changes: 19 additions & 10 deletions frontend/src/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,11 +461,15 @@ const getters = {
},
machineTypesByCloudProfileNameAndRegionAndZonesAndArchitecture (state, getters) {
return ({ cloudProfileName, region, zones, architecture }) => {
let machineTypes = getters.machineTypesOrVolumeTypesByCloudProfileNameAndRegionAndZones({ type: 'machineTypes', cloudProfileName, region, zones })
machineTypes = map(machineTypes, machineType => {
if (!machineType.architecture) {
machineType.architecture = 'amd64' // default if not maintained
}
let machineTypes = getters.machineTypesOrVolumeTypesByCloudProfileNameAndRegionAndZones({
type: 'machineTypes',
cloudProfileName,
region,
zones
})
machineTypes = map(machineTypes, item => {
const machineType = { ...item }
machineType.architecture ??= 'amd64' // default if not maintained
return machineType
})

Expand All @@ -474,8 +478,14 @@ const getters = {
},
machineArchitecturesByCloudProfileNameAndRegionAndZones (state, getters) {
return ({ cloudProfileName, region, zones }) => {
const machineTypes = getters.machineTypesOrVolumeTypesByCloudProfileNameAndRegionAndZones({ type: 'machineTypes', cloudProfileName, region, zones })
return uniq(map(machineTypes, 'architecture')).sort()
const machineTypes = getters.machineTypesOrVolumeTypesByCloudProfileNameAndRegionAndZones({
type: 'machineTypes',
cloudProfileName,
region,
zones
})
const architectures = uniq(map(machineTypes, 'architecture'))
return architectures.sort()
}
},
volumeTypesByCloudProfileName (state, getters) {
Expand Down Expand Up @@ -602,9 +612,8 @@ const getters = {
},
defaultMachineImageForCloudProfileNameAndMachineType (state, getters) {
return (cloudProfileName, machineType) => {
const machineImages = filter(getters.machineImagesByCloudProfileName(cloudProfileName), machineImage => {
return includes(machineImage.architectures, machineType.architecture)
})
const allMachineImages = getters.machineImagesByCloudProfileName(cloudProfileName)
const machineImages = filter(allMachineImages, ({ architectures }) => includes(architectures, machineType.architecture))
return firstItemMatchingVersionClassification(machineImages)
}
},
Expand Down

0 comments on commit 23a3e0b

Please sign in to comment.