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.

Sign up for GitHub

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

Jump to bottom

hostagent: avoid requiring fuser and ss in the guest ; CI: update colima (v0.5.5→v0.6.5) #2009

Merged
merged 2 commits into from
Nov 24, 2023
Merged

hostagent: avoid requiring fuser and ss in the guest ; CI: update colima (v0.5.5→v0.6.5) #2009

merged 2 commits into from
Nov 24, 2023

Conversation

AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Nov 17, 2023 •
edited
Loading

Fix #2010

@AkihiroSuda AkihiroSuda added this to the v0.19.0 milestone Nov 17, 2023
@AkihiroSuda
Copy link
Member Author

https://github.com/lima-vm/lima/actions/runs/6908739427/job/18798647503?pr=2009

colima start
time="2023-11-17T20:26:58Z" level=fatal msg="lima compatibility error: invalid semver version for Lima: e5b99bc is not in dotted-tri format"
Warning: Attempt 1 failed. Reason: Child_process exited with error code 1
+ colima start
time="2023-11-17T20:27:09Z" level=fatal msg="lima compatibility error: invalid semver version for Lima: e5b99bc is not in dotted-tri format"
Warning: Attempt 2 failed. Reason: Child_process exited with error code 1
+ colima start
time="2023-11-[17](https://github.com/lima-vm/lima/actions/runs/6908739427/job/18798647503?pr=2009#step:10:18)T[20](https://github.com/lima-vm/lima/actions/runs/6908739427/job/18798647503?pr=2009#step:10:21):27:20Z" level=fatal msg="lima compatibility error: invalid semver version for Lima: e5b99bc is not in dotted-tri format"
Error: Final attempt failed. Child_process exited with error code 1

Need to wait for v0.6.3

@AkihiroSuda AkihiroSuda marked this pull request as draft November 17, 2023 20:29
@abiosoft
Copy link
Contributor

v0.6.3 has been tagged.

@abiosoft
Copy link
Contributor

According to that error message, it still wouldn't pass the compatibility check 🤔
The PR only allowed HEAD prefix mainly for brew install --head lima.

I guess the enforcement should be removed for now, or permit overriding via flag or env var.

@afbjorklund
Copy link
Member

Not sure where that error is coming from, Lima should use some pseudo-semver that is parseable (but wrong*).

$ _output/bin/limactl --version
limactl version 0.18.0-98-ga21b5f3

* it is "wrong" (in semver) because it is a pre-release of version 0.18.0, instead of an imaginary version 0.19.0

It is also perfectly sane, unlike the standard.

@AkihiroSuda AkihiroSuda changed the title CI: update colima (v0.5.5→v0.6.2) CI: update colima (v0.5.5→v0.6.5) Nov 24, 2023
@AkihiroSuda AkihiroSuda changed the title CI: update colima (v0.5.5→v0.6.5) hostagent: avoid requiring fuser and ss in the guest ; CI: update colima (v0.5.5→v0.6.5) Nov 24, 2023
@AkihiroSuda AkihiroSuda marked this pull request as ready for review November 24, 2023 01:38
@AkihiroSuda AkihiroSuda mentioned this pull request Nov 24, 2023
Closed
@AkihiroSuda AkihiroSuda marked this pull request as draft November 24, 2023 01:43
@AkihiroSuda
hostagent: avoid requiring fuser and ss in the guest
482678b 
No need to check the guestagent socket with `fuser` or `ss`,
if the guestagent is actually functional

Fix issue 2010

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda
CI: update colima (v0.5.5→v0.6.5)
d1e52c9 
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda AkihiroSuda marked this pull request as ready for review November 24, 2023 03:48
@AkihiroSuda AkihiroSuda merged commit f3b4dec into lima-vm:master Nov 24, 2023
24 checks passed
@jandubois jandubois mentioned this pull request Dec 22, 2023
Open
norio-nomura
norio-nomura reviewed Aug 6, 2024
View reviewed changes
.github/workflows/test.yml
fetch-depth: 1
# fetch-depth is set to 0 to let `limactl --version` print semver-ish version
fetch-depth: 0
ref: ${{ github.event.pull_request.head.sha }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AkihiroSuda
Is there a reason for using the PR head commit instead of the PR merge commit? The code being tested might differ from what it will be after the merge, which seems problematic.

The intention here is not clear from reading the PR.
I am currently trying on a PR to consolidate build jobs per platform (#2532), and I'm unsure if it's acceptable to default to using the merge commit. Could you clarify?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my memory limactl --version didn't show the correct version without this, but I could be wrong.
Feel free to remove this in that case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your response. Based on my testing, git describe returns the expected version string format with just fetch-depth: 0, so it doesn’t seem necessary to set ref:.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@norio-nomura norio-nomura norio-nomura left review comments

Assignees
No one assigned
Labels
area/test Tests and CI downstream/colima
Projects
None yet
Milestone
v0.19.0
Development

Successfully merging this pull request may close these issues.

Colima initiated instance fails to start with HEAD of Lima
4 participants
@AkihiroSuda @abiosoft @afbjorklund @norio-nomura