Page MenuHomeFreeBSD

M1: CI Scripts for Developers
ClosedPublic

Authored by bofh on Feb 7 2024, 8:10 PM.
Tags
None
Referenced Files
F102457284: D43786.id134450.diff
Tue, Nov 12, 1:34 PM
Unknown Object (File)
Mon, Nov 11, 9:01 AM
Unknown Object (File)
Fri, Nov 8, 4:37 AM
Unknown Object (File)
Fri, Nov 8, 3:23 AM
Unknown Object (File)
Thu, Nov 7, 7:17 PM
Unknown Object (File)
Wed, Nov 6, 9:34 AM
Unknown Object (File)
Wed, Nov 6, 8:23 AM
Unknown Object (File)
Tue, Nov 5, 2:58 PM
Subscribers
None

Details

Summary

This is a complete redo of https://reviews.freebsd.org/D38815 without touching release directory although utilizing the same script.

Current Features:

  • Does smoke tests using either bhyve(amd64 only) or qemu(Non x86_64 or when defined USE_QEMU=1). Currently defined CITYPE=smoke. Once we have added full tests we can also utilize something like CITYPE=complete
  • Most of the resources are dynamically allocated based on available resources in the host
  • If CPU supports POPCNT or vmm can be loaded then bhyve is used for amd64 otherwise automatically installs and uses qemu@nox11
  • When required third party applications or packages for booting non-x86 images are automatically installed

Current Limitation:

  • Does not support full tests like the one in our Jenkins
  • At this moment this is also not suitable to be used in our Jenkins platform as the jobs are divided in multiple smaller tasks and artifacts are moved here and there which are not exactly the scenario for individual developers.

Future Works:

  • Add full tests like the one in ci.f.o
  • Add different tests or options to disable some tests
  • Add test profiles full
  • Possibly add test through Cloud Providers like AWS/GCP/Azure or Cirrus or Github Actions
Test Plan
cd ~
git clone git@github.com:5u623l20/freebsd-src.git --branch=ffcci2
cd freebsd-src/tests/ci
make ci
make TARGET=amd64 TARGET_ARCH=amd64 ci
make TARGET=amd64 TARGET_ARCH=amd64 USE_QEMU=1 ci
make TARGET=arm64 TARGET_ARCH=aarch64 ci
make TARGET=powerpc TARGET_ARCH=powerpc64 ci
make TARGET=powerpc TARGET_ARCH=powerpc64le ci  
make TARGET=riscv TARGET_ARCH=riscv64 ci

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
tests/ci/Makefile
172

Truly speaking I haven't tried utilizing this from command line but have tested through __MAKE_CONF with the KERNCONF directive. But I think when defined through command line it will also work.

tests/ci/tools/ci.conf
21

Noted. I will remove these while making other changes and resubmitting again.

tests/ci/tools/freebsdci
57

Not at all. After this is merged I will add the full tests scenario and more functions here. The purpose of this function is simply just printing this message.

62

:P

bofh marked an inline comment as done.Feb 12 2024, 5:02 PM
bofh marked an inline comment as not done.
  • Remove comments
  • Reduce pkg installations for smoke tests

I wonder if using make(1) is really a good choice for this. Doing things like computing the RAM size of the VM in make feels awkward. There's also no good way to see what parameters are available to developers, and the framework is not friendly to user error (e.g., misspelling a variable name on the command line means that make will silently do the wrong thing). There is no namespacing for parameters (parameters for the build, VM boot, and the test itself are all mixed together).

make is definitely not the best tool but this is what was discussed by many when we initially planned this work.

I'm still reading other parts, but I think I can response this one first.

I think the goal is having several make targets for CI operations, and when things getting well, adding a target to src root, such as 'make ci' for let people can run everything with a single command. This doesn't mean that we have to write everything in make. Similar to what is done in ports/Mk and src/releases, we can introduce several helper scripts written in other languages. These script interpreters may not even need to be part of the base system (in such cases, the Makefile would check for dependencies and prompt for their installation if necessary). For now I think the current Makefile isn't too overly complex so I can accept it. However, I will definitely support extracting the complex logic out of Makefile when we are adding more feature.

I wonder if using make(1) is really a good choice for this. Doing things like computing the RAM size of the VM in make feels awkward. There's also no good way to see what parameters are available to developers, and the framework is not friendly to user error (e.g., misspelling a variable name on the command line means that make will silently do the wrong thing). There is no namespacing for parameters (parameters for the build, VM boot, and the test itself are all mixed together).

make is definitely not the best tool but this is what was discussed by many when we initially planned this work.

I'm still reading other parts, but I think I can response this one first.

I think the goal is having several make targets for CI operations, and when things getting well, adding a target to src root, such as 'make ci' for let people can run everything with a single command. This doesn't mean that we have to write everything in make. Similar to what is done in ports/Mk and src/releases, we can introduce several helper scripts written in other languages. These script interpreters may not even need to be part of the base system (in such cases, the Makefile would check for dependencies and prompt for their installation if necessary). For now I think the current Makefile isn't too overly complex so I can accept it. However, I will definitely support extracting the complex logic out of Makefile when we are adding more feature.

So far the way I have divided the Makefiles there won't be any too much complexity in the Makefile itself. The entire complexity will add up in the freebsdci rc script. However if you want me to mimic everything from the jenkins ci scripts that will be real trouble. My point of view is we should stay as close as possible on how the re@ generates the images so that the problems are reproducible by multiple teams. The only complex thing that I will need to add in the Makefile is a method of archiving the images or separating the environments. But I won't bother too much with that now. Will think about the bridge when we need to cross the river. :D

bofh edited the summary of this revision. (Show Details)

Double quote variable comparison

tests/ci/Makefile
2–30

After you submitted this work, we discussed the copyright/license text with @imp. He asked that we do not include the license because that's covered with the SPDX license identifier. I understand this is different from many of the files in the tree, but the hope is that this format will become the standard.

tests/ci/Makefile.aarch64
2–29

Update copyright/license text.

tests/ci/Makefile.amd64
2–29

Update copyright/license text.

tests/ci/Makefile.armv7
2–29

Update copyright/license text.

tests/ci/Makefile.powerpc64
2–29

Update copyright/license text.

tests/ci/Makefile.powerpc64le
2–29

Update copyright/license text.

tests/ci/Makefile.riscv64
2–29

Update copyright/license text.

tests/ci/tools/ci.conf
2–4

Add copyright/license text.

tests/ci/tools/freebsdci
2–4

Add copyright/license text.

Updating Licensing in light with the latest SPDX compatible licensing information.

bofh marked 9 inline comments as done.Apr 11 2024, 9:54 PM
bofh marked an inline comment as done.

Submit my current review drafts, I have to change computer this weekend and I believe I can finish (and accept) this weekend.

tests/ci/tools/freebsdci
7

The reorder block is better ordered as: PROVIDE/REQUIRE/BEFORE/KEYWORD

11

In the following we have: : ${freebsdci_enable:="NO"} so it looks disabled by default in this script, the result VM image is enabled freebsdci due to VM_RC_LIST in ci.conf

I think we can just remove It is enabled by default ... /etc/rc.conf part.

27

Let's add a:

desc="Run FreeBSD CI"
31

Is this used in any where?

36

Let's just remove MIPS stuff.

57

That's exactly what @markj means for placeholder. I suggest we can do that before we populating the concrete tests. Just some skeletons should be fine.

bofh marked 3 inline comments as done.Apr 12 2024, 8:08 AM
bofh marked an inline comment as done.
tests/ci/tools/freebsdci
31

Can you clarify which line? I believe you had this before I updated and made some changes which is why it's showing wrong line numbers. If you meant os_version I was testing on different branches and hence needed this. But as we will have the same files in different branches we can safely remove this.

Address the issues mentioned by @lwhsu

bofh marked 7 inline comments as done.Apr 12 2024, 9:02 AM
bofh added inline comments.
tests/ci/tools/freebsdci
57

Added another placeholder for the full_tests scenario.

bofh marked an inline comment as done.Apr 12 2024, 9:08 AM
tests/ci/Makefile
83

I suggest just set a fix size for VM_MEM, just using all or the half will effect the possibility to run multiple VMs on the same host in the same time.

89

indent, you can use something like . include

110

Same as above.

128

As we already have ASSUME_ALWAYS_YES=yes then we don't really need -y.

133

Same as above.

136

Same as above.

141

Same as above.

151

Will this respect TARGET and TARGET_ARCH?

If we want to run this unconditionally (with .PHONY), should we use -DWITHOUT_CLEAN? Or just like other building image does: assuming the object files are there.

154

Same as above buildworld part.

tests/ci/Makefile.aarch64
19

Same as above.

22

If this is for "be aware of this", then XXX isn't the best option as it sometimes means a potential issue or workaround. Use NOTE or just *** could be better.

tests/ci/Makefile.amd64
19

same as above.

tests/ci/Makefile.armv7
13

duplicaed line?

20

Same as above.

23

same as above.

tests/ci/Makefile.powerpc64
21

same as above.

tests/ci/Makefile.powerpc64le
21

same as above.

tests/ci/Makefile.riscv64
19

Same as above.

22

Same as above.

tests/ci/tools/ci.conf
15

sort this list and trim unneeded whitespace.

17

indent.

18

${VM_EXTRA_PACKAGES}

19

indent.

23

Align whitespace of <<EOF with others.

34

Same as above.

41

Same as above.

65

Same as above.

91

Same as above.

tests/ci/tools/freebsdci
13

I suggest:

# REQUIRE: LOGIN FILESYSTEMS

and remove

# BEFORE: LOGIN

to ensure all the possibly required bits are started.

30

Let's set the default freebsdci_type as full here.

31

yes I mean os_version.

tests/ci/Makefile
151

Yes this will. Check the Test Plan. I have tested in that way too.

bofh marked 2 inline comments as done.Apr 16 2024, 1:17 PM
bofh added inline comments.
tests/ci/tools/freebsdci
13

I think then we should have it like:

# REQUIRE: LOGIN FILESYSTEMS NETWORKING

There is a possibility TCTP tests started before these are finalized.

30

I have not added other bits of pieces for the full tests so this might fail. I think for the sake of the example we might set it to `freebsdci_type="smoke"` for now until we start M2.

bofh marked 21 inline comments as done.Apr 16 2024, 1:22 PM

Some more cosmetic fixes mentioned by @lwhsu

bofh marked 2 inline comments as done.Apr 16 2024, 1:44 PM
tests/ci/Makefile
151

-DWITHOUT_CLEAN is actually handled by METAMODE here.

tests/ci/Makefile
151

I have just tested with something like the following which works perfectly:

make TARGET=arm64 TARGET_ARCH=aarch64 ci-buildworld
make TARGET=amd64 TARGET_ARCH=amd64 ci-buildworld

So we are good.

  • Fix an indention
  • Fix rcscript orders
bofh marked 2 inline comments as done.Apr 17 2024, 4:48 PM

I think this is good to land now and we can do further improvements in the later phases.

This revision is now accepted and ready to land.Apr 18 2024, 5:14 PM