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

[BUG] [Mac] 2.17.0 release does not use admin-helper and vfkit binary from installer #3608

Closed
praveenkumar opened this issue Apr 20, 2023 · 9 comments · Fixed by #3609
Closed
Labels
kind/bug Something isn't working status/need triage

Comments

@praveenkumar
Copy link
Member

admin-helper and vfkit binary is downloading as part of setup instead using from the installer on Mac. Looks like we introduce some regression when we separated tray from the binary as part of package.

@praveenkumar praveenkumar added kind/bug Something isn't working status/need triage labels Apr 20, 2023
@cfergeau cfergeau changed the title [BUG] [Mac] admin-helper and vfkit binary is not installed as part of installer [BUG] [Mac] 2.17.0 release does not use admin-helper and vfkit binary from installer Apr 20, 2023
@cfergeau
Copy link
Contributor

cfergeau commented Apr 20, 2023

This is a regression introduced in 09de497

-packagedir: clean embed-download-darwin macos-universal-binary
+packagedir: clean embed-download-darwin $(BUILD_DIR)/macos-universal/crc

We no longer build a release binary for the macos installer, so the binary does not use the installed vfkit/crc-admin-helper.

@cfergeau
Copy link
Contributor

@anjannath

@cfergeau
Copy link
Contributor

I suspect telemetry will also be broken on macos for this release.

cfergeau added a commit to cfergeau/crc that referenced this issue Apr 20, 2023
This fixes crc-org#3608
Release binaries use installed vfkit/crc-admin-helper instead of
downloading them, and they use the correct telemetry key.
cfergeau added a commit to cfergeau/crc that referenced this issue Apr 20, 2023
This fixes crc-org#3608
Release binaries use installed vfkit/crc-admin-helper instead of
downloading them, and they use the correct telemetry key.
@anjannath
Copy link
Member

yes, telemetry is also broken :(

@anjannath anjannath pinned this issue Apr 20, 2023
@anjannath
Copy link
Member

This is a regression introduced in 09de497

-packagedir: clean embed-download-darwin macos-universal-binary
+packagedir: clean embed-download-darwin $(BUILD_DIR)/macos-universal/crc

We no longer build a release binary for the macos installer, so the binary does not use the installed vfkit/crc-admin-helper.

This change was unnecessary, i thought since macos-release-binary was a .PHONY target it'll always run even if the out/macos-universal/crc was already present so made packagedir explicitly depend on the file at out/macos-universal/crc :(

@cfergeau
Copy link
Contributor

Misc ways to detect this regression during release testing:

  • run rm -rf ~/.crc/bin before the tests
  • check if after crc setup, there are vfkit/crc-admin-helper* binaries in ~/.crc/bin
  • or check crc setup output for INFO Caching crc-admin-helper executable and INFO Setting up virtualization with vfkit

We could also add some banner when starting a non-release binary.

praveenkumar pushed a commit that referenced this issue Apr 21, 2023
This fixes #3608
Release binaries use installed vfkit/crc-admin-helper instead of
downloading them, and they use the correct telemetry key.
@anjannath
Copy link
Member

We can also add a check in the installer generation jobs, to make sure that the installerBuild=true is set on the CRC binary its using, something like:

$ strings out/windows-amd64/crc | grep -e "-ldflags" | grep installerBuild=true

@jsliacan
Copy link
Contributor

jsliacan commented Apr 23, 2023

run rm -rf ~/.crc/bin before the tests

We do remove the entire ~/.crc folder each time. See this - I think that's the right place (for linux)... After we do that though, we still need to introduce checks. Is that what you meant?

@cfergeau
Copy link
Contributor

Yes, this is what I meant, the rm is a required preliminary step. The logs I mentioned won't be shown if the binaries are already cached in ~/.crc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working status/need triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants