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.

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

Add support for SoC separate var partition #518

Merged
merged 1 commit into from
Aug 31, 2024

Conversation

Yarboa
Copy link
Collaborator

@Yarboa Yarboa commented Aug 28, 2024

Updating tests/e2e/set-ffi-env-e2e to mkfs.ext4
On free sde partiton it finds VROOM-21987

@dougsland
Copy link
Collaborator

dougsland commented Aug 28, 2024

I don't believe these are related to your change, already exists as we talked. Should be okay to merge as soon you have a final patch.

/tests/ffi/memory (timeout)

Copying config sha256:728d9197c2ef33c51583e7e162d6055d654208c789a54dd033cb82607f81b444
Writing manifest to image destination
Getting image source signatures
Writing manifest to image destination

Maximum test time '20m' exceeded.
Adjust the test 'duration' attribute if necessary.
https://tmt.readthedocs.io/en/stable/spec/tests.html#duration

/tests/ffi/qm-oom-score-adj

Error: OCI runtime error: crun: the requested cgroup controller `pids` is not available
[ INFO  ] PASS: qm.container oom_score_adj value == 500
./test.sh: line 39: [: cat: /proc/0/oom_score_adj: No such file or directory: integer expression expected
[ INFO  ] FAIL: qm containers oom_score_adj != 750. Current value is cat: /proc/0/oom_score_adj: No such file or directory
Shared connection to 13.59.78.82 closed.

@dougsland
Copy link
Collaborator

Output from CI/CD also from your patch (pids exists from cgroup2):

Run echo "Checking cgroup version and settings..."
Checking cgroup version and settings...
nodev	cgroup
nodev	cgroup2
cgroup2 on /sys/fs/cgroup type cgroup2 (rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot)
#subsys_name	hierarchy	num_cgroups	enabled
cpuset	0	[1](https://github.com/containers/qm/actions/runs/10588279250/job/29340520739?pr=518#step:3:1)09	1
cpu	0	109	1
cpuacct	0	109	1
blkio	0	109	1
memory	0	109	1
devices	0	109	1
freezer	0	109	1
net_cls	0	10[9](https://github.com/containers/qm/actions/runs/10588279250/job/29340520739?pr=518#step:3:10)	1
perf_event	0	[10](https://github.com/containers/qm/actions/runs/10588279250/job/29340520739?pr=518#step:3:11)9	1
net_prio	0	109	1
hugetlb	0	109	1
pids	0	109	1
rdma	0	109	1
misc	0	109	1

@dougsland dougsland added the jira label Aug 28, 2024
@Yarboa Yarboa marked this pull request as draft August 28, 2024 13:39
@Yarboa Yarboa force-pushed the pinst-var-part branch 4 times, most recently from 14e5ce0 to 5adffd8 Compare August 29, 2024 03:21
@Yarboa Yarboa self-assigned this Aug 29, 2024
@Yarboa Yarboa marked this pull request as ready for review August 29, 2024 03:34
@Yarboa Yarboa force-pushed the pinst-var-part branch 2 times, most recently from ae03585 to 5ec8b93 Compare August 29, 2024 04:12
@Yarboa
Copy link
Collaborator Author

Yarboa commented Aug 29, 2024

@dougsland ready to merge, if now comments
It seems that tests/ffi/qm-oom-score-adj/test.sh returned.
I will open another issue

tests/e2e/lib/diskutils Outdated Show resolved Hide resolved
tests/e2e/lib/diskutils Outdated Show resolved Hide resolved
@Yarboa Yarboa force-pushed the pinst-var-part branch 2 times, most recently from 5380cfd to 90544ff Compare August 30, 2024 00:04
tests/e2e/lib/diskutils Outdated Show resolved Hide resolved
tests/e2e/lib/diskutils Outdated Show resolved Hide resolved
@Yarboa Yarboa force-pushed the pinst-var-part branch 2 times, most recently from b4828e5 to b4ec46a Compare August 30, 2024 18:03
Updating tests/e2e/set-ffi-env-e2e to mkfs.ext4
On free sde partiton it finds
Move disk functions to lib/diskutils script
Increase timers for memory and qm-oom-score-adj

Signed-off-by: Yariv Rachmani <[email protected]>

for disk in $disks_arr; do
if [[ ${disk} == "vda" || \
$(echo "${disk_table}" | grep -c "${disk}" ) -eq 1 && ${disk} != "zram0" ]];then
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like out of indent

fi

mkdir -p /new_var
mount "/dev/${DISK}${PART_ID}" /new_var
Copy link
Collaborator

@dougsland dougsland Aug 31, 2024

Choose a reason for hiding this comment

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

if_error_exit?

mount "/dev/${DISK}${PART_ID}" /new_var
rsync -aqxXP /var/* /new_var
if_error_exit "Error: rsync failed"
systemctl stop var-lib-nfs-rpc_pipefs.mount
Copy link
Collaborator

Choose a reason for hiding this comment

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

if_error_exit? if not, better comment.

rsync -aqxXP /var/* /new_var
if_error_exit "Error: rsync failed"
systemctl stop var-lib-nfs-rpc_pipefs.mount
umount /new_var
Copy link
Collaborator

Choose a reason for hiding this comment

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

if_error_exit? or just warning? or just echo before ?

echo
info_message "Checking if QM already installed"
info_message "=============================="
QM_STATUS="$(systemctl is-enabled qm 2>&1)"
if [ "$QM_STATUS" == "generated" ]; then
if [ "$(systemctl is-active qm)" == "active" ]; then
# Restart QM after mount /var on separate partition
if grep -qi "${QC_SOC}" "${SOC_DISTRO_FILE}"; then
systemctl restart qm
Copy link
Collaborator

Choose a reason for hiding this comment

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

if_error_exit?

Copy link
Collaborator

@dougsland dougsland left a comment

Choose a reason for hiding this comment

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

LGTM, just small comments.

@dougsland dougsland merged commit 4fe1420 into containers:main Aug 31, 2024
7 of 9 checks passed
@dougsland
Copy link
Collaborator

The patch is okay to merge now, we can handle the rest in a next patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants