On 04/05/2011 09:08 AM, Ivana Hutarova Varekova wrote:
> On 03/14/2011 05:24 PM, Jan Safranek wrote:
>> Various tests for cgclassify tool, including error cases and testing with
>> /etc/cgrules.conf. The tests will produce error messages to output, but it's
>> expected, reaction of cgclassify to wrong input is being tested as well.
>>
>> Signed-off-by: Jan Safranek<jsafr...@redhat.com>
> I have only minor problems with this patch otherwise it looks ok.
Thanks for the review!
>> ---
>>
>> tests/tools/Makefile.am | 2
>> tests/tools/cgclassify/Makefile.am | 3 +
>> tests/tools/cgclassify/cgclassify | 134 ++++++++++++++++++++++++
>> tests/tools/cgclassify/cgclassify-rules | 171
>> +++++++++++++++++++++++++++++++
>> tests/tools/cgclassify/simple.conf | 24 ++++
>> 5 files changed, 333 insertions(+), 1 deletions(-)
>> create mode 100644 tests/tools/cgclassify/Makefile.am
>> create mode 100755 tests/tools/cgclassify/cgclassify
>> create mode 100755 tests/tools/cgclassify/cgclassify-rules
>> create mode 100644 tests/tools/cgclassify/simple.conf
>>
>> diff --git a/configure.in b/configure.in
>> index ba8e1b1..03b2731 100644
>> --- a/configure.in
>> +++ b/configure.in
>> @@ -196,6 +196,7 @@ AC_CONFIG_FILES([Makefile
>> tests/tools/testenv.sh
>> tests/tools/Makefile
>> tests/tools/cgconfigparser/Makefile
>> + tests/tools/cgclassify/Makefile
>> src/Makefile
>> src/daemon/Makefile
>> src/tools/Makefile
>> diff --git a/tests/tools/Makefile.am b/tests/tools/Makefile.am
>> index 5c08525..4f2b1b0 100644
>> --- a/tests/tools/Makefile.am
>> +++ b/tests/tools/Makefile.am
>> @@ -1,2 +1,2 @@
>> -SUBDIRS = cgconfigparser
>> +SUBDIRS = cgconfigparser cgclassify
>>
>> diff --git a/tests/tools/cgclassify/Makefile.am
>> b/tests/tools/cgclassify/Makefile.am
>> new file mode 100644
>> index 0000000..c504e7f
>> --- /dev/null
>> +++ b/tests/tools/cgclassify/Makefile.am
>> @@ -0,0 +1,3 @@
>> +EXTRA_DIST = cgclassify cgclassify-rules simple.conf
>> +
>> +TESTS = cgclassify cgclassify-rules
>> diff --git a/tests/tools/cgclassify/cgclassify
>> b/tests/tools/cgclassify/cgclassify
>> new file mode 100755
>> index 0000000..9560a15
>> --- /dev/null
>> +++ b/tests/tools/cgclassify/cgclassify
>> @@ -0,0 +1,134 @@
>> +#!/bin/bash
>> +#
>> +# Test cgclassify with various arguments, without /etc/cgrules.conf.
>> +# cglassify is tested with exact destination group, multiple PIDs, groups
>> +# specified by '*', multiple target groups, lot of PIDs on command line
>> +# and various error cases.
>> +
>> +. `dirname $0`/../testenv.sh
>> +
>> +function checkpid()
>> +{
>> + # check that given process is in given groups
>> + local PID=$1
>> + # delete hierarchy number, ignore systemd
>> + cat /proc/$PID/cgroup | sed 's/^[0-9]*://' | grep -v systemd>
>> $TMP/pid-$PID.group
>> + printf>$TMP/pid-$PID.expected "$2"
>> + diff -u -w $TMP/pid-$PID.group $TMP/pid-$PID.expected
>> + return $?
>> +}
>> +
>> +function resetgroup()
>> +{
>> + # move given processes back to root group
>> + $TOOLSDIR/cgclassify -g "*:/" $*
>> +}
>> +
>> +# prepare some hierarchy
>> +$TOOLSDIR/cgconfigparser -l `prepare_config simple.conf` || \
>> + die "cannot parse simple.conf"
>> +
>> +# start few processes to torture
>> +/bin/sleep 10000&
>> +PID1=$!
>> +/bin/sleep 10000&
>> +PID2=$!
>> +/bin/sleep 10000&
>> +PID3=$!
>> +
>> +# STEP1: simple cgclassify to exact groups
>> +$TOOLSDIR/cgclassify -g net_cls,cpu:common $PID1 || \
>> + die "STEP1: cgclassify PID1 failed"
>> +$TOOLSDIR/cgclassify -g net_cls:net1 $PID2 || \
>> + die "STEP1: cgclassify PID2 failed"
>> +$TOOLSDIR/cgclassify -g cpu:cpu1 $PID3 || \
>> + die "STEP1: cgclassify PID3 failed"
>> +
>> +checkpid $PID1 "net_cls,freezer:/common\ncpuacct,cpu:/common\n" || \
>> + die "STEP1: unexpected group of pid1"
>> +checkpid $PID2 "net_cls,freezer:/net1\ncpuacct,cpu:/\n" || \
>> + die "STEP!: unexpected group of pid2"
>> +checkpid $PID3 "net_cls,freezer:/\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP!: unexpected group of pid3"
>> +
>> +# STEP2: try * and more PIDs
>> +$TOOLSDIR/cgclassify -g "*:/" $PID1 $PID2 $PID3 || \
>> + die "cgclassify 2 failed"
>> +checkpid $PID1 "net_cls,freezer:/\ncpuacct,cpu:/\n" || \
>> + die "STEP2: unexpected group of pid1"
>> +checkpid $PID2 "net_cls,freezer:/\ncpuacct,cpu:/\n" || \
>> + die "STEP2: unexpected group of pid2"
>> +checkpid $PID3 "net_cls,freezer:/\ncpuacct,cpu:/\n" || \
>> + die "STEP2: unexpected group of pid3"
>> +
>> +# STEP3: try different groups
>> +resetgroup $PID1 $PID2 $PID3
>> +$TOOLSDIR/cgclassify -g cpu:cpu1 -g net_cls:net1 $PID1
>> +checkpid $PID1 "net_cls,freezer:/net1\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP3: unexpected group of pid1"
>> +
>> +# STEP4: different groups multiple times (tha last should win)
>> +resetgroup $PID1 $PID2 $PID3
>> +$TOOLSDIR/cgclassify -g "*:/" -g cpu:common -g net_cls:common -g cpu:cpu1
>> -g net_cls:net1 $PID1 || \
>> + die "STEP4: cgclassify pid1 failed"
>> +checkpid $PID1 "net_cls,freezer:/net1\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP4: unexpected group of pid1"
>> +$TOOLSDIR/cgclassify -g "*:/" -g cpu:common -g net_cls:common -g cpu:cpu1
>> $PID2 || \
>> + die "STEP4: cgclassify pid2 failed"
>> +checkpid $PID2 "net_cls,freezer:/common\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP4: unexpected group of pid2"
>> +
>> +# STEP5: some error cases
> I preffer here to have the error messages redirected to /dev/null, the
> output is less readable with them
When the test goes wrong, there is no way how to analyze it without
stdout and stderr.
>> +# group does not exist
>> +echo "Testing error cases, error messages will appear!"
>> +$TOOLSDIR/cgclassify -g cpu:invalid_group $PID1&& \
>> + die "STEP5: cgclassify with invalig_group succeeded"
>> +# parameter is not a PID
>> +$TOOLSDIR/cgclassify -g cpu:/ xxx&& \
>> + die "STEP5: cgclassify with xxx pid succeeded"
>> +# let's hope process 1234567 does not exist
>> +$TOOLSDIR/cgclassify -g cpu:/ 1234567&& \
>> + die "STEP5: cgclassify with 1234567 succeeded"
>> +# not-mounted controller
>> +$TOOLSDIR/cgclassify -g xxx:/ $PID1&& \
>> + die "STEP5: cgclassify with xxx controller succeeded"
>> +# no -g parameter
>> +$TOOLSDIR/cgclassify -g $PID1&& \
>> + die "STEP5: cgclassify without -g succeeded"
>> +# invalid -g format
>> +$TOOLSDIR/cgclassify -g cpu/cpu1 $PID1&& \
>> + die "STEP5: cgclassify -g cpu/cpu1 succeeded"
>> +
>> +# some existing processes among unexisting
>> +resetgroup $PID1 $PID2 $PID3
>> +$TOOLSDIR/cgclassify -g cpu,net_cls:common $PID1 1234567 $PID2 1234568
>> $PID3&& \
>> + die "STEP5: cgclassify with mixed processed succeeded"
>> +checkpid $PID1 "net_cls,freezer:/common\ncpuacct,cpu:/common\n" || \
>> + die "STEP5: unexpected group of pid1"
>> +checkpid $PID2 "net_cls,freezer:/common\ncpuacct,cpu:/common\n" || \
>> + die "STEP5: unexpected group of pid2"
>> +checkpid $PID3 "net_cls,freezer:/common\ncpuacct,cpu:/common\n" || \
>> + die "STEP5: unexpected group of pid3"
>> +
>> +echo "End of error cases"
>> +
>> +# STEP6: *lot of* processes on command line
>> +echo "Testing lot of arguments, this will take some time"
>> +COUNT=1000
>
> Not sure whether the variable is necessary it is used only one time
I just think it's easier to find the number if it has a name.
So, as result of this review, I take the liberty of letting the patch as
it is now.
>> +echo>$TMP/pids
>> +(
>> + for i in `seq $COUNT`; do
>> + sleep 100000&
>> + echo $!>>$TMP/pids
>> + done
>> +)> /dev/null
>> +$TOOLSDIR/cgclassify -g net_cls,cpu:common `cat $TMP/pids` || \
>> + die "STEP6: cgclassify failed"
>> +
>> +kill `cat $TMP/pids`
>> +sleep 1 # to settle down the sleep load - sigterm does not kill sleep
>> immediatelly
>> +
>> +kill $PID1 $PID2 $PID3
>> +$TOOLSDIR/cgclear
>> +cleanup
>> +exit 0
>> diff --git a/tests/tools/cgclassify/cgclassify-rules
>> b/tests/tools/cgclassify/cgclassify-rules
>> new file mode 100755
>> index 0000000..b2b2cd8
>> --- /dev/null
>> +++ b/tests/tools/cgclassify/cgclassify-rules
>> @@ -0,0 +1,171 @@
>> +#!/bin/bash
>> +#
>> +# Test cgclassify with various /etc/cgrules.conf settings, like
>> +# simple rules, multiple matching rules, using @groups, executable
>> +# names and default rules.
>> +# The test relies on testenv.sh to backup/restore /etc/cgrules.conf.
>> +#
>> +
>> +. `dirname $0`/../testenv.sh
>> +
>> +# The test need sto start few processes with non-root UID/GID. Which should
>> be
>> +# used?
>> +TESTUSER=nobody
>> +TESTGROUP=nobody
>> +
>> +function checkpid()
>> +{
>> + # check that given process is in given groups
>> + local PID=$1
>> + # delete hierarchy number, ignore systemd
>> + cat /proc/$PID/cgroup | sed 's/^[0-9]*://' | grep -v systemd>
>> $TMP/pid-$PID.group
>> + printf>$TMP/pid-$PID.expected "$2"
>> + diff -u -w $TMP/pid-$PID.group $TMP/pid-$PID.expected
>> + return $?
>> +}
>> +
>> +function resetgroup()
>> +{
>> + # move given processes back to root group
>> + $TOOLSDIR/cgclassify -g "*:/" $*
>> +}
>> +
>> +# prepare some hierarchy
>> +$TOOLSDIR/cgconfigparser -l `prepare_config simple.conf` || \
>> + die "cannot parse simple.conf"
>> +
>> +# prepare specific process names
>> +ln -s /bin/sleep $TMP/sleep1
>> +$TMP/sleep1 10000&
>> +PID1=$!
>> +ln -s /bin/sleep $TMP/sleep2
>> +$TMP/sleep2 10000&
>> +PID2=$!
>> +
>> +# start some processes as $TESTUSER
>> +chmod o+rwX $TMP
>> +su $TESTUSER -s /bin/bash -c "$TMP/sleep1 10000"&
>> +su $TESTUSER -s /bin/bash -c "$TMP/sleep2 10000"&
>> +sleep 0.1
>> +NPID1=`ps h -u $TESTUSER | grep sleep1 | awk '{ print $1; }' | tail -n 1`
>> +NPID2=`ps h -u $TESTUSER | grep sleep2 | awk '{ print $1; }' | tail -n 1`
>> +
>> +# STEP1: simple global rule
>> +cat<<EOF>/etc/cgrules.conf
>> +unused * /
>> +* * common
>> +EOF
>> +$TOOLSDIR/cgclassify $PID1
>> +checkpid $PID1 "net_cls,freezer:/common\ncpuacct,cpu:/common\n" || \
>> + die "STEP1: unexpected group of pid1"
>> +resetgroup $PID1
>> +
>> +# STEP2: two destination groups
>> +cat<<EOF>/etc/cgrules.conf
>> +unused * /
>> +* cpu cpu1
>> +% net_cls net1
>> +EOF
>> +$TOOLSDIR/cgclassify $PID1
>> +checkpid $PID1 "net_cls,freezer:/net1\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP2: unexpected group of pid1"
>> +resetgroup $PID1
>> +
>> +# STEP3: two matching rules, only the first is executed
>> +cat<<EOF>/etc/cgrules.conf
>> +unused * /
>> +* cpu cpu1
>> +* net_cls net1
>> +EOF
>> +$TOOLSDIR/cgclassify $PID1
>> +checkpid $PID1 "net_cls,freezer:/\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP3: unexpected group of pid1"
>> +resetgroup $PID1
>> +
>> +# STEP4: process name in a rule
>> +cat<<EOF>/etc/cgrules.conf
>> +unused * /
>> +*:sleep1 cpu cpu1
>> +*:sleep2 net_cls net1
>> +EOF
>> +$TOOLSDIR/cgclassify $PID1
>> +checkpid $PID1 "net_cls,freezer:/\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP4: unexpected group of pid1"
>> +$TOOLSDIR/cgclassify $PID2
>> +checkpid $PID2 "net_cls,freezer:/net1\ncpuacct,cpu:/\n" || \
>> + die "STEP4: unexpected group of pid2"
>> +resetgroup $PID1 $PID2
>> +
>> +# STEP5: full path
>> +cat<<EOF>/etc/cgrules.conf
>> +unused * /
>> +*:$TMP/sleep1 cpu cpu1
>> +*:$TMP/sleep2 net_cls net1
>> +EOF
>> +$TOOLSDIR/cgclassify $PID1
>> +checkpid $PID1 "net_cls,freezer:/\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP5: unexpected group of pid1"
>> +$TOOLSDIR/cgclassify $PID2
>> +checkpid $PID2 "net_cls,freezer:/net1\ncpuacct,cpu:/\n" || \
>> + die "STEP5: unexpected group of pid2"
>> +resetgroup $PID1 $PID2
>> +
>> +#STEP6: username
>> +cat<<EOF>/etc/cgrules.conf
>> +unused * /
>> +$TESTUSER cpu cpu1
>> +* net_cls net1
>> +EOF
>> +$TOOLSDIR/cgclassify $PID1 $NPID1
>> +checkpid $NPID1 "net_cls,freezer:/\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP6: unexpected group of npid1"
>> +checkpid $PID1 "net_cls,freezer:/net1\ncpuacct,cpu:/\n" || \
>> + die "STEP6: unexpected group of pid1"
>> +resetgroup $PID1 $NPID1
>> +
>> +#STEP7: username + processname
>> +cat<<EOF>/etc/cgrules.conf
>> +unused * /
>> +$TESTUSER:$TMP/sleep1 cpu cpu1
>> +*:$TMP/sleep1 net_cls net1
>> +EOF
>> +$TOOLSDIR/cgclassify $PID1 $NPID1
>> +checkpid $NPID1 "net_cls,freezer:/\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP7: unexpected group of npid1"
>> +checkpid $PID1 "net_cls,freezer:/net1\ncpuacct,cpu:/\n" || \
>> + die "STEP7: unexpected group of pid1"
>> +resetgroup $PID1 $NPID1
>> +
>> +#STEP8: groupname + processname
>> +cat<<EOF>/etc/cgrules.conf
>> +unused * /
>> +@$TESTGROUP:$TMP/sleep1 cpu cpu1
>> +*:$TMP/sleep1 net_cls net1
>> +EOF
>> +$TOOLSDIR/cgclassify $PID1 $NPID1
>> +checkpid $NPID1 "net_cls,freezer:/\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP8: unexpected group of npid1"
>> +checkpid $PID1 "net_cls,freezer:/net1\ncpuacct,cpu:/\n" || \
>> + die "STEP8: unexpected group of pid1"
>> +resetgroup $PID1 $NPID1
>> +
>> +#STEP9: processname + default
>> +cat<<EOF>/etc/cgrules.conf
>> +unused * /
>> +*:$TMP/sleep1 cpu cpu1
>> +* net_cls net1
>> +EOF
>> +$TOOLSDIR/cgclassify $PID1 $PID2
>> +checkpid $PID1 "net_cls,freezer:/\ncpuacct,cpu:/cpu1\n" || \
>> + die "STEP9: unexpected group of pid1"
>> +checkpid $PID2 "net_cls,freezer:/net1\ncpuacct,cpu:/\n" || \
>> + die "STEP9: unexpected group of pid2"
>> +resetgroup $PID1 $NPID1
>> +
>> +kill -9 $PID1 $PID2 $NPID1 $NPID2
>> +
>> +sleep 0.1
>> +$TOOLSDIR/cgclear
>> +
>> +cleanup
>> +exit 0
>> diff --git a/tests/tools/cgclassify/simple.conf
>> b/tests/tools/cgclassify/simple.conf
>> new file mode 100644
>> index 0000000..07f8ccd
>> --- /dev/null
>> +++ b/tests/tools/cgclassify/simple.conf
>> @@ -0,0 +1,24 @@
>> +# Two hierarchies, two controllers each:
>> +
>> +mount {
>> + cpu = TMP/cgroup/cpu;
>> + cpuacct = TMP/cgroup/cpu;
>> + net_cls = TMP/cgroup/net;
>> + freezer = TMP/cgroup/net;
>> +}
>> +
>> +# One group common for all hierarchies:
>> +
>> +group common {
>> + cpu {}
>> + net_cls {}
>> +}
>> +
>> +# Two separate groups:
>> +group net1 {
>> + net_cls{}
>> +}
>> +
>> +group cpu1 {
>> + cpu {}
>> +}
>>
>>
>> ------------------------------------------------------------------------------
>> Colocation vs. Managed Hosting
>> A question and answer guide to determining the best fit
>> for your organization - today and in the future.
>> http://p.sf.net/sfu/internap-sfd2d
>> _______________________________________________
>> Libcg-devel mailing list
>> Libcg-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/libcg-devel
>
>
> ------------------------------------------------------------------------------
> Xperia(TM) PLAY
> It's a major breakthrough. An authentic gaming
> smartphone on the nation's most reliable network.
> And it wants your games.
> http://p.sf.net/sfu/verizon-sfdev
> _______________________________________________
> Libcg-devel mailing list
> Libcg-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/libcg-devel
------------------------------------------------------------------------------
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
