Start Windows CRC executable path with upper case drive-letter

[odockal]

Due to a strict check of a hidden_daemon.ps1 file content during crc setup on crc side there is an error when initializing the CRC in podman desktop. This PR should align the default windows crc executable location with this strict check.

Fixes #175

[gbraad]

Does this affect functionality?

[cfergeau]

I think I saw some mentions of lower case/upper case drive letters causing issues in relation with https://github.com/crc-org/crc/issues/3837 and similar issues we had recently. No idea if this PR is related to these ^^

[odockal]

It should fix #175.

[odockal]

right now it seems to work with Podman Desktop 1.4.0 and CRC 2.26.0. (Just testing locally)

[gbraad]

Does https://github.com/crc-org/crc/blob/45831b0e1410205465e33f477dd1fc4c5c724582/pkg/crc/preflight/preflight_daemon_task_check_windows.go#L220 also need to be fixed, @anjannath ?

[gbraad]

Please change format, as we do not make use of fix and chore. I would propose: Start Windows CRC executable path with upper case drive-letter

Make sure to provide a description as e letter is merely a continuation and does not help much to understand the issue you try to resolve.

[odockal]

@gbraad Fixed. I also pushed better commit message.

[anjannath]

Does https://github.com/crc-org/crc/blob/45831b0e1410205465e33f477dd1fc4c5c724582/pkg/crc/preflight/preflight_daemon_task_check_windows.go#L220 also need to be fixed, @anjannath ?

not the crcos.FileContentMatches function, but i was thinking to add in here: https://github.com/crc-org/crc/blob/45831b0e1410205465e33f477dd1fc4c5c724582/pkg/crc/preflight/preflight_daemon_task_check_windows.go#L209 a step to always make the drive letter uppercase.

[gbraad]

@odockal

there is no description to explain what is being solved here: https://github.com/crc-org/crc-extension/pull/178/commits/98f96507c7db20b40b558e5a1f307f8ff012aa89

The issue looks so trivial that it might not be understood why the case matters.

[odockal]

@odockal

there is no description to explain what is being solved here: 98f9650

The issue looks so trivial that it might not be understood why the case matters.

Added fixes #175 into commit message, so it is now clear, right?

[gbraad]

We never refer to just a github issue, as that means a developer would ALSO have to open a browser to understand what the change is about. On the command line that does not work well, like during a bisect.

[gbraad]

Simple example when it concerns a fixed bug: https://github.com/crc-org/crc/commit/45831b0e1410205465e33f477dd1fc4c5c724582 or more detailed: https://github.com/crc-org/crc/commit/f76e4b9eac99b79ffc8b29b0e932b8b3b9b7a6f2

We haven't been doing this in the repo when it concerns features, but since this is an obscure issue some details are appreciated.

[odockal]

Simple example when it concerns a fixed bug: crc-org/crc@45831b0 or more detailed: crc-org/crc@f76e4b9

We haven't been doing this in the repo when it concerns features, but since this is an obscure issue some details are appreciated.

Ok, getting closer now?

[gbraad]

Due to a strict check of a hidden_daemon.ps1 file content during crc setup

So, wouldn't it be smarter to prevent the strict check in hidden_daemon.ps1 instead? The casing should not have to matter. @anjannath WDYT?

[anjannath]

Due to a strict check of a hidden_daemon.ps1 file content during crc setup

So, wouldn't it be smarter to prevent the strict check in hidden_daemon.ps1 instead? The casing should not have to matter. @anjannath WDYT?

I don't think the strict check should be relaxed, (mostly because its using a generic FileContentMatches function which does byte to byte comparison that is used for checking other files that are created in pre-flight checks)

I tested this and it fixes the issue #175, it seems golang os.Executable output is affected by the PATH env variable, and since we add this file path to the beginning of the existing PATH we are getting the small letter drive name

[odockal]

Due to a strict check of a hidden_daemon.ps1 file content during crc setup

So, wouldn't it be smarter to prevent the strict check in hidden_daemon.ps1 instead? The casing should not have to matter. @anjannath WDYT?

Even though I strongly agree with this. We would have to wait until next CRC version release to get this fixed. Right now any CRC extension cannot be used on Windows at all with crc 2.26.0, if I am not mistaken, so it would be nice to have this fixed by latest crc extension (if released with this patch) and have all working together - podman desktop, extension and crc.

[anjannath]

I have started working on a fix on CRC's side, https://github.com/anjannath/crc/commit/fda052d9bc475a1c3681ab3385e2b727d8aad7d5

[anjannath]

we had a discussion about this on slack, and we decided since, this is going to fix the issue for current release of CRC as well as newer releases of CRC, to merge this PR and discuss further improvements to the preflight check on CRC side (to make it independent of whether the path starts with a lower case or upper case drive letter)

[odockal]

@anjannath Absolutely! thank you.

[anjannath]

@odockal welcome, fyi, we are not yet able to publish the extension image with this change to the official redhat-developer/openshift-local-extension due to some permission issue, hope to get it published there by EOD, thanks!

[odockal]

@anjannath no problem. Let me know once it is out so I can test it.

[odockal]

@anjannath If I tried to install latest from ghcr.io it seems to work fine with the patch. I can initialize microshift preset atm.