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

DESKTOP=gnome System's ram must be at least 4GB #18521

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rfan1
Copy link
Contributor

@rfan1 rfan1 commented Jan 25, 2024

@rfan1 rfan1 added WIP Work in progress qe-core Tag used to filter PR's created by QE-Core's members or are assigned to them labels Jan 25, 2024
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

  • Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files.

@ilmanzo
Copy link
Contributor

ilmanzo commented Jan 25, 2024

I like the concept and the code is perfect, but why do we need yet another variable to pass just for this specific case ? e.g. GNOME_MEMORY_GE_4G=1 ?

@foursixnine
Copy link
Member

foursixnine commented Jan 25, 2024

I like the concept and the code is perfect, but why do we need yet another variable to pass just for this specific case ? e.g. GNOME_MEMORY_GE_4G=1 ?

From my comment on the progress ticket:

we will still need to add a followup where we certify that effectively 2gb is the absolute minimum, with kdump disabled for gnome

That variable basically will pave the way for the follow up

@rfan1 rfan1 force-pushed the gnome_4gb_memory branch 2 times, most recently from 1f8234d to 49d398d Compare January 26, 2024 02:20
@rfan1
Copy link
Contributor Author

rfan1 commented Jan 26, 2024

I like the concept and the code is perfect, but why do we need yet another variable to pass just for this specific case ? e.g. GNOME_MEMORY_GE_4G=1 ?

From my comment on the progress ticket:

we will still need to add a followup where we certify that effectively 2gb is the absolute minimum, with kdump disabled for gnome

That variable basically will pave the way for the follow up

Thanks for your kindly comments, I wanted to use NO_KDUMP as a switch, but seems it might not be a perfect one, since we may install the system without kdump but enable kdump tests later :)

And add a new variable GNOME_MEMORY_GE_4G=1 can help us to eliminate effects for some workers with limit memory resource [xen/vmware etc]

@rfan1 rfan1 removed the WIP Work in progress label Jan 26, 2024
Copy link
Contributor

@alvarocarvajald alvarocarvajald 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 a small suggestion.

lib/utils.pm Outdated Show resolved Hide resolved
@rfan1
Copy link
Contributor Author

rfan1 commented Feb 1, 2024

@foursixnine If you want to approve/merge this PR, please help inform other squads, since it will introduce new failures if QEMURAM<4096 :)

@foursixnine
Copy link
Member

@foursixnine If you want to approve/merge this PR, please help inform other squads, since it will introduce new failures if QEMURAM<4096 :)

They have been informed via Slack and via Matrix https://suse.slack.com/archives/C02CANHLANP/p1706264121953869 but we can mention them here:

@os-autoinst/tests-maintainer FYI

@rfan1 can you check on Tumbleweed and Leap and adapt changes if necessary? @DimStar77 @lkocman

@rfan1
Copy link
Contributor Author

rfan1 commented Feb 1, 2024

@foursixnine If you want to approve/merge this PR, please help inform other squads, since it will introduce new failures if QEMURAM<4096 :)

They have been informed via Slack and via Matrix https://suse.slack.com/archives/C02CANHLANP/p1706264121953869 but we can mention them here:

@os-autoinst/tests-maintainer FYI

@rfan1 can you check on Tumbleweed and Leap and adapt changes if necessary? @DimStar77 @lkocman

https://en.opensuse.org/Hardware_requirements_15.5, I can see that no such requirement for leap, but for TW, I failed to find the document

@foursixnine
Copy link
Member

@foursixnine If you want to approve/merge this PR, please help inform other squads, since it will introduce new failures if QEMURAM<4096 :)

They have been informed via Slack and via Matrix https://suse.slack.com/archives/C02CANHLANP/p1706264121953869 but we can mention them here:
@os-autoinst/tests-maintainer FYI
@rfan1 can you check on Tumbleweed and Leap and adapt changes if necessary? @DimStar77 @lkocman

https://en.opensuse.org/Hardware_requirements_15.5, I can see that no such requirement for leap, but for TW, I failed to find the document

I'm pretty sure Leap and TW will have the same limitation, so we'd need to check by running some tests, but I would like to avoid a is_opensuse in the memory check in general

@rfan1
Copy link
Contributor Author

rfan1 commented Feb 1, 2024

@foursixnine If you want to approve/merge this PR, please help inform other squads, since it will introduce new failures if QEMURAM<4096 :)

They have been informed via Slack and via Matrix https://suse.slack.com/archives/C02CANHLANP/p1706264121953869 but we can mention them here:
@os-autoinst/tests-maintainer FYI
@rfan1 can you check on Tumbleweed and Leap and adapt changes if necessary? @DimStar77 @lkocman

https://en.opensuse.org/Hardware_requirements_15.5, I can see that no such requirement for leap, but for TW, I failed to find the document

I'm pretty sure Leap and TW will have the same limitation, so we'd need to check by running some tests, but I would like to avoid a is_opensuse in the memory check in general

Well, even is_tumbleweed is not recommended, this new logic is more simple :)

@Vogtinator
Copy link
Member

Most GNOME tests on TW use QEMURAM=1536 and work fine.

The linked bug report is specific to ppc64le and some SAP stuff FWICT.

lib/utils.pm Outdated Show resolved Hide resolved
@foursixnine
Copy link
Member

Most GNOME tests on TW use QEMURAM=1536 and work fine.

The linked bug report is specific to ppc64le and some SAP stuff FWICT.

We'll follow up https://progress.opensuse.org/issues/153808#note-6 to find out the lower limit and normalize it for both, while the bug was specific for ppc64le, we've had the same problem with other architectures, specially when enabling kdump (which takes considerably larger portion on ppc64le) and likely create the follow up ticket during review

@Vogtinator
Copy link
Member

FWICT this will break pretty much all GNOME tests on Leap 15.5, 15.6 and Tumbleweed?

@foursixnine
Copy link
Member


FWICT this will break pretty much all GNOME tests on Leap 15.5, 15.6 and Tumbleweed?

This we need to modify before merging, so that openSUSE is not affected, and is aligned with the strategy.

as it is Friday... next week perhaps :D - The minimum memory requirements will be tested via: #18521 (comment) as a follow up

@rfan1
Copy link
Contributor Author

rfan1 commented Feb 4, 2024

FWICT this will break pretty much all GNOME tests on Leap 15.5, 15.6 and Tumbleweed?

This we need to modify before merging, so that openSUSE is not affected, and is aligned with the strategy.

as it is Friday... next week perhaps :D - The minimum memory requirements will be tested via: #18521 (comment) as a follow up

Draft PR for o3, os-autoinst/opensuse-jobgroups#424.

For SLE tests, It should be better if each squad can take care of it's own job groups :)

@rfan1 rfan1 removed the qe-core Tag used to filter PR's created by QE-Core's members or are assigned to them label Mar 6, 2024
@rfan1 rfan1 marked this pull request as draft March 6, 2024 09:17
@rfan1
Copy link
Contributor Author

rfan1 commented Mar 6, 2024

Mark it as draft now, since we still need check with other squads and O3 user to adapt the job settings before merging this PR. I will double confirm with @foursixnine on this ticket.

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