Skip to content

CI: add macos-15-large#63

Merged
jandubois merged 1 commit into
lima-vm:masterfrom
AkihiroSuda:dev
Nov 25, 2024
Merged

CI: add macos-15-large#63
jandubois merged 1 commit into
lima-vm:masterfrom
AkihiroSuda:dev

Conversation

@AkihiroSuda
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda commented Nov 4, 2024

No description provided.

@AkihiroSuda AkihiroSuda added this to the v1.1.8 milestone Nov 4, 2024
@AkihiroSuda
Copy link
Copy Markdown
Member Author

AkihiroSuda commented Nov 4, 2024

https://github.com/lima-vm/socket_vmnet/actions/runs/11668029741/job/32487531768?pr=63

▶ Run limactl shell vm1 ip -4 -json addr show dev lima0 | jq -r .[0].addr_info[0].local | tee /tmp/vm1_iP
192.168.105.3
▶ Run iperf3 -c "$(cat /tmp/vm1_ip)"
  
iperf3: error - unable to connect to server - server may have stopped running or use a different port, firewall issue, etc.: No route to host

jandubois
jandubois previously approved these changes Nov 4, 2024
Copy link
Copy Markdown
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

Comment thread .github/workflows/test.yml Outdated
@AkihiroSuda
Copy link
Copy Markdown
Member Author

AkihiroSuda commented Nov 4, 2024

https://github.com/lima-vm/socket_vmnet/actions/runs/11670466432/job/32494724116?pr=63

+ netstat -rn
Routing tables

Internet:
Destination        Gateway            Flags               Netif Expire
default            192.168.64.1       UGScg                 en0       
default            link#11            UCSIg           bridge100      !
127                127.0.0.1          UCS                   lo0       
127.0.0.1          127.0.0.1          UH                    lo0       
169.254            link#5             UCS                   en0      !
192.168.64         link#5             UCS                   en0      !
192.168.64.1/32    link#5             UCS                   en0      !
192.168.64.1       16:9d:99:c7:98:64  UHLWIir               en0   1144
192.168.64.30/32   link#5             UCS                   en0      !
192.168.105        link#11            UC              bridge100      !
192.168.105.2      link#11            UHLWIi          bridge100      !
192.168.105.3      52.55.55.e7.4b.ea  UHLWIi          bridge100   1186
[...]

++ cat /tmp/vm1_ip
+ ping -c 3 192.168.105.3
PING 192.168.105.3 (192.168.105.3): 56 data bytes
64 bytes from 192.168.105.3: icmp_seq=0 ttl=64 time=1.498 ms
64 bytes from 192.168.105.3: icmp_seq=1 ttl=64 time=1.661 ms
64 bytes from 192.168.105.3: icmp_seq=2 ttl=64 time=3.294 ms

--- 192.168.105.3 ping statistics ---
3 packets transmitted, 3 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 1.498/2.151/3.294/0.811 ms
++ cat /tmp/vm1_ip
+ iperf3 -c 192.168.105.3
iperf3: error - unable to connect to server - server may have stopped running or use a different port, firewall issue, etc.: No route to host

The routing table seems just same as macos-13.

"No route to host" doesn't seem to occur with ping 🤔

The CI is passing for macos-14-large as well as macos-13-large, so something should have changed in macos-15-large.

@nirs
Copy link
Copy Markdown
Member

nirs commented Nov 7, 2024

I would split the changes to:

  • remove macos-12 to ubreak the build
  • add macos-14 since it works now
  • add macos-15 later, probably need more work

@AkihiroSuda AkihiroSuda removed this from the v1.1.8 milestone Nov 8, 2024
@AkihiroSuda AkihiroSuda force-pushed the dev branch 2 times, most recently from 4af81af to ac0ca93 Compare November 21, 2024 00:01
@AkihiroSuda AkihiroSuda marked this pull request as draft November 21, 2024 00:02
@AkihiroSuda AkihiroSuda force-pushed the dev branch 2 times, most recently from 0871f01 to ad667b1 Compare November 21, 2024 14:49
@AkihiroSuda AkihiroSuda changed the title CI: remove macos-12; add macos-15-large CI: add macos-15-large Nov 21, 2024
Comment thread .github/workflows/test.yml Outdated
@nirs
Copy link
Copy Markdown
Member

nirs commented Nov 22, 2024

Should work after #83 is merged.

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda AkihiroSuda marked this pull request as ready for review November 23, 2024 02:23
@AkihiroSuda AkihiroSuda added this to the v1.2.0 milestone Nov 23, 2024
@AkihiroSuda AkihiroSuda requested a review from nirs November 23, 2024 02:23
name: Integration tests
# Only a single version of macOS is used for running integration tests, due to the flakiness of the CI.
runs-on: macos-15-large # Intel (supports nested virtualization)
timeout-minutes: 40
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The build takes 9 minutes now (with #84) - so we can use much smaller timeout now, maybe 15 minutes?

#
# TODO: add macos-15-large https://github.com/lima-vm/socket_vmnet/pull/63
platform: [macos-13-large, macos-14-large]
platform: [macos-13, macos-14, macos-15]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This build only on aarch64, previously we built only on x86_64. We don't wan to test build on both?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Intel is covered in the integration test

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will be easier to maintain if we use:

platform:
- macos-13
- macos-14
- macos-15

runs-on: macos-15-large # Intel (supports nested virtualization)
timeout-minutes: 40
steps:
- uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This compiles both for x86_64 and aarch64, but we test only on x86_64 - any reason to build twice when we test the build in the build job?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For testing reproducibility on Intel

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, makes sense, this is also very quick.

run: make
integration:
name: Integration tests
# Only a single version of macOS is used for running integration tests, due to the flakiness of the CI.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good idea but better to add macos-15 and change the build in separate commits.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change is doing 2 logical changes - adding new plafrom for testing and removing older platform. We can do this separately but it works as is.

Copy link
Copy Markdown
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

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

Lets merge, we can always add and remove platforms.

@jandubois jandubois merged commit d929ac7 into lima-vm:master Nov 25, 2024
@nirs nirs mentioned this pull request Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants