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

[qfix] Save generated kernel interface name #100

Merged

Conversation

Bolodya1997
Copy link

No description provided.

Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@Bolodya1997 Bolodya1997 force-pushed the qfix/kernel-if-name branch from a54d6eb to 2efa62a Compare June 9, 2021 07:24
@Bolodya1997 Bolodya1997 marked this pull request as ready for review June 9, 2021 08:06
@denis-tingaikin denis-tingaikin merged commit aa4a0cc into networkservicemesh:main Jun 9, 2021
@@ -100,7 +100,7 @@ func (m *Mechanism) GetInterfaceName(conn *networkservice.Connection) string {
if len(name) > LinuxIfMaxLength {
name = name[:LinuxIfMaxLength]
}
return name
m.GetParameters()[InterfaceNameKey] = name
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Member

Choose a reason for hiding this comment

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

Vlad found that in case of healing, we could re-generate the name for the interface and add it again.

@Bolodya1997 Could you explain it in detail?

Copy link
Author

Choose a reason for hiding this comment

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

probably it is not fully correct to name the case I have tested as NSMgr restart, it is most actually a NSMgr is getting replaced with another NSMgr with the same unix/IP addresses - so the difference between is that another NSMgr has another name
in production we should get this case when we use rolling update on the NSMgr daemonset template:

  • update NSMgr version
  • change NSMgr envs
    so it is not actually a restart, but it also should be healed as a restore

in such case new NSMgr overwrites old NSMgr path segments, so we have:

NSC -> NSMgr(1) -> Forwarder -> NSMgr(2) -> NSE

turning to

NSC -> new NSMgr(1) -> Forwarder -> new NSMgr(2) -> NSE

so when Forwarder starts trying to use new NSMgr(2) instead of NSMgr(2) for generating the link name, it generates a new name and receives a Link not found error and so fails a restore

Copy link
Member

Choose a reason for hiding this comment

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

@edwarnicke Could you share your thoughts on why previously we do not store generated name in the parameters?

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.

3 participants