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.

Sign up for GitHub

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

[teamd]: Clean teamd process if LAG creation fails #2888

Merged
merged 9 commits into from
Sep 13, 2023
Merged

[teamd]: Clean teamd process if LAG creation fails #2888

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9c6877f
[teamd]: Clean teamd process if LAG creation fails
theasianpianist Aug 24, 2023
111d91a
Merge branch 'master' into teammgrd-cleanup
theasianpianist Aug 24, 2023
706b5e6
[tests]: Add teammgrd unit test
theasianpianist Aug 25, 2023
4bb1aba
[teammgrd]: Fix unit test setup
theasianpianist Aug 25, 2023
fb5176a
[teammgr]: remove extra spaces from cli command
theasianpianist Aug 26, 2023
2072d0c
[teammgr]: Fix removeLag
theasianpianist Aug 26, 2023
5c88f99
update gitignore
theasianpianist Aug 26, 2023
fc0582d
update test expected commands
theasianpianist Aug 26, 2023
87225ef
Merge branch 'master' into teammgrd-cleanup
theasianpianist Sep 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions cfgmgr/teammgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,9 +658,6 @@ bool TeamMgr::removeLag(const string &alias)
string res;
pid_t pid;

cmd << TEAMD_CMD << " -k -t " << shellquote(alias);
EXEC_WITH_ERROR_THROW(cmd.str(), res);

Copy link

Choose a reason for hiding this comment

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

FWIW, can I suggest to the following:

bool TeamMgr::removeLag(const string &alias, bool assertSuccess)
{
...
if (asserSuccess) {
    EXEC_WITH_ERROR_THROW(cmd.str(), res);
} else {
    exec( cmd.str(), res );
}
   ...
}

In the case when the addLag() fails, removeLag() will be called with assertSuccess=false, otherwise the code path stays the same. IMO this would address the issue while still keeping the addLag() and removeLag() symmetrical... my 2cents.

try
{
std::stringstream cmd;
Expand Down