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

[feature]: add dummy network interface for yurthub #289

Merged
merged 1 commit into from
May 6, 2021

Conversation

rambohe-ch
Copy link
Member

@rambohe-ch rambohe-ch commented May 6, 2021

Ⅰ. Describe what this PR does

  • background:
    Yurthub is a node daemon that serves as a proxy for the outbound traffic from the Kubernetes node daemons(like kubelet, kube-proxy and so on), and yurthub is listening on 127.0.0.1:10261 for proxying outbound traffic. so components with hostNetwork=false can not use yurthub to proxy outbound request because of traffic from container network namespace to 127.0.0.1 can not route on edge node.

  • solution:
    Add a dummy network interface on edge node by yurthub, and yurthub listens on new dummy interface, so components in its own network namespace can access yurthub through dummy interface.
    the default ip address of new dummy interface is: 169.254.2.1, the interface name is: yurthub-dummy0

  • usage for components on edge node:
    to access kube-apiserver through yurthub for components, at first make sure that components can support to set yurthub address instead of kube-apiserver address. and the address of yurthub as following:

    1. yurthub address for components on host network: http://127.0.0.1:10261
    2. yurthub address for components on dependent network namespace: http://169.254.2.1:10261

Ⅱ. Does this pull request fix one issue?

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅳ. Describe how to verify it

make all
and make sure yurthub can work correctly.

Ⅴ. Special notes for reviews

@rambohe-ch rambohe-ch requested review from Fei-Guo and zyjhtangtang May 6, 2021 03:03
@rambohe-ch rambohe-ch force-pushed the add-dummy-interface branch 2 times, most recently from 449ec78 to e1466a6 Compare May 6, 2021 03:40
@zyjhtangtang
Copy link
Contributor

lgtm

@charleszheng44
Copy link
Member

Just have a question, on the edge node, except kubelet, which component will need to access yurthub?

@rambohe-ch
Copy link
Member Author

Just have a question, on the edge node, except kubelet, which component will need to access yurthub?

any components(like kubelet, kube-proxy, flannel and so on) that want to use yurthub cache when cloud-edge network disconnected.

type unsupportedInterfaceController struct {
}

func NewDummyInterfaceController() DummyInterfaceController {
Copy link
Member

Choose a reason for hiding this comment

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

Should NewDummyInterfaceController be NewUnsupportedDummyInterfaceController?

Where is the unsupportedDummyController used?

Copy link
Member Author

Choose a reason for hiding this comment

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

when os is not linux, unsupportedDummyController will be used by NetworkManager, and when os is linux, dummyif_linux.go will be used.


klog.Infof("ip address for %s interface changed to %s", ifName, ifIP.String())
return dic.AddrReplace(link, &netlink.Addr{IPNet: netlink.NewIPNet(ifIP)})
}
Copy link
Member

Choose a reason for hiding this comment

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

if err != nil, why not returning err but calling addDummyInterface? Maybe a comment is needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, add error handle here, if err is Link not found , we will call addDummyInterface, otherwise return err.

networkMgr.Run(stopCh)
trace++

klog.Infof("%d. new %s server and begin to serve, proxy server: %s;%s, hub server: %s", trace,
Copy link
Member

Choose a reason for hiding this comment

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

You should add "dummy proxy server" before "%s" in the log line.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@@ -157,10 +158,28 @@ func Run(cfg *config.YurtHubConfiguration, stopCh <-chan struct{}) error {
}
trace++

klog.Infof("%d. new %s server and begin to serve, proxy server: %s, hub server: %s", trace, projectinfo.GetHubName(), cfg.YurtHubProxyServerAddr, cfg.YurtHubServerAddr)
s := server.NewYurtHubServer(cfg, certManager, yurtProxyHandler)
klog.Infof("%d. create dummy network interface %s and init iptables manager", trace, cfg.HubAgentDummyIfName)
Copy link
Member

Choose a reason for hiding this comment

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

I think the entire change in this file should be guarded by "if cfg.EnableDummyIf ". This change should introduce zero overhead if people does not need/disable this feature.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@rambohe-ch rambohe-ch force-pushed the add-dummy-interface branch from e1466a6 to da97b02 Compare May 6, 2021 07:17
Copy link
Member

@Fei-Guo Fei-Guo left a comment

Choose a reason for hiding this comment

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

LGTM

@Fei-Guo Fei-Guo merged commit 84d3fe8 into openyurtio:master May 6, 2021
@felix0080
Copy link
Member

@rambohe-ch hello rambohe, I'd like to consult you about.. What does iptable do in your commit file

@rambohe-ch rambohe-ch deleted the add-dummy-interface branch May 27, 2021 06:26
MrGirl pushed a commit to MrGirl/openyurt that referenced this pull request Mar 29, 2022
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