fix crc-support run.sh script extra popd

[lilyLuLiu]

No description provided.

[adrianriobo]

What is this extra popd doing in your case, I directly migrated this from qe-platform as it was there. Also just in case and to match all platforms this is still on the windows script

ci-definitions/crc-support/oci/lib/windows/run.ps1

Line 150 in f67810d

popd

[lilyLuLiu]

@adrianriobo In windows script, there is a push(https://gitlab.cee.redhat.com/crc/qe-platform/-/blob/master/support/images/crc-support/oci/lib/windows/run.ps1?ref_type=heads#L126) that match the popd.

[adrianriobo]

Ahh I see but here there is too..I mean I think the wrong thing here is that on unix/run.sh the pushd is insde the if for download...and it should be outside (so it applies to download or install or both) no?

https://github.com/crc-org/ci-definitions/blob/main/crc-support/oci/lib/unix/run.sh#L94

I mean move that outside instead of remove the popd should work and then all platforms will match WDYT?

[adrianriobo]

🤔 what I was suggesting is to move the mkdir target and pushd target before the if (as we do on windows)

With the last change I think if we need to download and install is gonna fail cause is gonna download inside target and then popd and finally try to install from the previous folder not from target

[lilyLuLiu]

@adrianriobo Hi, I am considering that remove the parameter targetPath.
As the targetPath folder(crc-support) is already copied by deliverset, and contains the the run.sh script. So we can use $0 to get the target path.

If we install crc from outside the target file, we need to pass the full path of the installer location. For example, -aName '/Users/$(cat username)/crc-support/linux-arm64.tar'. If we install inside the folder, it can be simplified to -aName 'linux-arm64.tar'

WDYT?

[adrianriobo]

Kind of, targetPath value is not always crc-support i.e. if you wanna download a bundle typically the targetPath for it would be ~/OpenshiftLocal/bundle/4.17.6 as we want to keep it there

[lilyLuLiu]

@adrianriobo I get that we may need to download bundles in some scenarios.
But the parameters aBaseURL & aName are the address for the download item, aName is the CRC installer name. The current logic does not support downloading the bundle.

[adrianriobo]

🤔 aName is the name of the asset it you use:

aBase=https://crcqe-asia.s3.ap-south-1.amazonaws.com/bundles/openshift/4.17.1-arm64
aName=crc_libvirt_4.17.1_arm64.crcbundle

It is working, actually I am testing it.

Also consider if we use it for download crc released version typically the targetPath for it would be something like ~/OpenshiftLocal/crc/2.43.0

And actually if you use you strategy to copy crc through the ASSETS_FOLDER from deliverest you can use targetPath to crc-support and download=false and install=true in that scenario the missing part as you commented is currently crc-support is expecting the xz https://github.com/crc-org/ci-definitions/blob/main/crc-support/oci/lib/linux/lib.sh#L32

You can add some logic there to parse the aName and check if it is a .xz or crc and if is the last direcly copy it to /usr/local/bin

[lilyLuLiu]

For the use of parameter aName , the functions install crc and download bundle can't be realized at the same time, right?

[adrianriobo]

yeah, may that was the thing confusing you, yeah in my pipeline i.e. I got the task download-bundle which is crc-support with some specific params and other task to donwload/ install crc with other set of parameters.

https://gitlab.cee.redhat.com/crc/qe-platform/-/merge_requests/253/diffs#887300116dc6088f7305e9afd71bd3ee2b6696d1_0_311

and

https://gitlab.cee.redhat.com/crc/qe-platform/-/merge_requests/253/diffs#887300116dc6088f7305e9afd71bd3ee2b6696d1_0_337

So yeah crc-support offer both modes but they can not be executed both at the same time

[lilyLuLiu]

Get it. Thanks for the detailed explanation. I have re-committed the code change.

[adrianriobo]

One last request, can you change the commit message to explain this fix matches the logic between platforms + includes the option to install crc binary directly on linux and add as prefix something like

[crc-support] fix: "your message"

This will help us to later cherry-pick only commits for crc-support when creating a new version.

Otherwise LGTM