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

CI Profiling precursor changes #14051

Merged
merged 15 commits into from
Sep 9, 2024
Merged

CI Profiling precursor changes #14051

merged 15 commits into from
Sep 9, 2024

Conversation

tomponline
Copy link
Member

@tomponline tomponline commented Sep 6, 2024 •
edited
Loading

This PR:

  1. Moves the building of the LXD binaries for the system tests into the code test stage and uses GH artifacts system in order to avoid building the LXD binaries multiple times for each concurrent runner.
  2. Adds functionality to build LXD binaries with code coverage support and uploads them to the GH artifact system for collection later (this will come in a future PR).
  3. Various Makefile improvements and cleanups.

@tomponline tomponline self-assigned this Sep 6, 2024
@tomponline tomponline force-pushed the tp-profiling branch 7 times, most recently from 2e936bf to 0fa25b0 Compare September 6, 2024 10:26
@tomponline tomponline changed the title WIP: Profiling WIP: CI profiling Sep 6, 2024
@tomponline tomponline force-pushed the tp-profiling branch 15 times, most recently from 74e7c57 to 7a7d5db Compare September 6, 2024 21:37
@tomponline
github: Move go version compatibility check into code test phase
d977e3b 
No need to run this repeatedly on every system test workflow.

Signed-off-by: Thomas Parrott <[email protected]>
@tomponline tomponline force-pushed the tp-profiling branch 3 times, most recently from af0cf2c to 94cbf89 Compare September 9, 2024 08:44
@tomponline
Makefile: Avoid unnecessary repeat builds during build
52b7bb2 
Build lxd project binaries only, and not all of the test and client binaries too.

Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
github: Cache binaries built in code tests and use in system tests
e414a26 
Rather than build it all again.

Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
github: Clearly indicate the LXD client binaries in test artifact naming
5a69984 
Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
github: Remove unused dqlite ppa usage in system tests
e81585c 
We use the same dqlite built during the code tests now for consistency.

Also avoids delays with ppa being built.

Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
github: Strip binaries during test
90cf7a6 
Like its done in snap.

Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
Makefile: lxc doesn't need sqlite3 tag
93c0c4d 
As doesn't link against sqlite3.

Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
Makefile: Remove unused dependencies from check stage
fb64bec 
Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
Makefile: Add -failfast to unit test run
f853a6d 
So errors are more visible.

Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
Makefile: Use -trimpath
0f51ee0 
Like its done in snap.

Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
Makefile: Add separate check-unit for unit tests
421d707 
Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
github: Use make check-unit for unit tests
a858141 
Signed-off-by: Thomas Parrott <[email protected]>
@tomponline
Makefile: Add GOCOVERDIR support
937ce67 
Signed-off-by: Thomas Parrott <[email protected]>
@tomponline tomponline force-pushed the tp-profiling branch 7 times, most recently from ff04671 to 0427f63 Compare September 9, 2024 15:21
@tomponline tomponline changed the title WIP: CI profiling CI Profiling Sep 9, 2024
@tomponline tomponline marked this pull request as ready for review September 9, 2024 15:31
@tomponline tomponline changed the title CI Profiling CI Profiling precursor changes Sep 9, 2024
simondeziel
simondeziel approved these changes Sep 9, 2024
View reviewed changes
Copy link
Member

@simondeziel simondeziel left a comment

Choose a reason for hiding this comment

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

LGTM thanks! There might be some things to further remove but those can wait till later.

One thing we lose with that is we are now only testing/building on Go 1.22. That said, maybe you wanted to have that done first and then we add another step to the Code job later on.

.github/workflows/tests.yml Show resolved Hide resolved
.github/workflows/tests.yml Show resolved Hide resolved
@tomponline tomponline merged commit 94110ee into canonical:main Sep 9, 2024
29 checks passed
@tomponline tomponline deleted the tp-profiling branch September 9, 2024 18:14
@tomponline
Copy link
Member Author

One thing we lose with that is we are now only testing/building on Go 1.22. That said, maybe you wanted to have that done first and then we add another step to the Code job later on.

Note, thanks for highlighting.

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

@simondeziel simondeziel simondeziel approved these changes

Assignees

@tomponline tomponline

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tomponline @simondeziel