ENT-12414: Use `librsync`'s Stream API in `GET <FILENAME>` requests #5629

larsewi · 2024-11-19T14:03:44Z

Added file stream API
New network protocol version v4 - filestream

TODO:

Depends on ENT-12414: Added librsync to deps-packaging buildscripts#1520
Depends on Fixed compiler warning -Werror=enum-int-mismatch NorthernTechHQ/libntech#230
Check that errors in different places in the code does not make client/server hang
Add librsync as dependency in CI
Should be merged before hand https://gitlab.com/Northern.tech/CFEngine/jenkins-vms/-/merge_requests/59

cf-serverd/server_common.c

libcfnet/client_code.c

libcfnet/file_stream.c

vpodzime

Looks good to me otherwise. Very nice! Thanks for taking the hard road! 👏 🍻 I'm very excited about this!

libcfnet/file_stream.c

vpodzime

Let's see if it works! 😁 🤞 🍻

craigcomstock

Some comments and little typos.

configure.ac

libcfnet/README.md

libcfnet/file_stream.c

libcfnet/file_stream.h

vpodzime

ACK

larsewi · 2024-12-09T16:21:54Z

Just tested this manually on Windows and it works like a charm.

PS C:\Program Files\Cfengine\bin> .\cf-agent.exe -KI
    info: Copied file '/var/cfengine/masterfiles/promises.cf' from '172.31.39.27' to 'C:\promises.cf.cfnew'
    info: Moved 'C:\promises.cf.cfnew' to 'C:\promises.cf'
    info: Updated file 'C:\promises.cf' from '172.31.39.27:/var/cfengine/masterfiles/promises.cf'

Did however find a separate (surprisingly unrelated) issue in cf-net.exe (see ENT-12511)

.github/workflows/acceptance_tests.yml

libcfnet/Makefile.am

libcfnet/file_stream.c

This commit was added while working on implementing the File Stream API in ticket ENT-12414. The File Stream API utilizes librsync. To begin with I had `AC_CHECK_HEADERS([librsync_export.h], [], AC_MSG_ERROR(Cannot find librsync))` in configure.ac. This file did not exist in earlier versions of librync. Hence, I upgraded the Ubuntu platforms in GitHub CI to make configure work. Later a realized that I should remove that line enirely to make CFEngine compatible with earlier versions. However, it does not hurt to upgrade the Ubuntu platforms, so decided on keeping this commit. Ticket: None Changelog: None Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>

After upgrading GitHub CI platorms to Ubuntu 24, the ASAN Unit Tests started complaining about One Definition Rule (ODR) violations. Hence, I disabled ODR and created a follow-up ticket (see CFE-4454) to fix the violations and enable ODR again. Ticket: None Changelog: None Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>

Ticket: ENT-12414 Changelog: None Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>

Added API for streaming file contents over the network using the RSYNC algorithm from librsync. Ticket: ENT-12414 Changelog: None Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech> Co-authored-by: Craig Comstock <craig.comstock@northern.tech>

Ticket: ENT-12414 Changelog: Title Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech> Co-authored-by: Craig Comstock <craig.comstock@northern.tech>

The implementation is based on the previous protocol for `GET <FILENAME>` requests that used sparse files. Ticket: ENT-12414 Changelog: Title Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>

Apparently there are functions called `SendMessage()` and `GetFileSize()` in the Win32 API. I fixed these name conflicts by adding a `Protocol` prefix to all functions related to the simple network protocol for file stream communication. Furthermore, I changed the name of `GetFileSize()` to `GetSizeOfFile()`. Ticket: ENT-12414 Changelog: None Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>

Signed-off-by: Ole Herman Schumacher Elgesem <ole@northern.tech> (cherry picked from commit 1486c24) Conflicts: .github/workflows/macos_unit_tests.yml Use of librsync (cfengine#5629) was not cherry picked to 3.24.x

Check for Homebrew if we're on Darwin and use brew's --prefix. Ticket: CFE-4385 (cherry picked from commit 7e88d4b) Conflicts: .github/workflows/macos_unit_tests.yml configure.ac using librsync (cfengine#5629) was not cherry-picked to 3.24.x so adjustments were needed.

Signed-off-by: Ole Herman Schumacher Elgesem <ole@northern.tech> (cherry picked from commit 1486c24) Conflicts: .github/workflows/macos_unit_tests.yml Use of librsync (cfengine#5629) was not cherry picked to 3.24.x (cherry picked from commit d91eebd) Conflicts: .github/workflows/macos_unit_tests.yml Had to adjust as 3.21 uses pcre not pcre2 as the commit from newer branch had.

Check for Homebrew if we're on Darwin and use brew's --prefix. Ticket: CFE-4385 (cherry picked from commit 7e88d4b) Conflicts: .github/workflows/macos_unit_tests.yml configure.ac using librsync (cfengine#5629) was not cherry-picked to 3.24.x so adjustments were needed. (cherry picked from commit faf08d9) Conflicts: .github/workflows/macos_unit_tests.yml configure.ac Needed some finesse as rsync and pcre2 were in commit and not used in 3.21.

larsewi force-pushed the librsync branch 7 times, most recently from c29dfd8 to 937589a Compare November 20, 2024 12:43

github-advanced-security bot found potential problems Nov 20, 2024

View reviewed changes

larsewi force-pushed the librsync branch from 937589a to 9e3dcf0 Compare November 20, 2024 15:59

larsewi mentioned this pull request Nov 21, 2024

ENT-12414: New network protocol v4 - SAFEGET #5621

Closed

larsewi force-pushed the librsync branch 8 times, most recently from 37a8af2 to 9cb3c14 Compare November 28, 2024 13:53

larsewi marked this pull request as ready for review November 28, 2024 13:54

larsewi requested review from craigcomstock, olehermanse and vpodzime November 28, 2024 13:54

larsewi force-pushed the librsync branch 3 times, most recently from fafd971 to af5bd02 Compare November 29, 2024 10:51

vpodzime reviewed Nov 29, 2024

View reviewed changes

larsewi force-pushed the librsync branch 3 times, most recently from b627503 to 2e808ed Compare December 2, 2024 09:10

vpodzime previously approved these changes Dec 2, 2024

View reviewed changes

larsewi force-pushed the librsync branch from 0f985d8 to 04211a3 Compare December 2, 2024 15:22

craigcomstock reviewed Dec 2, 2024

View reviewed changes

larsewi force-pushed the librsync branch 5 times, most recently from f728290 to 8ad0742 Compare December 5, 2024 14:32

vpodzime previously approved these changes Dec 6, 2024

View reviewed changes

craigcomstock previously approved these changes Dec 6, 2024

View reviewed changes

larsewi force-pushed the librsync branch from 8ad0742 to af5cbd9 Compare December 10, 2024 12:20

olehermanse reviewed Dec 11, 2024

View reviewed changes

.github/workflows/acceptance_tests.yml Show resolved Hide resolved

libcfnet/Makefile.am Outdated Show resolved Hide resolved

libcfnet/file_stream.c Show resolved Hide resolved

larsewi dismissed stale reviews from vpodzime and craigcomstock via 380dc3d December 11, 2024 12:18

larsewi force-pushed the librsync branch from af5cbd9 to 380dc3d Compare December 11, 2024 12:18

larsewi and others added 7 commits December 11, 2024 13:18

Add librsync as dependency to GitHub CI

5a617e9

Ticket: ENT-12414 Changelog: None Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>

Added file stream API

a7b50e8

Added API for streaming file contents over the network using the RSYNC algorithm from librsync. Ticket: ENT-12414 Changelog: None Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech> Co-authored-by: Craig Comstock <craig.comstock@northern.tech>

New network protocol version v4 - filestream

4bbaeeb

Ticket: ENT-12414 Changelog: Title Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech> Co-authored-by: Craig Comstock <craig.comstock@northern.tech>

File Stream API now writes sparse files

c82e7fc

The implementation is based on the previous protocol for `GET <FILENAME>` requests that used sparse files. Ticket: ENT-12414 Changelog: Title Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>

larsewi force-pushed the librsync branch from 380dc3d to 898fcc5 Compare December 11, 2024 12:21

craigcomstock approved these changes Dec 11, 2024

View reviewed changes

larsewi merged commit 0b53a32 into cfengine:master Dec 13, 2024
42 of 48 checks passed

larsewi deleted the librsync branch December 18, 2024 08:08

ENT-12414: Use librsync's Stream API in GET <FILENAME> requests #5629

ENT-12414: Use librsync's Stream API in GET <FILENAME> requests #5629

Uh oh!

Conversation

larsewi commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vpodzime left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vpodzime left a comment

Choose a reason for hiding this comment

Uh oh!

craigcomstock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vpodzime left a comment

Choose a reason for hiding this comment

Uh oh!

larsewi commented Dec 9, 2024 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

ENT-12414: Use `librsync`'s Stream API in `GET <FILENAME>` requests #5629

ENT-12414: Use `librsync`'s Stream API in `GET <FILENAME>` requests #5629

larsewi commented Nov 19, 2024 •

edited

Loading

larsewi commented Dec 9, 2024 •

edited by atlassian bot

Loading