Add CU scenario for puma worker count #13544
Conversation
Griffin-Sullivan
added
CherryPick
|
trigger: test-robottelo |
Griffin-Sullivan
force-pushed
the
puma-install
branch
from
4261498 to
f813003
Compare
|
trigger: test-robottelo |
Griffin-Sullivan
force-pushed
the
puma-install
branch
from
f813003 to
0191b31
Compare
|
trigger: test-robottelo |
| :customerscenario: true | ||
| """ | ||
| result = target_sat.execute('ps aux | grep -v grep | grep -e USER -e puma') | ||
| for i in range(9): |
There was a problem hiding this comment.
this feels a bit fragile towards future default value changes, would it make sense to first source the default setting from the answer file?
There was a problem hiding this comment.
The answer file is blank which is what makes this create 9 puma workers by default. We could instead count the amount of puma workers and then range off of the len?
There was a problem hiding this comment.
Ok so I learned this piece of code determines worker count https://github.com/theforeman/puppet-foreman/blob/908ed638790f3cfc6e2f5f62e1b023c907a9913c/manifests/config.pp#L83C1-L92. Therefore, I changed the test to record what the current worker count is, run the installer with worker count set to a number less than that count, and then assert it worked.
|
@Gauravtalreja1 sorry didn't mean to request review. The GH page I was looking at was outdated. |
Griffin-Sullivan
force-pushed
the
puma-install
branch
from
0191b31 to
d610fd3
Compare
|
trigger: test-robottelo |
Griffin-Sullivan
force-pushed
the
puma-install
branch
from
d610fd3 to
cf19594
Compare
|
trigger: test-robottelo |
Griffin-Sullivan
force-pushed
the
puma-install
branch
from
cf19594 to
49bcaf8
Compare
|
trigger: test-robottelo |
| 1. Check how many puma workers there are by default (9) | ||
| 2. Change answer's file to have 5 puma workers | ||
| 3. Run satellite-installer --foreman-foreman-service-puma-workers 5 --foreman-foreman-service-puma-threads-max 5 |
There was a problem hiding this comment.
Are these steps still accurate, or should they be updated based on the newest version of the test?
| numbers = range(1, count)[:-1] | ||
| worker_count = str(random.choice(numbers)) |
There was a problem hiding this comment.
Can you add a comment line explaining what these are doing?
also, if you just need a string number, would this also work?
| numbers = range(1, count)[:-1] | |
| worker_count = str(random.choice(numbers)) | |
| worker_count = str(random.randint(1, count)) |
There was a problem hiding this comment.
yeah I'll add your suggestion and a comment
| result = target_sat.execute('ps aux | grep -v grep | grep -e USER -e puma') | ||
| count = 0 | ||
| for line in result.stdout.splitlines(): | ||
| if 'puma: cluster worker' in line: | ||
| count += 1 |
There was a problem hiding this comment.
likely easier to do it this way
| result = target_sat.execute('ps aux | grep -v grep | grep -e USER -e puma') | |
| count = 0 | |
| for line in result.stdout.splitlines(): | |
| if 'puma: cluster worker' in line: | |
| count += 1 | |
| count = int(target_sat.execute('pgrep --full "puma: cluster worker" | wc -l').stdout) |
| result = target_sat.execute('ps aux | grep -v grep | grep -e USER -e puma') | ||
| for i in range(count): | ||
| if i < int(worker_count): | ||
| assert f'cluster worker {i}' in result.stdout | ||
| else: | ||
| assert f'cluster worker {i}' not in result.stdout |
There was a problem hiding this comment.
see above for better process counting
There was a problem hiding this comment.
What I liked about this loop is it also asserted that there were certain workers not in the output
Griffin-Sullivan
force-pushed
the
puma-install
branch
from
49bcaf8 to
9e0e296
Compare
|
trigger: test-robottelo |
Problem Statement
Missing coverage for BZ 2025760.
Solution
Add new test to cover the scenario for changing the puma worker count.
Related Issues