crc-org / crc-extension

Red Hat OpenShift Local Extension for integration of OpenShift Local clusters with Podman Desktop
Apache License 2.0
17 stars 19 forks source link

fix: allow to customize config values on create new page #228

Closed lstocchi closed 2 months ago

lstocchi commented 3 months ago

This PR updates the extension to show the properties that can be customized when starting the VM in the create new page (memory, cpus, disksize and pull-secret-file).

They have been removed from the settings page to be in line with the other podman desktop extensions.

So, when clicking on create, it updates the config values of cpus/memory/disksize/pull-secret and if the start now option is true (by default) the VM is also started. Otherwise it just updates the config and register the provider to start/stop/delete the VM into Desktop.

Right now there is no way to customize/update dynamically the minimum value of a config so i had to create 2 entries for both the memory and cpus (one for openshift and one for minishift). They are displayed accordingly to the preset value.

image

This requires https://github.com/containers/podman-desktop/pull/7659

lstocchi commented 3 months ago

This should be ready for a review. You need to use the main branch of desktop as this PR requires https://github.com/containers/podman-desktop/pull/7659 to work.

There is just one missing part which is we should hide the create new ... button when the kubernetes connection is registered but it's not easy as i thought so i'm going to tackle it in a different PR.

slemeur commented 2 months ago

FYI, the font for the pullsecret is not displayed (testing with https://github.com/containers/podman-desktop/pull/7995)

Screenshot 2024-07-10 at 10 58 23
lstocchi commented 2 months ago

FYI, the font for the pullsecret is not displayed (testing with containers/podman-desktop#7995)

Nice catch 👍 It is a problem on podman desktop tough, i'm gonna fix it there

lstocchi commented 2 months ago

Remarks:

  • memory and disk are described in GiB but displayed in GB: that's incoherent

updated

  • can't we detect the pull secret has already been set and hide the parameter

if it's set, the input field is filled. I think it is more correct, if you notice that you're using a wrong file you can still update it. image

  • why can't we set the preset ?

The preset is currently chosen during the setup as it downloads the correct bundle. Not sure if setting the preset during the create new ... phase is more user friendly bc when you click on start it would download everything requiring minutes.

Also I was in the state where CRC was installed but not setup (missing the bundle) and it is very hard from the resource page to leave that state but that may be not related to this PR

I'll look into this in following PR

jeffmaury commented 2 months ago

2 remarks:

lstocchi commented 2 months ago

2 remarks:

  • some values for memory (4,5, 4.6 leads to errors but we cannot read the error message). Strange to understand but some values 4,5 and 4,6 or 5 and 5.1 leads to the same setting

When did you face the error with the memory value? When setting the slider or when hitting create?

jeffmaury commented 2 months ago

2 remarks:

  • some values for memory (4,5, 4.6 leads to errors but we cannot read the error message). Strange to understand but some values 4,5 and 4,6 or 5 and 5.1 leads to the same setting

When did you face the error with the memory value? When setting the slider or when hitting create?

No create as the value set for memory is rejected by HyperV

lstocchi commented 2 months ago
  • some values for memory (4,5, 4.6 leads to errors but we cannot read the error message). Strange to understand but some values 4,5 and 4,6 or 5 and 5.1 leads to the same setting

I was not able to replicate it. Tried to set the memory to 4.5 -5 and the vm was started without any problem. I had an issue that occurred repeatedly when the VM was started and the cli was not able to reach it (ssh: handshake failed). Eventually i clean up everything and re-done the setup + start and worked fine. Maybe you had some similar thing? Can you try by deleting + cleanup + setup + start again?

  • when the machine is created, an emty screen is displayed:

After some investigation it seems that the problem is not that easy to solve. Basically the create new button that you see in the resources page, it is displayed if the KubernetesProviderConnectionFactory is initialized. And because it does not make any sense to have it once one VM is created, the factory was disposed to hide that button. As a quick solution I just removed the dispose action. I think we should update podman desktop to provide an enablement clause to allow extensions to decide when the create new button should be visible or not. WDYT?

jeffmaury commented 2 months ago
  • some values for memory (4,5, 4.6 leads to errors but we cannot read the error message). Strange to understand but some values 4,5 and 4,6 or 5 and 5.1 leads to the same setting

I was not able to replicate it. Tried to set the memory to 4.5 -5 and the vm was started without any problem. I had an issue that occurred repeatedly when the VM was started and the cli was not able to reach it (ssh: handshake failed). Eventually i clean up everything and re-done the setup + start and worked fine. Maybe you had some similar thing? Can you try by deleting + cleanup + setup + start again?

  • when the machine is created, an emty screen is displayed:

After some investigation it seems that the problem is not that easy to solve. Basically the create new button that you see in the resources page, it is displayed if the KubernetesProviderConnectionFactory is initialized. And because it does not make any sense to have it once one VM is created, the factory was disposed to hide that button. As a quick solution I just removed the dispose action. I think we should update podman desktop to provide an enablement clause to allow extensions to decide when the create new button should be visible or not. WDYT?

I don't understand fully because when I create a Kind cluster (I suppose this is also KubernetesProviderConnectionFactory), I have this screen:

image

So what is the difference ?

lstocchi commented 2 months ago

I don't understand fully because when I create a Kind cluster (I suppose this is also KubernetesProviderConnectionFactory), I have this screen:

image

So what is the difference ?

With the latest commit (remove of the disposable) you should see the same when the creation + start process of crc is completed. The difference should be about this button image

For kind it makes sense to have it always there because you can create X cluster. For crc, you can just create one VM, so it would be great if the create button is hidden when a VM is there.

By disposing the factory we were able to achieve this behavior. But if we dispose the factory, we would see the empty screen as you noted above when the start process is completed

jeffmaury commented 2 months ago

I don't understand fully because when I create a Kind cluster (I suppose this is also KubernetesProviderConnectionFactory), I have this screen: image So what is the difference ?

With the latest commit (remove of the disposable) you should see the same when the creation + start process of crc is completed. The difference should be about this button image

For kind it makes sense to have it always there because you can create X cluster. For crc, you can just create one VM, so it would be great if the create button is hidden when a VM is there.

By disposing the factory we were able to achieve this behavior. But if we dispose the factory, we would see the empty screen as you noted above when the start process is completed

I think we are not in sync: I'm talking about the screen that is displayed when the VM has been created: for kind I have to button to return to the Resources page. For CRC I have

I don't understand fully because when I create a Kind cluster (I suppose this is also KubernetesProviderConnectionFactory), I have this screen: image So what is the difference ?

With the latest commit (remove of the disposable) you should see the same when the creation + start process of crc is completed. The difference should be about this button image

For kind it makes sense to have it always there because you can create X cluster. For crc, you can just create one VM, so it would be great if the create button is hidden when a VM is there.

By disposing the factory we were able to achieve this behavior. But if we dispose the factory, we would see the empty screen as you noted above when the start process is completed

I think we are not in sync: for kind I have a screen with a button that returns to the Resources page. For CRC I have an empty screen

lstocchi commented 2 months ago

I think we are not in sync: for kind I have a screen with a button that returns to the Resources page. For CRC I have an empty screen

Have you tried with the latest commit?

jeffmaury commented 2 months ago

I think we are not in sync: for kind I have a screen with a button that returns to the Resources page. For CRC I have an empty screen

Have you tried with the latest commit?

Not yet :)

lstocchi commented 2 months ago

Not yet :)

Ah ok 👍 This is what you should see (i cut the gif bc it was 3 mins long lol) crc_start

jeffmaury commented 2 months ago

Seems it's working better than expected:

crc

So if I set 4.5GB I have these settings:

- consent-telemetry                     : no
- disk-size                             : 32
- memory                                : 4291
- preset                                : microshift

And starting crc --log-level debug give me this:```

DEBU CRC version: 2.37.1+36d451 DEBU OpenShift version: 4.15.14 DEBU Running 'crc start' DEBU Total memory of system is 34105286656 bytes WARN A new version (2.39.0) has been published on https://developers.redhat.com/content-gateway/file/pub/openshift-v4/clients/crc/2.39.0/crc-windows-installer.zip DEBU Checking file: C:\Users\Jeff.crc\machines\crc.crc-exist INFO Using bundle path C:\Users\Jeff.crc\cache\crc_microshift_hyperv_4.15.14_amd64.crcbundle INFO Checking minimum RAM requirements DEBU Total memory of system is 34105286656 bytes INFO Check if Podman binary exists in: C:\Users\Jeff.crc\bin\oc INFO Checking if running in a shell with administrator rights DEBU Running '$currentPrincipal = New-Object Security.Principal.WindowsPrincipal([Security.Principal.WindowsIdentity]::GetCurrent());$currentPrincipal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)' INFO Checking Windows release DEBU Running '(Get-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion" -Name ReleaseId).ReleaseId' INFO Checking Windows edition DEBU Running '(Get-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion").EditionID' DEBU Running on Windows Professional edition INFO Checking if Hyper-V is installed and operational DEBU Running '@(Get-Wmiobject Win32_ComputerSystem).HypervisorPresent' DEBU Running '@(Get-Service vmms).Status' INFO Checking if Hyper-V service is enabled DEBU Running '@(Get-Service vmms).Status' INFO Checking if crc-users group exists DEBU Running 'Get-LocalGroup -Name crc-users' INFO Checking if current user is in crc-users and Hyper-V admins group DEBU Running '(Get-LocalGroupMember -Group 'crc-users').Name' DEBU Checking current user is in the 'crc-user' group DEBU group members: DESKTOP-JEFF\Jeff DEBU Running '(Get-LocalGroupMember -SID 'S-1-5-32-578').Name' DEBU Checking current user is in the 'Hyper-v Administrators' group DEBU group members: DESKTOP-JEFF\Jeff INFO Checking if vsock is correctly configured DEBU Running 'Get-Item -Path "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Virtualization\GuestCommunicationServices\00000400-FACB-11E6-BD58-64006A7986D3"' INFO Checking if the win32 background launcher is installed DEBU Running '(Get-Item 'C:\Program Files\Red Hat OpenShift Local\crc-background-launcher.exe').VersionInfo.FileVersion'

DEBU Found crc-background-launcher.exe version 0.0.0.1 INFO Checking if the daemon task is installed DEBU Running 'Get-ScheduledTask -TaskName crcDaemon' DEBU Running '(Get-ScheduledTask -TaskName "crcDaemon").Version' INFO Checking if the daemon task is running DEBU Running '(Get-ScheduledTask -TaskName "crcDaemon").State' INFO Checking admin helper service is running DEBU Running '(Get-Service crcAdminHelper).Status' INFO Checking SSH port availability DEBU Checking file: C:\Users\Jeff.crc\machines\crc.crc-exist DEBU Copying 'C:\Users\Jeff.crc\cache\crc_microshift_hyperv_4.15.14_amd64\oc.exe' to 'C:\Users\Jeff.crc\bin\oc\oc.exe'

DEBU Copying 'C:\Users\Jeff.crc\cache\crc_microshift_hyperv_4.15.14_amd64\podman.exe' to 'C:\Users\Jeff.crc\bin\podman\podman.exe' INFO Loading bundle: crc_microshift_hyperv_4.15.14_amd64... DEBU Cannot load secret from configuration: empty path DEBU Using secret from keyring INFO Creating CRC VM for MicroShift 4.15.14... DEBU Running pre-create checks... DEBU Running '@(Get-Module -ListAvailable hyper-v).Name | Get-Unique' DEBU Running '@([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole(([System.Security.Principal.SecurityIdentifier]::new("S-1-5-32-578")))' DEBU Creating machine... DEBU Creating VM... DEBU Running 'Hyper-V\New-VM crc -Path 'C:\Users\Jeff.crc\machines\crc' -MemoryStartupBytes 4291MB' DEBU Command failed: exit status 1 DEBU stdout: DEBU stderr: Hyper-V\New-VM : �chec de la modification du p�riph�rique 'Memory'. Valeur de m�moire non valide attribu�e � ��crc��. Les valeurs de m�moire doivent �tre correctement align�es. 'crc' n'a pas r�ussi � modifier le p�riph�rique 'Memory'. (ID d'ordinateur virtuel BBB1A6F8-4048-4461-B5B4-FBBE8D6E6DC4) Valeur de m�moire non valide attribu�e � �crc�. La valeur de m�moire attribu�e (�4291�Mo) n'est pas correctement align�e. R�essayez avec une valeur de m�moire correctement align�e. (ID d'ordinateur virtuel BBB1A6F8-4048-4461-B5B4-FBBE8D6E6DC4) Au caract�re Ligne:1 : 43

Error creating machine: Error in driver during machine creation: exit status 1

lstocchi commented 2 months ago

I still have an issue with VM start failure not reported but it is not related to this PR so I will open a follow up issue

Yeah, I'll investigate it as well. I'm worried that when using the minimum amount of memory allowed, the conversion is not optimal and the resources given to micro shift are not enough. I'll give this a look when I have some free time