Add template column for OpenShift VMs

[rszwajko]

Depends on #741

Display the name of the template used to create the VM.
The name is retrieved from label vm.kubevirt.io/template.

Reference-Url: https://kubevirt.io/user-guide/virtual_machines/templates/#relationship-between-templates-and-vms

[yaacov]

@rszwajko can you clean the commits ?

[yaacov]

some thoughts about this renderer:
a. we should not need to cast, this is an Openshift specific method, it should already get the correct type.
b. this is a long renderer, it dos not look right implemented inline, i see two options, one is implementing a method that return the template string, another is implementing a cell rendered and using i

[rszwajko]

we should not need to cast, this is an Openshift specific method, it should already get the correct type.

As already discussed, this is not happening because VMCellProps is not generic.

this is a long renderer, it dos not look right implemented inline, i see two options, one is implementing a method that return the template string, another is implementing a cell rendered and using i

The renderer is very simple, it's the the property path that is too long:) In the previous approach ("flat" objects) this was handled in the flattening method. In the current approach ("blob" objects) we need to dive deep to get the prop. Btw in the k8s world this seems a common pattern - adding a method for each nested prop would is not scalable.

[yaacov]

lets consider a getVmTemplate(vm: ProviderVirtualMachine) if will check the type, and return the currect template for each type.

the code will look like:

<TableCell>{getVmTemplate(data.?vm)}</TableCell>

i would prefer using a method to get the template over the current way

[rszwajko]

lets consider a getVmTemplate(vm: ProviderVirtualMachine) if will check the type, and return the currect

Done. Note that:

1. right now it works only for OpenShift
2. the same value will be retrieved in 2 different ways (jsonpath and accessor) - this impacts maintenance.
3. following this pattern (accessor method per prop) will result in many accessor methods. In the long term we need a structured way to handle them i.e. folder structure and an utility file per type. As for now I've created a vmProps.ts utility.

[yaacov]

a. we should not need to cast here
b. this is actually just a json path, why do you use a method ? what am i missing ?

[yaacov]

try:

jsonPath: ['object', 'metadata' ,'labels', 'vm.kubevirt.io/template'],

[rszwajko]

a. we should not need to cast here

The proposed solution with generics was dropped. If you have any other idea please share.

b. this is actually just a json path, why do you use a method ? what am i missing ?

To benefit from the type system and avoid i.e. typos.

[yaacov]

the idea of json path is to make the code clear and clean, if you add a method here, you add casting, you add a long .? code snippet, I prefer using jsonPath if possible.

[rszwajko]

the idea of json path is to make the code clear and clean

IMHO the opposite is true i.e. as stated above we lose type safety here.

if you add a method here, you add casting, you add a long .? code snippet

1. Casting is needed because we dropped generics in VMCellProps that were implemented in the VM list screens. Compare the code above with that version:
   https://github.com/rszwajko/forklift-console-plugin/blob/fb778cf9e4b2ec24be6c754122233cda013de594/packages/forklift-console-plugin/src/modules/Providers/views/details/tabs/VirtualMachines/VSphereVirtualMachinesRow.tsx#L39
2. Long path is needed because we dropped "flat" object approach. Using a string(jsonpath) is here similar to standard JS syntax.

I prefer using jsonPath if possible.

OK. Note that currently array of strings is not accepted - it's either a string or a method.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
2.6% Duplication

[yaacov]

#746 - add automation for kubevirt types

[yaacov]

nice !, didn't know you can do this 👍