Skip to content

Generate minigraph with default ansible_hostv6#1983

Merged
yxieca merged 1 commit intosonic-net:masterfrom
NazarTkachuk:fix_ansible_hostv6
Aug 1, 2020
Merged

Generate minigraph with default ansible_hostv6#1983
yxieca merged 1 commit intosonic-net:masterfrom
NazarTkachuk:fix_ansible_hostv6

Conversation

@NazarTkachuk
Copy link
Contributor

Generate minigraph with default ansible_hostv6 as it was before PR #1851

Signed-off-by: Nazar Tkachuk nazarx.tkachuk@intel.com

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Approach

What is the motivation for this PR?

Define default value for ansible_hostv6 if it is not defined

How did you do it?

How did you verify/test it?

Run ./testbed-cli.sh deploy-mg ${SETUP} lab ./password.txt

Any platform specific information?

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

Documentation

…t ansible_hostv6

Signed-off-by: Nazar Tkachuk <nazarx.tkachuk@intel.com>
@NazarTkachuk
Copy link
Contributor Author

<b:IPPrefix>{{ ansible_hostv6 if ansible_hostv6 is defined else 'FC00:2::32' }}/64</b:IPPrefix>
</a:Prefix>
<a:PrefixStr>{{ ansible_hostv6 }}/64</a:PrefixStr>
<a:PrefixStr>{{ ansible_hostv6 if ansible_hostv6 is defined else 'FC00:2::32' }}/64</a:PrefixStr>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to be aware that if ansible_hostv6 is not defined, then tacacs v6 test will fail?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SuvarnaMeenakshi , can you answer this question? If yes, then how do we know whether we should run tacacs v6 test case or not?

Copy link
Contributor

@msosyak msosyak Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lguohan As I understand from the "How did you verify/test it?" section the issue is actually during deploy-mg command. The template failed if ansible_hostv6 is not defined for the testbed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we check if ptf has IPv6 address assigned or not. If ptf does not have IPv6 address assigned, then tacacs ipv6 test case will be skipped.
If ptf has IPv6 assigned and minigraph does not have IPv6, tacacs ipv6 will be executed, and it will fail.
Should we have additional check to see if both ptf and DUT has IPv6 configured, only then execute tacacs ipv6 test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for my 2 cents, I suppose we need to have backward compatibility, run deploy-mg with/without ansible_hostv6 definition

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NazarTkachuk , I am trying understand what else changes we need so that we are not breaking the tacacs v6 test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SuvarnaMeenakshi , in this case, the dut will have ipv6 address. How do you check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lguohan, that is right, we will have IPv6 address in this case, can think of 2 options: 1. Expect that if PTF docker has IPv6 address, then the DUT has IPv6 address reachable from ptf docker – If that is the case, then current implementation is good.
2. Specifically check for the default IPv6 address given to DUT here, if it is the default address, then skip IPv6 tacacs test. This does not seem clean as it is specific check for address: 'FC00:2::32'.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we change the default v6 address on DUT a IPv6 address that is reachable to the ptf docker?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lguohan We could do that for VS testbed, if ptf-docker IP address is taken from a specific lab network, then we cannot generate a default reachable IPv6 address for DUT.

@yxieca yxieca merged commit 2921e1f into sonic-net:master Aug 1, 2020
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
4236bc4 [config reload] Fixing config reload when timer based delayed services are disabled (sonic-net#1967)
d2514e4 [GCU] Different apply-patch runs should produce same sorted steps (sonic-net#1988)
2878adb [GCU] Using simulated config instead of target config when validating replace operation in NoDependencyMoveValidator (sonic-net#1987)
fb8ca98 [GCU] Loading yang-models only once (sonic-net#1981)
f88ee92 [GCU] Copying config_db before callding sonic_yang.loadData (sonic-net#1983)
9ed0e91 [GCU] Implementing DryRun by printing patch-sorter steps/imitating config_db (sonic-net#1973)
b36b5e3 [GCU] Moving PatchSorter unit-test to json file to make it easier to read/maintain (sonic-net#1977)
c0fa28b [generic-config-updater] Improving CreateOnly validator and marking /LOOPBACK_INTERFACE/LOOPBACK#/vrf_name as create-only (sonic-net#1969)
0559d04 [generic-config-updater] Adding non-strict mode (sonic-net#1929)
b07f477 [debug dump util] FDB debug dump util changes (sonic-net#1968)
6d8757a [warm/fast-reboot] Fix kexec portion to support platforms based on Device Tree (sonic-net#1966)
cc1409e [Auto Techsupport] Event driven Techsupport Bug Fixes (sonic-net#1986)
6c48bd5 Fix wrong help message for cable length setting (sonic-net#1978)
c0bbbe3 [breakout] Fix the check  when port is not present in BREAKOUT_CFG table (sonic-net#1765)
5bb8cad [doc][DPB] Update DPB related interface breakout command Info (sonic-net#1438)
e6fd990 [config] Fix 'config reload -l' command to get filename by default (sonic-net#1611)
bd8f7bb Update swss_ready check to check per namespace swss service (sonic-net#1974)
5439f94 [soft-reboot] Add support for platforms based on Device Tree (sonic-net#1963)
7c5810a [config] Add portchannel support  for static route  (sonic-net#1857)
7cb6a1b preserve old order for config reload (sonic-net#1964)
20bddbd [Auto-Techsupport] Issues related to Multiple Cores crashing handled (sonic-net#1948)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants