Testing jobs can specify the VM base dynamically during deploy the test environment.

[gord1306]

Description of PR

Testing jobs can specify the VM base dyanmically during deploy the test environment.

1. add/remove topo - add a new argument to specify the VM Base nubmer
   The usage will be ./testbed-cli.sh -b VM0108 add-topo 1-1_t0 ~/.password​

2. deploy minigraph - fix to deploy correct VM METADAT in to minigraph when
   user specify vm_base dynamically. The vm_base can be assgined from 2 sources,
   one is read from testbed.csv file, another is from extra argument.
   assigned from testbed.csv file
   ansible-playbook -i lab config_sonic_basedon_testbed.yml -l as7816-64x -e testbed_name=1-1_t0 -e deploy=true -e save=true

   assigned from extra arguments
   ansible-playbook -i lab config_sonic_basedon_testbed.yml -l as7816-64x -e vm_base=VM0108 -e topo=t0 -e deploy=true -e save=true

Signed-off-by: Gord Chen [email protected]

Type of change

• Bug fix
• Testbed and Framework(new/improvement)
• Test case(new/improvement)

Approach

What is the motivation for this PR?

User can dynamically specify the VM base and make multiple users can perform testing on a single host concurrently

How did you do it?

How did you verify/test it?

add/remove topolgoy

Any platform specific information?

No

Supported testbed topology if it's a new test case?

All

Documentation

[yxieca]

@gord1306 can you elaborate the motivation of this change?

I feel that this change can leave the dut at uncertain state. e.g. minigraph and deployment don't match. And for test cases needs to access the VM, e.g. advanced reboot. They will likely find wrong VM and cause test to fail. Is there a particular reason you don't want to check in the testbed detail?

[gord1306]

@gord1306 can you elaborate the motivation of this change?

I feel that this change can leave the dut at uncertain state. e.g. minigraph and deployment don't match. And for test cases needs to access the VM, e.g. advanced reboot. They will likely find wrong VM and cause test to fail. Is there a particular reason you don't want to check in the testbed detail?

@gord1306 can you elaborate the motivation of this change?

I feel that this change can leave the dut at uncertain state. e.g. minigraph and deployment don't match. And for test cases needs to access the VM, e.g. advanced reboot. They will likely find wrong VM and cause test to fail. Is there a particular reason you don't want to check in the testbed detail?

@yxieca , for deployment, the options already exists in https://github.com/Azure/sonic-mgmt/blob/master/ansible/config_sonic_basedon_testbed.yml#L14 I just fix the yml to make it work.

And the motivation is to make the topology deployment more flexible on a single host, if the user wants to perform multiple testing jobs concurrently.

For the case advanced reboot, I am not clear what case you concerned. the deployment will write the VM IP address in the METADATA according to the selected VM Base, if the add topology/deployment use same VM base, it should have no match issue

[wangxin]

I don't understand why dynamically specifying vm_base is for concurrent deploy. If you need to concurrently deploy multiple topologies, then the VMs must not overlap. That means multiple test setups will co-exist on a same test server. Then, in testbed.csv, the vm_base must be correctly specified to avoid overlap in the first place. If vm_base is already correctly specified, why need to dynamically specify it again?

[wangxin]

Could you elaborate the purpose of moving platform.json to platform.json.bak?

[gord1306]

This is unnecessary modification, will remove it

[gord1306]

I don't understand why dynamically specifying vm_base is for concurrent deploy. If you need to concurrently deploy multiple topologies, then the VMs must not overlap. That means multiple test setups will co-exist on a same test server. Then, in testbed.csv, the vm_base must be correctly specified to avoid overlap in the first place. If vm_base is already correctly specified, why need to dynamically specify it again?

My description is not suitable before, it's not concurrency, this modification should be related to the VMs' utilization. It depend on different deployment scenario of the testbed. If the content of testbed.csv is not dynamically/frequently generated , that means the VM base are defined solid. I would like to maximum the VM utilization in this kind of case. If there are any VMs are free, they can be dynamically assigned to new testing job, and be not restrict by the testbed.csv.

[wangxin]

Do you mean testbed.csv is outdated? The vm_base information in testbed.csv is too old and inaccurate?
We have to manually check if VMs are free, right? I am not sure whether it is easy to do that. Assume we have a large pool of VMs, it is usually not straightforward to tell which VMs are used by which testbed. Maybe the better way is to code correct vm_base info in testbed.csv.

[gord1306]

Maybe not outdated, in our testing environment, the vm_base of testbed.csv more likes the default value. There is extra service (e.g. Jenkins) to arrange the VM resources for testing. We also check the bridges' interfaces to get the VMs' usages,
e.g. VM0212~VM0215 are allocated to the 2-7 testbed, the other VMs are free

sonic@sonic-server-02:~$ for i in $(sudo ovs-vsctl list-br | grep br-b); do echo ""; echo $i ; sudo ovs-vsctl list-ifaces $i ; done;
[sudo] password for sonic:

br-b-vms2-7
veth-bb-VM0212
veth-bb-VM0213
veth-bb-VM0214
veth-bb-VM0215

Therefore we add this extra argument and we can specify the VM base dynamically when perform the testing

[wangxin]

The updated file config_sonic_basedon_testbed.yml is called by testbed-cli.sh in generate_minigraph, deploy_minigraph and test_minigraph. Let's take generate_minigraph as an example:

function generate_minigraph
{
  topology=$1
  inventory=$2
  passfile=$3
  shift
  shift
  shift

  echo "Generating minigraph '$topology'"

  read_file $topology

  ansible-playbook -i "$inventory" config_sonic_basedon_testbed.yml --vault-password-file="$passfile" -l "$duts" -e testbed_name="$topology" -e testbed_file=$tbfile -e vm_file=$vmfile -e local_minigraph=true $@

  echo Done
}

According to the ansible-playbook command line, variables vm_base and topo are not passed to the playbook. I suspect that in this playbook, variables vm_base and topo are undefined.