Re: [PATCH master 3/3] Fix KVM pinning tests to not depend on the local machine

['Viktor Bachraty' via ganeti-devel]

On Fri, Jun 10, 2016 at 10:02 AM, Iustin Pop  wrote:

> 2016-06-10 10:58 GMT+02:00 Viktor Bachraty :
>
>>
>> On Thu, Jun 9, 2016 at 11:33 PM, Iustin Pop  wrote:
>>
>>> From: Iustin Pop 
>>>
>>> Commit 8b2ec2f added unittests for KVM pinning, but it introduced a
>>> non-obvious
>>> local dependency in the tests: the CPU_PINNING_OFF calls work by looking
>>> at the
>>> (current) machine's core count, and pinning to all those CPUs. In order
>>> to make
>>> this work independently from the test machine, we must also mock the
>>> result of
>>> process.cpu_count(). Do this by using a core count that is very much
>>> unlikely
>>> to ever be present in the real world.
>>>
>>> Signed-off-by: Iustin Pop 
>>> ---
>>>  test/py/ganeti.hypervisor.hv_kvm_unittest.py | 11 +++
>>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py b/test/py/
>>> ganeti.hypervisor.hv_kvm_unittest.py
>>> index c7a53b5..55ffb9b 100755
>>> --- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
>>> +++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
>>> @@ -37,6 +37,7 @@ import socket
>>>  import os
>>>  import struct
>>>  import re
>>> +from contextlib import nested
>>>
>>>  from ganeti import serializer
>>>  from ganeti import constants
>>> @@ -636,12 +637,13 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
>>>  cpu_mask = self.params['cpu_mask']
>>>  worker_cpu_mask = self.params['worker_cpu_mask']
>>>  hypervisor = hv_kvm.KVMHypervisor()
>>> -with mock.patch('psutil.Process', return_value=mock_process):
>>> +with nested(mock.patch('psutil.Process', return_value=mock_process),
>>> +mock.patch('psutil.cpu_count', return_value=1237)):
>>>
>>
>> 'nested' is deprecated in Python 2.7 but unfortunately 2.6 does not
>> support comma separated contexts as 2.7 does.
>>
>
> Yep, saw that. However, nested still works (no warnings) in 2.6 as well,
> so it should be good for now.
>
>
>> All Debian versions newer than Squeeze should have 2.7 already. I believe
>> once master goes stable, we can safely drop support for Python <2.7, what
>>  do you think?
>>
>
> It is possible, but if we want to do that, I think we should do it across
> the board. E.g. on Haskell side we have a number of workaround for old
> compilers/library versions, and we could do some cleanup if we remove that,
> but we don't need to hurry.
>

> So declaring that some version (e.g. 2.19, or 2.20 - that is a round
> number) is only supported on (Jessie+, or 2015+, or something) would be a
> good move.
>

I tend see too much backwards compatibility more as a burden than a
feature, but agree that we should do declare/decide about it at release
time, not in a small patch.
LGTM.

thanks for your patches,
viktor


>
> thanks,
> iustin
>
>hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask,
>>> worker_cpu_mask)
>>>
>>>  self.assertEqual(mock_process.set_cpu_affinity.call_count, 1)
>>>  self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
>>> - mock.call(range(0,12)))
>>> + mock.call(range(0,1237)))
>>>
>>>def testCpuPinningPerVcpu(self):
>>>  mock_process = mock.MagicMock()
>>> @@ -659,11 +661,12 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
>>>  def get_mock_process(unused_pid):
>>>return mock_process
>>>
>>> -with mock.patch('psutil.Process', get_mock_process):
>>> +with nested(mock.patch('psutil.Process', get_mock_process),
>>> +mock.patch('psutil.cpu_count', return_value=1237)):
>>>hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask,
>>> worker_cpu_mask)
>>>self.assertEqual(mock_process.set_cpu_affinity.call_count, 7)
>>>self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
>>> -   mock.call(range(0,12)))
>>> +   mock.call(range(0,1237)))
>>>self.assertEqual(mock_process.set_cpu_affinity.call_args_list[6],
>>> mock.call([15, 16, 17]))
>>>
>>> --
>>> 2.8.1
>>>
>>>
>>
>

Re: [PATCH master 3/3] Fix KVM pinning tests to not depend on the local machine

['Iustin Pop' via ganeti-devel]

2016-06-10 10:58 GMT+02:00 Viktor Bachraty :

>
> On Thu, Jun 9, 2016 at 11:33 PM, Iustin Pop  wrote:
>
>> From: Iustin Pop 
>>
>> Commit 8b2ec2f added unittests for KVM pinning, but it introduced a
>> non-obvious
>> local dependency in the tests: the CPU_PINNING_OFF calls work by looking
>> at the
>> (current) machine's core count, and pinning to all those CPUs. In order
>> to make
>> this work independently from the test machine, we must also mock the
>> result of
>> process.cpu_count(). Do this by using a core count that is very much
>> unlikely
>> to ever be present in the real world.
>>
>> Signed-off-by: Iustin Pop 
>> ---
>>  test/py/ganeti.hypervisor.hv_kvm_unittest.py | 11 +++
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py b/test/py/
>> ganeti.hypervisor.hv_kvm_unittest.py
>> index c7a53b5..55ffb9b 100755
>> --- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
>> +++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
>> @@ -37,6 +37,7 @@ import socket
>>  import os
>>  import struct
>>  import re
>> +from contextlib import nested
>>
>>  from ganeti import serializer
>>  from ganeti import constants
>> @@ -636,12 +637,13 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
>>  cpu_mask = self.params['cpu_mask']
>>  worker_cpu_mask = self.params['worker_cpu_mask']
>>  hypervisor = hv_kvm.KVMHypervisor()
>> -with mock.patch('psutil.Process', return_value=mock_process):
>> +with nested(mock.patch('psutil.Process', return_value=mock_process),
>> +mock.patch('psutil.cpu_count', return_value=1237)):
>>
>
> 'nested' is deprecated in Python 2.7 but unfortunately 2.6 does not
> support comma separated contexts as 2.7 does.
>

Yep, saw that. However, nested still works (no warnings) in 2.6 as well, so
it should be good for now.


> All Debian versions newer than Squeeze should have 2.7 already. I believe
> once master goes stable, we can safely drop support for Python <2.7, what
>  do you think?
>

It is possible, but if we want to do that, I think we should do it across
the board. E.g. on Haskell side we have a number of workaround for old
compilers/library versions, and we could do some cleanup if we remove that,
but we don't need to hurry.

So declaring that some version (e.g. 2.19, or 2.20 - that is a round
number) is only supported on (Jessie+, or 2015+, or something) would be a
good move.

thanks,
iustin

   hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask,
>> worker_cpu_mask)
>>
>>  self.assertEqual(mock_process.set_cpu_affinity.call_count, 1)
>>  self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
>> - mock.call(range(0,12)))
>> + mock.call(range(0,1237)))
>>
>>def testCpuPinningPerVcpu(self):
>>  mock_process = mock.MagicMock()
>> @@ -659,11 +661,12 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
>>  def get_mock_process(unused_pid):
>>return mock_process
>>
>> -with mock.patch('psutil.Process', get_mock_process):
>> +with nested(mock.patch('psutil.Process', get_mock_process),
>> +mock.patch('psutil.cpu_count', return_value=1237)):
>>hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask,
>> worker_cpu_mask)
>>self.assertEqual(mock_process.set_cpu_affinity.call_count, 7)
>>self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
>> -   mock.call(range(0,12)))
>> +   mock.call(range(0,1237)))
>>self.assertEqual(mock_process.set_cpu_affinity.call_args_list[6],
>> mock.call([15, 16, 17]))
>>
>> --
>> 2.8.1
>>
>>
>

Re: [PATCH master 3/3] Fix KVM pinning tests to not depend on the local machine

['Viktor Bachraty' via ganeti-devel]

On Thu, Jun 9, 2016 at 11:33 PM, Iustin Pop  wrote:

> From: Iustin Pop 
>
> Commit 8b2ec2f added unittests for KVM pinning, but it introduced a
> non-obvious
> local dependency in the tests: the CPU_PINNING_OFF calls work by looking
> at the
> (current) machine's core count, and pinning to all those CPUs. In order to
> make
> this work independently from the test machine, we must also mock the
> result of
> process.cpu_count(). Do this by using a core count that is very much
> unlikely
> to ever be present in the real world.
>
> Signed-off-by: Iustin Pop 
> ---
>  test/py/ganeti.hypervisor.hv_kvm_unittest.py | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py b/test/py/
> ganeti.hypervisor.hv_kvm_unittest.py
> index c7a53b5..55ffb9b 100755
> --- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
> +++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
> @@ -37,6 +37,7 @@ import socket
>  import os
>  import struct
>  import re
> +from contextlib import nested
>
>  from ganeti import serializer
>  from ganeti import constants
> @@ -636,12 +637,13 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
>  cpu_mask = self.params['cpu_mask']
>  worker_cpu_mask = self.params['worker_cpu_mask']
>  hypervisor = hv_kvm.KVMHypervisor()
> -with mock.patch('psutil.Process', return_value=mock_process):
> +with nested(mock.patch('psutil.Process', return_value=mock_process),
> +mock.patch('psutil.cpu_count', return_value=1237)):
>

'nested' is deprecated in Python 2.7 but unfortunately 2.6 does not support
comma separated contexts as 2.7 does. All Debian versions newer than
Squeeze should have 2.7 already. I believe once master goes stable, we can
safely drop support for Python <2.7, what  do you think?

   hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask,
> worker_cpu_mask)
>
>  self.assertEqual(mock_process.set_cpu_affinity.call_count, 1)
>  self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
> - mock.call(range(0,12)))
> + mock.call(range(0,1237)))
>
>def testCpuPinningPerVcpu(self):
>  mock_process = mock.MagicMock()
> @@ -659,11 +661,12 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
>  def get_mock_process(unused_pid):
>return mock_process
>
> -with mock.patch('psutil.Process', get_mock_process):
> +with nested(mock.patch('psutil.Process', get_mock_process),
> +mock.patch('psutil.cpu_count', return_value=1237)):
>hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask,
> worker_cpu_mask)
>self.assertEqual(mock_process.set_cpu_affinity.call_count, 7)
>self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
> -   mock.call(range(0,12)))
> +   mock.call(range(0,1237)))
>self.assertEqual(mock_process.set_cpu_affinity.call_args_list[6],
> mock.call([15, 16, 17]))
>
> --
> 2.8.1
>
>

[PATCH master 3/3] Fix KVM pinning tests to not depend on the local machine

[Iustin Pop]

From: Iustin Pop 

Commit 8b2ec2f added unittests for KVM pinning, but it introduced a non-obvious
local dependency in the tests: the CPU_PINNING_OFF calls work by looking at the
(current) machine's core count, and pinning to all those CPUs. In order to make
this work independently from the test machine, we must also mock the result of
process.cpu_count(). Do this by using a core count that is very much unlikely
to ever be present in the real world.

Signed-off-by: Iustin Pop 
---
 test/py/ganeti.hypervisor.hv_kvm_unittest.py | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py 
b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
index c7a53b5..55ffb9b 100755
--- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
+++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
@@ -37,6 +37,7 @@ import socket
 import os
 import struct
 import re
+from contextlib import nested
 
 from ganeti import serializer
 from ganeti import constants
@@ -636,12 +637,13 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
 cpu_mask = self.params['cpu_mask']
 worker_cpu_mask = self.params['worker_cpu_mask']
 hypervisor = hv_kvm.KVMHypervisor()
-with mock.patch('psutil.Process', return_value=mock_process):
+with nested(mock.patch('psutil.Process', return_value=mock_process),
+mock.patch('psutil.cpu_count', return_value=1237)):
   hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask, 
worker_cpu_mask)
 
 self.assertEqual(mock_process.set_cpu_affinity.call_count, 1)
 self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
- mock.call(range(0,12)))
+ mock.call(range(0,1237)))
 
   def testCpuPinningPerVcpu(self):
 mock_process = mock.MagicMock()
@@ -659,11 +661,12 @@ class TestKvmCpuPinning(testutils.GanetiTestCase):
 def get_mock_process(unused_pid):
   return mock_process
 
-with mock.patch('psutil.Process', get_mock_process):
+with nested(mock.patch('psutil.Process', get_mock_process),
+mock.patch('psutil.cpu_count', return_value=1237)):
   hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask, 
worker_cpu_mask)
   self.assertEqual(mock_process.set_cpu_affinity.call_count, 7)
   self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0],
-   mock.call(range(0,12)))
+   mock.call(range(0,1237)))
   self.assertEqual(mock_process.set_cpu_affinity.call_args_list[6],
mock.call([15, 16, 17]))
 
-- 
2.8.1