Skip to content

Commit

Permalink
Merge pull request #15781 from danwinship/split-master-node
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 15862, 15781, 15944)

Split up SDN master/node/proxy/CNI code

Right now `pkg/sdn/plugin` is a mix of code that runs on the master and code that runs on nodes. This splits things up. I think it makes sense? @openshift/networking what do you think?

This gets rid of the `pkg/sdn/plugin` directory completely; node-specific code is now in `pkg/sdn/node` because it doesn't actually use the network plugin API at all any more.
  • Loading branch information
openshift-merge-robot committed Aug 30, 2017
2 parents 4a85dec + 4570e27 commit 558cdda
Show file tree
Hide file tree
Showing 47 changed files with 212 additions and 182 deletions.
2 changes: 1 addition & 1 deletion hack/lib/build/constants.sh
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ readonly OS_OUTPUT_PKGDIR="${OS_OUTPUT}/pkgdir"
readonly OS_GO_PACKAGE=github.com/openshift/origin

readonly OS_SDN_COMPILE_TARGETS_LINUX=(
pkg/sdn/plugin/sdn-cni-plugin
pkg/sdn/sdn-cni-plugin
vendor/github.com/containernetworking/cni/plugins/ipam/host-local
vendor/github.com/containernetworking/cni/plugins/main/loopback
)
Expand Down
8 changes: 4 additions & 4 deletions pkg/cmd/server/kubernetes/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,12 +301,12 @@ func SetFakeContainerManagerInterfaceForIntegrationTest() {
defaultContainerManagerInterface = cm.NewStubContainerManager()
}

// RunPlugin starts the local SDN plugin, if enabled in configuration.
func (c *NodeConfig) RunPlugin() {
if c.SDNPlugin == nil {
// RunSDN starts the SDN, if the OpenShift SDN network plugin is enabled in configuration.
func (c *NodeConfig) RunSDN() {
if c.SDNNode == nil {
return
}
if err := c.SDNPlugin.Start(); err != nil {
if err := c.SDNNode.Start(); err != nil {
glog.Fatalf("error: SDN node startup failed: %v", err)
}
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/cmd/server/kubernetes/node/node_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ type NodeConfig struct {
// DNSConfig controls the DNS configuration.
DNSServer *dns.Server

// SDNPlugin is an optional SDN plugin
SDNPlugin sdn.NodeInterface
// SDNNode is an optional SDN node interface
SDNNode sdn.NodeInterface
// SDNProxy is an optional service endpoints filterer
SDNProxy sdn.ProxyInterface
}
Expand Down Expand Up @@ -234,10 +234,10 @@ func BuildKubernetesNodeConfig(options configapi.NodeConfig, enableProxy, enable
internalKubeInformers := kinternalinformers.NewSharedInformerFactory(kubeClient, proxyconfig.ConfigSyncPeriod.Duration)

// Initialize SDN before building kubelet config so it can modify option
var sdnPlugin sdn.NodeInterface
var sdnNode sdn.NodeInterface
var sdnProxy sdn.ProxyInterface
if sdn.IsOpenShiftNetworkPlugin(options.NetworkConfig.NetworkPluginName) {
sdnPlugin, sdnProxy, err = NewSDNInterfaces(options, originClient, kubeClient, internalKubeInformers, proxyconfig)
sdnNode, sdnProxy, err = NewSDNInterfaces(options, originClient, kubeClient, internalKubeInformers, proxyconfig)
if err != nil {
return nil, fmt.Errorf("SDN initialization failed: %v", err)
}
Expand Down Expand Up @@ -312,8 +312,8 @@ func BuildKubernetesNodeConfig(options configapi.NodeConfig, enableProxy, enable
ProxyConfig: proxyconfig,
EnableUnidling: options.EnableUnidling,

SDNPlugin: sdnPlugin,
SDNProxy: sdnProxy,
SDNNode: sdnNode,
SDNProxy: sdnProxy,
}

if enableDNS {
Expand Down
7 changes: 4 additions & 3 deletions pkg/cmd/server/kubernetes/node/sdn_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import (
osclient "github.com/openshift/origin/pkg/client"
configapi "github.com/openshift/origin/pkg/cmd/server/api"
"github.com/openshift/origin/pkg/sdn"
sdnplugin "github.com/openshift/origin/pkg/sdn/plugin"
sdnnode "github.com/openshift/origin/pkg/sdn/node"
sdnproxy "github.com/openshift/origin/pkg/sdn/proxy"
)

func NewSDNInterfaces(options configapi.NodeConfig, originClient *osclient.Client, kubeClient kclientset.Interface, internalKubeInformers kinternalinformers.SharedInformerFactory, proxyconfig *componentconfig.KubeProxyConfiguration) (sdn.NodeInterface, sdn.ProxyInterface, error) {
node, err := sdnplugin.NewNodePlugin(&sdnplugin.OsdnNodeConfig{
node, err := sdnnode.New(&sdnnode.OsdnNodeConfig{
PluginName: options.NetworkConfig.NetworkPluginName,
Hostname: options.NodeName,
SelfIP: options.NodeIP,
Expand All @@ -28,7 +29,7 @@ func NewSDNInterfaces(options configapi.NodeConfig, originClient *osclient.Clien
return nil, nil, err
}

proxy, err := sdnplugin.NewProxyPlugin(options.NetworkConfig.NetworkPluginName, originClient, kubeClient)
proxy, err := sdnproxy.New(options.NetworkConfig.NetworkPluginName, originClient, kubeClient)
if err != nil {
return nil, nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/server/origin/controller/network_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
osclient "github.com/openshift/origin/pkg/client"
configapi "github.com/openshift/origin/pkg/cmd/server/api"
"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
sdnplugin "github.com/openshift/origin/pkg/sdn/plugin"
sdnmaster "github.com/openshift/origin/pkg/sdn/master"
)

type SDNControllerConfig struct {
Expand All @@ -29,7 +29,7 @@ func (c *SDNControllerConfig) RunController(ctx ControllerContext) (bool, error)
if err != nil {
return false, err
}
err = sdnplugin.StartMaster(
err = sdnmaster.Start(
c.NetworkConfig,
osClient,
kClient,
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/start/start_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ func StartNode(nodeConfig configapi.NodeConfig, components *utilflags.ComponentF
config.RunKubelet()
}
if components.Enabled(ComponentPlugins) {
config.RunPlugin()
config.RunSDN()
}
if components.Enabled(ComponentProxy) {
config.RunProxy()
Expand Down
18 changes: 9 additions & 9 deletions pkg/sdn/plugin/common.go → pkg/sdn/common/common.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package plugin
package common

import (
"fmt"
Expand All @@ -23,11 +23,11 @@ import (
kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
)

func hostSubnetToString(subnet *osapi.HostSubnet) string {
func HostSubnetToString(subnet *osapi.HostSubnet) string {
return fmt.Sprintf("%s (host: %q, ip: %q, subnet: %q)", subnet.Name, subnet.Host, subnet.HostIP, subnet.Subnet)
}

func clusterNetworkToString(n *osapi.ClusterNetwork) string {
func ClusterNetworkToString(n *osapi.ClusterNetwork) string {
return fmt.Sprintf("%s (network: %q, hostSubnetBits: %d, serviceNetwork: %q, pluginName: %q)", n.Name, n.Network, n.HostSubnetLength, n.ServiceNetwork, n.PluginName)
}

Expand All @@ -36,7 +36,7 @@ type NetworkInfo struct {
ServiceNetwork *net.IPNet
}

func parseNetworkInfo(clusterNetwork string, serviceNetwork string) (*NetworkInfo, error) {
func ParseNetworkInfo(clusterNetwork string, serviceNetwork string) (*NetworkInfo, error) {
cn, err := netutils.ParseCIDRMask(clusterNetwork)
if err != nil {
_, cn, err := net.ParseCIDR(clusterNetwork)
Expand All @@ -60,7 +60,7 @@ func parseNetworkInfo(clusterNetwork string, serviceNetwork string) (*NetworkInf
}, nil
}

func (ni *NetworkInfo) validateNodeIP(nodeIP string) error {
func (ni *NetworkInfo) ValidateNodeIP(nodeIP string) error {
if nodeIP == "" || nodeIP == "127.0.0.1" {
return fmt.Errorf("invalid node IP %q", nodeIP)
}
Expand All @@ -82,7 +82,7 @@ func (ni *NetworkInfo) validateNodeIP(nodeIP string) error {
return nil
}

func (ni *NetworkInfo) checkHostNetworks(hostIPNets []*net.IPNet) error {
func (ni *NetworkInfo) CheckHostNetworks(hostIPNets []*net.IPNet) error {
errList := []error{}
for _, ipNet := range hostIPNets {
if ipNet.Contains(ni.ClusterNetwork.IP) {
Expand All @@ -101,7 +101,7 @@ func (ni *NetworkInfo) checkHostNetworks(hostIPNets []*net.IPNet) error {
return kerrors.NewAggregate(errList)
}

func (ni *NetworkInfo) checkClusterObjects(subnets []osapi.HostSubnet, pods []kapi.Pod, services []kapi.Service) error {
func (ni *NetworkInfo) CheckClusterObjects(subnets []osapi.HostSubnet, pods []kapi.Pod, services []kapi.Service) error {
var errList []error

for _, subnet := range subnets {
Expand Down Expand Up @@ -142,13 +142,13 @@ func (ni *NetworkInfo) checkClusterObjects(subnets []osapi.HostSubnet, pods []ka
return kerrors.NewAggregate(errList)
}

func getNetworkInfo(osClient *osclient.Client) (*NetworkInfo, error) {
func GetNetworkInfo(osClient *osclient.Client) (*NetworkInfo, error) {
cn, err := osClient.ClusterNetwork().Get(osapi.ClusterNetworkDefault, metav1.GetOptions{})
if err != nil {
return nil, err
}

return parseNetworkInfo(cn.Network, cn.ServiceNetwork)
return ParseNetworkInfo(cn.Network, cn.ServiceNetwork)
}

type ResourceName string
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package plugin
package common

import (
"net"
Expand Down Expand Up @@ -73,7 +73,7 @@ func Test_checkHostNetworks(t *testing.T) {
}

for _, test := range tests {
err := test.networkInfo.checkHostNetworks(hostIPNets)
err := test.networkInfo.CheckHostNetworks(hostIPNets)
if test.expectError {
if err == nil {
t.Fatalf("unexpected lack of error checking %q", test.name)
Expand Down Expand Up @@ -162,7 +162,7 @@ func Test_checkClusterObjects(t *testing.T) {
}

for _, test := range tests {
err := test.ni.checkClusterObjects(subnets, pods, services)
err := test.ni.CheckClusterObjects(subnets, pods, services)
if err == nil {
if len(test.errs) > 0 {
t.Fatalf("test %q unexpectedly did not get an error", test.name)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdn/plugin/dns.go → pkg/sdn/common/dns.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package plugin
package common

import (
"fmt"
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdn/plugin/dns_test.go → pkg/sdn/common/dns_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package plugin
package common

import (
"fmt"
Expand Down
2 changes: 2 additions & 0 deletions pkg/sdn/common/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Package common contains the OpenShift SDN code that is shared between master, node, and proxy
package common
8 changes: 4 additions & 4 deletions pkg/sdn/plugin/egress_dns.go → pkg/sdn/common/egress_dns.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package plugin
package common

import (
"net"
Expand Down Expand Up @@ -30,15 +30,15 @@ type EgressDNS struct {
added chan bool

// Report changes when there are dns updates
updates chan EgressDNSUpdate
Updates chan EgressDNSUpdate
}

func NewEgressDNS() *EgressDNS {
return &EgressDNS{
pdMap: map[ktypes.UID]*DNS{},
namespaces: map[ktypes.UID]string{},
added: make(chan bool),
updates: make(chan EgressDNSUpdate),
Updates: make(chan EgressDNSUpdate),
}
}

Expand Down Expand Up @@ -100,7 +100,7 @@ func (e *EgressDNS) Sync() {
}

if changed {
e.updates <- EgressDNSUpdate{policyUID, policyNamespace}
e.Updates <- EgressDNSUpdate{policyUID, policyNamespace}
}
continue
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package plugin
package common

import (
"fmt"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package plugin
package common

import (
"fmt"
Expand Down
2 changes: 2 additions & 0 deletions pkg/sdn/master/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Package master contains the OpenShift SDN code that runs on the master
package master
22 changes: 12 additions & 10 deletions pkg/sdn/plugin/master.go → pkg/sdn/master/master.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package plugin
package master

import (
"fmt"
Expand All @@ -12,6 +12,8 @@ import (
"github.com/openshift/origin/pkg/sdn"
osapi "github.com/openshift/origin/pkg/sdn/apis/network"
osapivalidation "github.com/openshift/origin/pkg/sdn/apis/network/validation"
"github.com/openshift/origin/pkg/sdn/common"
"github.com/openshift/origin/pkg/sdn/node"
"github.com/openshift/origin/pkg/util/netutils"

kapierrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -26,7 +28,7 @@ import (
type OsdnMaster struct {
kClient kclientset.Interface
osClient *osclient.Client
networkInfo *NetworkInfo
networkInfo *common.NetworkInfo
subnetAllocator *netutils.SubnetAllocator
vnids *masterVNIDMap
informers kinternalinformers.SharedInformerFactory
Expand All @@ -35,7 +37,7 @@ type OsdnMaster struct {
hostSubnetNodeIPs map[ktypes.UID]string
}

func StartMaster(networkConfig osconfigapi.MasterNetworkConfig, osClient *osclient.Client, kClient kclientset.Interface, informers kinternalinformers.SharedInformerFactory) error {
func Start(networkConfig osconfigapi.MasterNetworkConfig, osClient *osclient.Client, kClient kclientset.Interface, informers kinternalinformers.SharedInformerFactory) error {
if !sdn.IsOpenShiftNetworkPlugin(networkConfig.NetworkPluginName) {
return nil
}
Expand All @@ -50,7 +52,7 @@ func StartMaster(networkConfig osconfigapi.MasterNetworkConfig, osClient *osclie
}

var err error
master.networkInfo, err = parseNetworkInfo(networkConfig.ClusterNetworkCIDR, networkConfig.ServiceNetworkCIDR)
master.networkInfo, err = common.ParseNetworkInfo(networkConfig.ClusterNetworkCIDR, networkConfig.ServiceNetworkCIDR)
if err != nil {
return err
}
Expand Down Expand Up @@ -85,7 +87,7 @@ func StartMaster(networkConfig osconfigapi.MasterNetworkConfig, osClient *osclie
if _, err = master.osClient.ClusterNetwork().Create(configCN); err != nil {
return false, err
}
log.Infof("Created ClusterNetwork %s", clusterNetworkToString(configCN))
log.Infof("Created ClusterNetwork %s", common.ClusterNetworkToString(configCN))

if err = master.checkClusterNetworkAgainstClusterObjects(); err != nil {
log.Errorf("WARNING: cluster contains objects incompatible with new ClusterNetwork: %v", err)
Expand All @@ -101,9 +103,9 @@ func StartMaster(networkConfig osconfigapi.MasterNetworkConfig, osClient *osclie
if _, err = master.osClient.ClusterNetwork().Update(configCN); err != nil {
return false, err
}
log.Infof("Updated ClusterNetwork %s", clusterNetworkToString(configCN))
log.Infof("Updated ClusterNetwork %s", common.ClusterNetworkToString(configCN))
} else {
log.V(5).Infof("No change to ClusterNetwork %s", clusterNetworkToString(configCN))
log.V(5).Infof("No change to ClusterNetwork %s", common.ClusterNetworkToString(configCN))
}
}

Expand Down Expand Up @@ -137,11 +139,11 @@ func StartMaster(networkConfig osconfigapi.MasterNetworkConfig, osClient *osclie
}

func (master *OsdnMaster) checkClusterNetworkAgainstLocalNetworks() error {
hostIPNets, _, err := netutils.GetHostIPNetworks([]string{Tun0})
hostIPNets, _, err := netutils.GetHostIPNetworks([]string{node.Tun0})
if err != nil {
return err
}
return master.networkInfo.checkHostNetworks(hostIPNets)
return master.networkInfo.CheckHostNetworks(hostIPNets)
}

func (master *OsdnMaster) checkClusterNetworkAgainstClusterObjects() error {
Expand All @@ -158,7 +160,7 @@ func (master *OsdnMaster) checkClusterNetworkAgainstClusterObjects() error {
services = serviceList.Items
}

return master.networkInfo.checkClusterObjects(subnets, pods, services)
return master.networkInfo.CheckClusterObjects(subnets, pods, services)
}

func clusterNetworkChanged(obj *osapi.ClusterNetwork, old *osapi.ClusterNetwork) (bool, error) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package plugin
package master

import (
"testing"

osapi "github.com/openshift/origin/pkg/sdn/apis/network"
"github.com/openshift/origin/pkg/sdn/common"
)

func Test_clusterNetworkChanged(t *testing.T) {
Expand Down Expand Up @@ -118,10 +119,10 @@ func Test_clusterNetworkChanged(t *testing.T) {

changed, err := clusterNetworkChanged(&newCN, &origCN)
if changed != expectChanged {
t.Fatalf("unexpected result (%t instead of %t) on %q: %s -> %s", changed, expectChanged, test.name, clusterNetworkToString(&origCN), clusterNetworkToString(&newCN))
t.Fatalf("unexpected result (%t instead of %t) on %q: %s -> %s", changed, expectChanged, test.name, common.ClusterNetworkToString(&origCN), common.ClusterNetworkToString(&newCN))
}
if (err != nil) != test.expectError {
t.Fatalf("unexpected error on %q: %s -> %s: %v", test.name, clusterNetworkToString(&origCN), clusterNetworkToString(&newCN), err)
t.Fatalf("unexpected error on %q: %s -> %s: %v", test.name, common.ClusterNetworkToString(&origCN), common.ClusterNetworkToString(&newCN), err)
}
}
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Loading

0 comments on commit 558cdda

Please sign in to comment.