HARVESTER: Fix HARVESTER v1.0.3 VM issues #6560
Conversation
n313893254
changed the title
|
@n313893254 This one too, is now targeted to merge in 2.7.0 |
| @@ -156,6 +150,14 @@ export default { | |||
| }); | |||
| } | |||
| }); | |||
|
|
|||
| try { | |||
| const { data: nodes } = await this.$store.dispatch('cluster/request', { url: `${ url }/${ NODE }s` }); | |||
There was a problem hiding this comment.
Why did this request need to move out of the allHashSettled above?
There was a problem hiding this comment.
I think we need to catch the errors of node's request since the response of node may have 404/500 error code.
There was a problem hiding this comment.
Sorry for I missed the link to the issue 2567 that the local cluster owner has no permission to access the node resource.
| const inStore = this.overrideStore || this.$store.getters['currentStore'](NAMESPACE); | ||
| let choices = this.$store.getters[`${ inStore }/all`](NAMESPACE); | ||
|
|
||
| if (this.overrideStore === HARVESTER) { |
There was a problem hiding this comment.
We are trying to reduce harvester-specific code in generic components as part of making a Harvester plugin. It is important to look for alternative solutions to this type of logic. See my comment in the harvester machine-config.
There was a problem hiding this comment.
I am going to remove the overrideStore prop, we can pass namespaces into podAffinity.
| } | ||
| }, | ||
|
|
||
| hasNamespaces: { |
There was a problem hiding this comment.
This also doesn't need to be a prop: compute from this.allNamespaces.length
| default: '' | ||
| }, | ||
|
|
||
| hasNodes: { |
There was a problem hiding this comment.
This doesn't need to be a prop: compute from this.nodes.length
Changing this to a computed property will change the UX in workload pod scheduling, but I think the behavior in harvester machine-config is preferable there, too. eg if a user making a workload can't see nodes, they could still type in a topology key.
shell/machine-config/harvester.vue
Outdated
| :nodes="allNodeObjects" | ||
| :has-nodes="allNodeObjects.length > 0" | ||
| :has-namespaces="isImportCluster" | ||
| override-store="harvester" |
There was a problem hiding this comment.
Since the harvester machine-config component already has a list of namespaceOptions, we can pass that through as a prop and override choices in allNamespaces. Then we keep harvester-specific code out of the PodAffinity component.
|
@mantis-toboggan-md Done, thanks for your review. |
Summary
Fixes VM issues
Occurred changes and/or fixed issues
Technical notes summary
cloneValueis a steve model and loss most of the spec property, need tocloneSpecto clone the real spec.$attrschanged every time when the resource is updated by the Websocket.dashboard/shell/components/form/UnitInput.vue
Line 215 in 5d4e32e
Areas or cases that should be tested
Areas which could experience regressions
Screenshot/Video