Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define SAI_HEADER_DIR for saithrift compilation to avoid headers installation #1723

Merged
merged 1 commit into from
May 18, 2018

Conversation

NStetskovych-zz
Copy link
Contributor

@NStetskovych-zz NStetskovych-zz commented May 17, 2018

Signed-off-by: Nadiya Stetskovych [email protected]

- What I did
Define SAI_HEADER_DIR for saithrift
to build thrift for the currently used SAI version
https://github.com/opencomputeproject/SAI/blob/949cec8a15e926239e78c92a8e57c49c9f0e3874/test/saithrift/Makefile#L3
- How I did it
export variable to a sub-make
- How to verify it
build libsaithrift-dev_1.2.1_amd64.deb without SAI headers prior installed
- Description for the changelog
define sai header dir
- A picture of a cute animal (not mandatory but encouraged)
./.../
(.'•..•'.)
=*=

@pavel-shirshov pavel-shirshov merged this pull request into sonic-net:201803 May 18, 2018
qiluo-msft added a commit that referenced this pull request May 20, 2021
…7647)

Includes below commits
```
sonic-swss
05f0438 2021-05-19 | Fix build error (#1751) [Shi Su]
1682a8d 2021-05-12 | [Bulk mode] Improve log in bulk failures (#1740) [Shi Su]
4509e88 2021-04-27 | [routeorch] Add support for blackhole routes (#1723) [Shi Su]
642014d 2019-10-31 | [fix] Use the same storm detection condition for queue occupancy non-zero case as the zero case (#1111) [Wenda Ni]

sonic-utilities
97fc442 2021-05-10 | [config]: Use mod_entry when editing VLAN_INTERFACE (#1602) [Lawrence Lee]
f60d579 2021-04-15 | [techsupport] Update show ip interface command (#1562) [Mahesh Maddikayala]
821d29d 2021-04-14 | Fix Multi-ASIC show specific resursive route by using common parsing function (#1560) [gechiang]

sonic-linux-kernel
61b331f 2021-05-06 | [dps200] Add dps200 PSU module driver (#207) [schobtr]

sonic-platform-daemons
5d2960f 2021-05-17 | [xcvrd] add support for logging mux_metrics events into state DB (#185) [vdahiya12]
a5c9207 2021-05-11 | [thermalctld] Enable stopping thermal manager (#180) [Junchao-Mellanox]
9123f63 2021-05-10 | [muxcable] Remove Xcvrd Sleep (#174) [Tamer Ahmed]
```
qiluo-msft pushed a commit that referenced this pull request Sep 28, 2021
0d538d3 [ci]: Support code diff coverage (#1834)
48887d1 [config] support for configuring muxcable to standby mode of operation (#1837)
2088a9a Provide support to install platform extensions (#1578)
c97fe54 Add check_db_integrity script to setup.py (#1828)
c0b9917 [debug dump util] COPP Module Added (#1670)
826311c [techsupport] Removed interactive option for docker commands and Improved Error Reporting (#1723)
ce11545 [config reload] Removed job-mode for sonic.target restart (#1820)
f76f672 [fdbshow]: Fix typo in comment (#1809)
17208a0 [ci]: Support PR coverage (#1806)
c2c2354 fix wrong code indent in sfputil (#1808)
47a9a0f [portconfig] Validate duplicate speed value and interface type value (#1745)
f1086ee [sonic_installer]Add --skip-platform-check option for sonic_installer when image mismatch (#1791)
c007d65 [warm-reboot] Add new preboot health check: verify database integrity (#1785)
41e31e8 Fix PatchApplier init order (#1762)
2416175 [config reload] Fix config reload failure due to sonic.target job cancellation (#1814)
2b12aad [portstat, intfstat] added rates and utilization (#1750)
26e700a [debug dump util] Techsupport addition (#1669)
9f2326e [debug dump util] Base Skeleton and Click Class added (#1668)
theasianpianist pushed a commit to theasianpianist/sonic-buildimage that referenced this pull request Feb 5, 2022
Allow route orch to add/remove routes blackhole routes and add dvs tests.
taras-keryk pushed a commit to taras-keryk/sonic-buildimage that referenced this pull request Apr 28, 2022
…oved Error Reporting (sonic-net#1723)

#### Why I did

Recently, a bug was seen which was related to saisdkdump and particularly showed up when `show techsupport` was invoked. Although, it was fixed, the sonic-mgmt test failed to capture it beforehand. 

This highlighted a few shortcomings of the `generate_dump` script and this PR addresses those and also a few additional issues seen

This PR fixes a few things, I'll explain each of them in the next section.

#### What I did

**1) Remove the "Interactive option (-i) for the docker invocation commands"**
  This was the reason why the bug which was  was not captured previously. When the techsupport was invoked remotely (Eg: using sshpass), the `docker exec -it  <docker> <cmd>` command would fail saying ` ‘the input device is not a TTY'`. Hence the (-i) option was removed.

**2) Change the Return Code**
Currently, the script doesn't return any non-zero error codes for most of the intermediate steps (even though they fail), which makes validation hard.

To handle this, a helper function and trap cmd are used. 
```
handle_error() {
  if [ "$1" != "0" ]; then
    echo "ERR: RC:-$1 observed on line $2" >&2
    RETURN_CODE=1
  fi
}
trap 'handle_error $? $LINENO' ERR # This would trap any executions with non-zero return codes
```
The global variable RETURN_CODE is set when this is called and the same RETURN_CODE is returned when `generate_dump` invocation process exits

You may see this is used in multiple functions instead of placing it once on the top of the script. This is because, every function can itself be considered as a subshell and each of them requires a explicit trap command.

When a command is failed with error, this logic would get append a corresponding log to stderr. 
`ERR: RC:-1 observed on line 729`

**3) Improve Error Reporting for save_cmd function**

Currently any error written to the stderr by the intermediate calls are redirected to the same location as stdout, which is usually the file we see under dump/ dir.  This is perfectly fine, but the sonic-mgmt test only parses the text seen in stdout. 

So, a new option (-r) is added to `generate_dump` script and subsequently to `show techsupport` to redirect any intermediate errors seen to the generate_dump's stderr.

With this option enabled, these sort of errors can be captured on the stderr.

```
root@sonic:/home/admin# show techsupport -r
..........
timeout --foreground 5m show queue counters > /var/dump/sonic_dump_r-tigon-04_20210714_062239/dump/queue.counters_1
Traceback (most recent call last):
  File "/usr/local/bin/queuestat", line 373, in <module>
    main()
  File "/usr/local/bin/queuestat", line 368, in main
    queuestat.get_print_all_stat(json_opt)
  File "/usr/local/bin/queuestat", line 239, in get_print_all_stat
    cnstat_dict = self.get_cnstat(self.port_queues_map[port])
  File "/usr/local/bin/queuestat", line 168, in get_cnstat
    cnstat_dict[queue] = get_counters(queue_map[queue])
  File "/usr/local/bin/queuestat", line 158, in get_counters
    fields[pos] = str(int(counter_data))
ValueError: invalid literal for int() with base 10: ''
handle_error $? $LINENO
ERR: RC:-1 observed on line 199
Command: show queue counters timedout after 5 minutes.
.............

Without that option, this'll be the output seen

root@sonic:/home/admin# show techsupport 
..........
timeout --foreground 5m show queue counters &> /var/dump/sonic_dump_r-tigon-04_20210714_062239/dump/queue.counters_1
handle_error $? $LINENO
ERR: RC:-1 observed on line 199
Command: show queue counters timedout after 5 minutes.
.............
```

**4) Minor Error in sdk-dump collection logic handled**
save_file is only called for the files seen in sdk_dump_path and not for directories
```
cp: -r not specified; omitting directory '/tmp/sdk-dumps'
handle_error $? $LINENO
ERR: RC:-1 observed on line 729
tar: sonic_dump_r-tigon-04_20210714_062239/sai_sdk_dump/sdk-dumps: Cannot stat: No such file or directory
tar: Exiting with failure status due to previous errors
tar append operation failed. Aborting to prevent data loss.
```
The reason being, `find /tmp/sdk-dumps` returns ["/tmp/sdk-dumps"] even if the dir is empty. In the next step, save_file cmd is applied on the folder and thus the error. This can be handled by the change specified above

**5) Minor Error in custom plugins logic handled**
Added a condition to check if the dir exists before proceeding forward.
```
if [[ -d ${PLUGINS_DIR} ]]; then
        local -r dump_plugins="$(find ${PLUGINS_DIR} -type f -executable)"
        for plugin in $dump_plugins; do
            # save stdout output of plugin and gzip it
            save_cmd "$plugin" "$(basename $plugin)" true false
        done
    fi
```
Otherwise,  find command might fail saying
```
root@sonic:/home/admin# find /usr/local/bin/debug-dump -type f -executable
find: ‘/usr/local/bin/debug-dump’: No such file or directory
```
taras-keryk pushed a commit to taras-keryk/sonic-buildimage that referenced this pull request Apr 28, 2022
…#1844)

What I did
Fix: sonic-net#8850

Issue was introduced by sonic-net#1723, sonic-net#1833, and sonic-net#1843 (pending merge)

The error_handler is a great idea but the bash script needs to be error free first.

How I did it
Fix bash script errors.

How to verify it
run show techsupport test..

Signed-off-by: Ying Xie <[email protected]>
taras-keryk pushed a commit to taras-keryk/sonic-buildimage that referenced this pull request Apr 28, 2022
- What I did
This PR include some fixes which were missed for sonic-net#1723
i.e. removing -t option from the docker exec commands. to understand why the -it option was removed, refer sonic-net#1723.
Also, the show techsupport exits with $RETURN_CODE only when --redirect-stderr option is used.

Signed-off-by: Vivek Reddy Karri <[email protected]>
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.

2 participants