Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #4279 Fix case sensitive WSL2 validation check #4280

Merged

Conversation

alegacy
Copy link
Contributor

@alegacy alegacy commented Jul 23, 2024

The WSL validation check assumes that the name "Microsoft" is capitalized but this is not true in all cases.

Fixes: Issue #4279

Relates to: Issue #2131 , PR #2323

Solution/Idea

Enhancing WSL validation check to be case-insensitive to prevent users from attempting to run on an unsupported platform.

Proposed changes

Updated validation check to force toLower(...) on the output of /proc/version prior to searching for the target substring of microsoft.

Testing

Validation check now prevents running on systems running WSL regardless of the capitalization of the word Microsoft.

alegacy@localhost:~/src/go/github.com/crc$ ./out/crc setup
INFO Using bundle path /home/alegacy/.crc/cache/crc_libvirt_0.0.0-unset_amd64.crcbundle 
INFO Checking if running as non-root              
INFO Checking if running inside WSL2              
CRC is unsupported using WSL2

alegacy@localhost:~/src/go/github.com/crc$ cat /proc/version
Linux version 5.15.153.1-microsoft-standard-WSL2 (root@941d701f84f1) (gcc (GCC) 11.2.0, GNU ld (GNU Binutils) 2.37) #1 SMP Fri Mar 29 23:14:13 UTC 2024
```shell

The WSL validation check assumes that the name "Microsoft" is
capitalized but this is not true in all cases.

Signed-off-by: Allain Legacy <[email protected]>
Copy link

openshift-ci bot commented Jul 23, 2024

Hi @alegacy. Thanks for your PR.

I'm waiting for a crc-org member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci openshift-ci bot requested review from gbraad and praveenkumar July 23, 2024 11:40
@cfergeau
Copy link
Contributor

/ok-to-test

Copy link

openshift-ci bot commented Jul 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfergeau, gbraad

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gbraad
Copy link
Contributor

gbraad commented Jul 23, 2024

Thanks. The test can be stricter, as it contains WSL2 too when MS provided the kernel.
Note: however, any custom kernel will not contain this, so the current test is not restrictive enough (but works for most cases).

@alegacy
Copy link
Contributor Author

alegacy commented Jul 23, 2024

@cfergeau @gbraad @praveenkumar ... some tests appear to be failing. I don't think any of them are affected by my changes. Do these tests have a history of intermittent failures? Should I re-trigger and hope for the best or is there some automation that will come around to do that?

@alegacy
Copy link
Contributor Author

alegacy commented Jul 24, 2024

/retest-required

Copy link

openshift-ci bot commented Jul 24, 2024

@alegacy: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/security 4107929 link false /test security
ci/prow/integration-crc 4107929 link true /test integration-crc

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@cfergeau
Copy link
Contributor

@cfergeau @gbraad @praveenkumar ... some tests appear to be failing. I don't think any of them are affected by my changes. Do these tests have a history of intermittent failures? Should I re-trigger and hope for the best or is there some automation that will come around to do that?

Don't worry about the tests, they are unfortunately very unreliable :(

@cfergeau cfergeau merged commit ec24d65 into crc-org:main Jul 24, 2024
21 of 29 checks passed
@alegacy alegacy deleted the bugfix/4279/fix-case-sensitive-validation-check branch July 24, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants