Skip to content

SDN options and CNI directory cleanup #18404

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

Merged
merged 2 commits into from
Feb 21, 2018
Merged

SDN options and CNI directory cleanup #18404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed29904
node: remove un-needed kubelet flags
dcbw Feb 1, 2018
10c212d
sdn: rationalize data directories between kubelet, CNI, and SDN
dcbw Feb 2, 2018
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
7 changes: 7 additions & 0 deletions pkg/cmd/server/kubernetes/network/sdn_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"k8s.io/client-go/tools/record"
kinternalclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
kcni "k8s.io/kubernetes/pkg/kubelet/network/cni"
"k8s.io/kubernetes/pkg/proxy/apis/kubeproxyconfig"

configapi "github.com/openshift/origin/pkg/cmd/server/api"
Expand All @@ -35,6 +36,11 @@ func NewSDNInterfaces(options configapi.NodeConfig, networkClient networkclient.
}
}

cniConfDir := kcni.DefaultNetDir
if val, ok := options.KubeletArguments["cni-conf-dir"]; ok && len(val) == 1 {
cniConfDir = val[0]
}

// dockershim + kube CNI driver delegates hostport handling to plugins,
// while CRI-O handles hostports itself. Thus we need to disable the
// SDN's hostport handling when run under CRI-O.
Expand All @@ -49,6 +55,7 @@ func NewSDNInterfaces(options configapi.NodeConfig, networkClient networkclient.
Hostname: options.NodeName,
SelfIP: options.NodeIP,
RuntimeEndpoint: runtimeEndpoint,
CNIConfDir: cniConfDir,
MTU: options.NetworkConfig.MTU,
NetworkClient: networkClient,
KClient: kubeClient,
Expand Down
4 changes: 0 additions & 4 deletions pkg/cmd/server/kubernetes/node/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
utilfeature "k8s.io/apiserver/pkg/util/feature"
kclientsetexternal "k8s.io/client-go/kubernetes"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig"
kubeletcni "k8s.io/kubernetes/pkg/kubelet/network/cni"
kubelettypes "k8s.io/kubernetes/pkg/kubelet/types"

Expand Down Expand Up @@ -88,9 +87,6 @@ func ComputeKubeletFlagsAsMap(startingArgs map[string][]string, options configap
if network.IsOpenShiftNetworkPlugin(options.NetworkConfig.NetworkPluginName) {
// SDN plugin pod setup/teardown is implemented as a CNI plugin
setIfUnset(args, "network-plugin", kubeletcni.CNIPluginName)
setIfUnset(args, "cni-conf-dir", kubeletcni.DefaultNetDir)
setIfUnset(args, "cni-bin-dir", kubeletcni.DefaultCNIDir)
setIfUnset(args, "hairpin-mode", kubeletconfig.HairpinNone)
} else {
setIfUnset(args, "network-plugin", options.NetworkConfig.NetworkPluginName)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/node/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func updateARPMetrics() {

func updatePodIPMetrics() {
numAddrs := 0
items, err := ioutil.ReadDir("/var/lib/cni/networks/openshift-sdn/")
items, err := ioutil.ReadDir(hostLocalDataDir + "/networks/openshift-sdn/")
if err != nil && os.IsNotExist(err) {
// Don't log an error if the directory doesn't exist (eg, no pods started yet)
return
Expand Down
11 changes: 7 additions & 4 deletions pkg/network/node/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ import (
)

const (
cniDirPath = "/etc/cni/net.d"
openshiftCNIFile = "80-openshift-network.conf"
hostLocalDataDir = "/var/lib/cni"
)

type osdnPolicy interface {
Expand All @@ -75,6 +75,7 @@ type OsdnNodeConfig struct {
RuntimeEndpoint string
MTU uint32
EnableHostports bool
CNIConfDir string

NetworkClient networkclient.Interface
KClient kclientset.Interface
Expand Down Expand Up @@ -102,6 +103,7 @@ type OsdnNode struct {
useConnTrack bool
iptablesSyncPeriod time.Duration
mtu uint32
cniDirPath string

// Synchronizes operations on egressPolicies
egressPoliciesLock sync.Mutex
Expand Down Expand Up @@ -154,7 +156,7 @@ func New(c *OsdnNodeConfig) (network.NodeInterface, error) {

// If our CNI config file exists, remove it so that kubelet doesn't think
// we're ready yet
os.Remove(filepath.Join(cniDirPath, openshiftCNIFile))
os.Remove(filepath.Join(c.CNIConfDir, openshiftCNIFile))

if err := c.setNodeIP(); err != nil {
return nil, err
Expand Down Expand Up @@ -184,6 +186,7 @@ func New(c *OsdnNodeConfig) (network.NodeInterface, error) {
kubeInformers: c.KubeInformers,
networkInformers: c.NetworkInformers,
egressIP: newEgressIPWatcher(oc, c.SelfIP, c.MasqueradeBit),
cniDirPath: c.CNIConfDir,

runtimeEndpoint: c.RuntimeEndpoint,
// 2 minutes is the current default value used in kubelet
Expand Down Expand Up @@ -380,7 +383,7 @@ func (node *OsdnNode) Start() error {
}
}

if err := os.MkdirAll(cniDirPath, 0755); err != nil {
if err := os.MkdirAll(node.cniDirPath, 0755); err != nil {
return err
}

Expand All @@ -396,7 +399,7 @@ func (node *OsdnNode) Start() error {

// Write our CNI config file out to disk to signal to kubelet that
// our network plugin is ready
return ioutil.WriteFile(filepath.Join(cniDirPath, openshiftCNIFile), []byte(`
return ioutil.WriteFile(filepath.Join(node.cniDirPath, openshiftCNIFile), []byte(`
{
"cniVersion": "0.2.0",
"name": "openshift-sdn",
Expand Down
18 changes: 11 additions & 7 deletions pkg/network/node/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
kcontainer "k8s.io/kubernetes/pkg/kubelet/container"
knetwork "k8s.io/kubernetes/pkg/kubelet/network"
kcni "k8s.io/kubernetes/pkg/kubelet/network/cni"
kubehostport "k8s.io/kubernetes/pkg/kubelet/network/hostport"
kbandwidth "k8s.io/kubernetes/pkg/util/bandwidth"
utildbus "k8s.io/kubernetes/pkg/util/dbus"
Expand All @@ -42,6 +43,7 @@ import (

const (
podInterfaceName = knetwork.DefaultInterfaceName
cniBinPath = kcni.DefaultCNIDir
Copy link

Choose a reason for hiding this comment

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

As a follow up, we could add a validation to check kubeletArguments["cni-bin-path"] is not set when openshift network plugin is used.

Copy link

Choose a reason for hiding this comment

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

Addressed in #18464

)

type podHandler interface {
Expand Down Expand Up @@ -114,9 +116,10 @@ func getIPAMConfig(clusterNetworks []common.ClusterNetwork, localSubnet string)
}

type hostLocalIPAM struct {
Type string `json:"type"`
Subnet cnitypes.IPNet `json:"subnet"`
Routes []cnitypes.Route `json:"routes"`
Type string `json:"type"`
Subnet cnitypes.IPNet `json:"subnet"`
Routes []cnitypes.Route `json:"routes"`
DataDir string `json:"dataDir"`
}

type cniNetworkConfig struct {
Expand Down Expand Up @@ -153,7 +156,8 @@ func getIPAMConfig(clusterNetworks []common.ClusterNetwork, localSubnet string)
Name: "openshift-sdn",
Type: "openshift-sdn",
IPAM: &hostLocalIPAM{
Type: "host-local",
Type: "host-local",
DataDir: hostLocalDataDir,
Copy link

Choose a reason for hiding this comment

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

Where is hostLocalIPAM.DataDir used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pravisankar it's serialized into JSON and passed to the host-local plugin, which then puts its IPAM data there instead of the default. In reality hostLocalDataDir is already the same as the host-local plugin default so no change. But the path will be consistent now.

Subnet: cnitypes.IPNet{
IP: nodeNet.IP,
Mask: nodeNet.Mask,
Expand Down Expand Up @@ -410,7 +414,7 @@ func createIPAMArgs(netnsPath string, action cniserver.CNICommand, id string) *i
ContainerID: id,
NetNS: netnsPath,
IfName: podInterfaceName,
Path: "/opt/cni/bin",
Path: cniBinPath,
}
}

Expand All @@ -421,7 +425,7 @@ func (m *podManager) ipamAdd(netnsPath string, id string) (*cni020.Result, net.I
}

args := createIPAMArgs(netnsPath, cniserver.CNI_ADD, id)
r, err := invoke.ExecPluginWithResult("/opt/cni/bin/host-local", m.ipamConfig, args)
r, err := invoke.ExecPluginWithResult(cniBinPath+"/host-local", m.ipamConfig, args)
if err != nil {
return nil, nil, fmt.Errorf("failed to run CNI IPAM ADD: %v", err)
}
Expand All @@ -441,7 +445,7 @@ func (m *podManager) ipamAdd(netnsPath string, id string) (*cni020.Result, net.I
// Run CNI IPAM release for the container
func (m *podManager) ipamDel(id string) error {
args := createIPAMArgs("", cniserver.CNI_DEL, id)
err := invoke.ExecPluginWithoutResult("/opt/cni/bin/host-local", m.ipamConfig, args)
err := invoke.ExecPluginWithoutResult(cniBinPath+"/host-local", m.ipamConfig, args)
if err != nil {
return fmt.Errorf("failed to run CNI IPAM DEL: %v", err)
}
Expand Down