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

Minor/boring change: Consistently return user facing errors in SDN #13154

Merged
merged 1 commit into from
Mar 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion pkg/sdn/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func ValidateHostSubnet(hs *sdnapi.HostSubnet) field.ErrorList {
if hs.Subnet == "" {
// check if annotation exists, then let the Subnet field be empty
if _, ok := hs.Annotations[sdnapi.AssignHostSubnetAnnotation]; !ok {
allErrs = append(allErrs, field.Invalid(field.NewPath("subnet"), hs.Subnet, "Field cannot be empty"))
allErrs = append(allErrs, field.Invalid(field.NewPath("subnet"), hs.Subnet, "field cannot be empty"))
}
} else {
_, _, err := net.ParseCIDR(hs.Subnet)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdn/plugin/cniserver/cniserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func cniRequestToPodRequest(r *http.Request) (*PodRequest, error) {

cmd, ok := cr.Env["CNI_COMMAND"]
if !ok {
return nil, fmt.Errorf("Unexpected or missing CNI_COMMAND")
return nil, fmt.Errorf("unexpected or missing CNI_COMMAND")
}

req := &PodRequest{
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdn/plugin/cniserver/cniserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func TestCNIServer(t *testing.T) {
Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
},
result: nil,
errorPrefix: "Unexpected or missing CNI_COMMAND",
errorPrefix: "unexpected or missing CNI_COMMAND",
},
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/sdn/plugin/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ type NetworkInfo struct {
func parseNetworkInfo(clusterNetwork string, serviceNetwork string) (*NetworkInfo, error) {
_, cn, err := net.ParseCIDR(clusterNetwork)
if err != nil {
return nil, fmt.Errorf("Failed to parse ClusterNetwork CIDR %s: %v", clusterNetwork, err)
return nil, fmt.Errorf("failed to parse ClusterNetwork CIDR %s: %v", clusterNetwork, err)
}
_, sn, err := net.ParseCIDR(serviceNetwork)
if err != nil {
return nil, fmt.Errorf("Failed to parse ServiceNetwork CIDR %s: %v", serviceNetwork, err)
return nil, fmt.Errorf("failed to parse ServiceNetwork CIDR %s: %v", serviceNetwork, err)
}

return &NetworkInfo{
Expand All @@ -56,21 +56,21 @@ func parseNetworkInfo(clusterNetwork string, serviceNetwork string) (*NetworkInf

func (ni *NetworkInfo) validateNodeIP(nodeIP string) error {
if nodeIP == "" || nodeIP == "127.0.0.1" {
return fmt.Errorf("Invalid node IP %q", nodeIP)
return fmt.Errorf("invalid node IP %q", nodeIP)
}

// Ensure each node's NodeIP is not contained by the cluster network,
// which could cause a routing loop. (rhbz#1295486)
ipaddr := net.ParseIP(nodeIP)
if ipaddr == nil {
return fmt.Errorf("Failed to parse node IP %s", nodeIP)
return fmt.Errorf("failed to parse node IP %s", nodeIP)
}

if ni.ClusterNetwork.Contains(ipaddr) {
return fmt.Errorf("Node IP %s conflicts with cluster network %s", nodeIP, ni.ClusterNetwork.String())
return fmt.Errorf("node IP %s conflicts with cluster network %s", nodeIP, ni.ClusterNetwork.String())
}
if ni.ServiceNetwork.Contains(ipaddr) {
return fmt.Errorf("Node IP %s conflicts with service network %s", nodeIP, ni.ServiceNetwork.String())
return fmt.Errorf("node IP %s conflicts with service network %s", nodeIP, ni.ServiceNetwork.String())
}

return nil
Expand Down
8 changes: 4 additions & 4 deletions pkg/sdn/plugin/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ func (plugin *OsdnNode) getLocalSubnet() (string, error) {
}
})
if err != nil {
return "", fmt.Errorf("Failed to get subnet for this host: %s, error: %v", plugin.hostName, err)
return "", fmt.Errorf("failed to get subnet for this host: %s, error: %v", plugin.hostName, err)
}

if err = plugin.networkInfo.validateNodeIP(subnet.HostIP); err != nil {
return "", fmt.Errorf("Failed to validate own HostSubnet: %v", err)
return "", fmt.Errorf("failed to validate own HostSubnet: %v", err)
}

return subnet.Subnet, nil
Expand Down Expand Up @@ -347,11 +347,11 @@ func (plugin *OsdnNode) SetupSDN() (bool, error) {
// Enable IP forwarding for ipv4 packets
err = sysctl.SetSysctl("net/ipv4/ip_forward", 1)
if err != nil {
return false, fmt.Errorf("Could not enable IPv4 forwarding: %s", err)
return false, fmt.Errorf("could not enable IPv4 forwarding: %s", err)
}
err = sysctl.SetSysctl(fmt.Sprintf("net/ipv4/conf/%s/forwarding", TUN), 1)
if err != nil {
return false, fmt.Errorf("Could not enable IPv4 forwarding on %s: %s", TUN, err)
return false, fmt.Errorf("could not enable IPv4 forwarding on %s: %s", TUN, err)
}

// Table 253: rule version; note action is hex bytes separated by '.'
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdn/plugin/egress_network_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (plugin *OsdnNode) watchEgressNetworkPolicies() {

vnid, err := plugin.policy.GetVNID(policy.Namespace)
if err != nil {
return fmt.Errorf("Could not find netid for namespace %q: %v", policy.Namespace, err)
return fmt.Errorf("could not find netid for namespace %q: %v", policy.Namespace, err)
}

policies := plugin.egressPolicies[vnid]
Expand Down
4 changes: 2 additions & 2 deletions pkg/sdn/plugin/eventqueue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ func TestEventQueueDeletedFinalStateUnknown(t *testing.T) {
called = true
if _, ok := delta.Object.(cache.DeletedFinalStateUnknown); !ok {
// Capture error that Pop() logs the error but doesn't return
processErr = fmt.Errorf("Unexpected item type %T", delta.Object)
processErr = fmt.Errorf("unexpected item type %T", delta.Object)
return processErr
}
return nil
Expand Down Expand Up @@ -649,7 +649,7 @@ func TestEventQueueDeletedFinalStateUnknown(t *testing.T) {
called = true
if _, ok := delta.Object.(*api.ObjectMeta); !ok {
// Capture error that Pop() logs the error but doesn't return
processErr = fmt.Errorf("Unexpected item type %T", delta.Object)
processErr = fmt.Errorf("unexpected item type %T", delta.Object)
return processErr
}
return nil
Expand Down
16 changes: 8 additions & 8 deletions pkg/sdn/plugin/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,32 +120,32 @@ func (master *OsdnMaster) validateNetworkConfig() error {
// Ensure cluster and service network don't overlap with host networks
for _, ipNet := range hostIPNets {
if ipNet.Contains(ni.ClusterNetwork.IP) {
errList = append(errList, fmt.Errorf("Error: Cluster IP: %s conflicts with host network: %s", ni.ClusterNetwork.IP.String(), ipNet.String()))
errList = append(errList, fmt.Errorf("cluster IP: %s conflicts with host network: %s", ni.ClusterNetwork.IP.String(), ipNet.String()))
}
if ni.ClusterNetwork.Contains(ipNet.IP) {
errList = append(errList, fmt.Errorf("Error: Host network with IP: %s conflicts with cluster network: %s", ipNet.IP.String(), ni.ClusterNetwork.String()))
errList = append(errList, fmt.Errorf("host network with IP: %s conflicts with cluster network: %s", ipNet.IP.String(), ni.ClusterNetwork.String()))
}
if ipNet.Contains(ni.ServiceNetwork.IP) {
errList = append(errList, fmt.Errorf("Error: Service IP: %s conflicts with host network: %s", ni.ServiceNetwork.String(), ipNet.String()))
errList = append(errList, fmt.Errorf("service IP: %s conflicts with host network: %s", ni.ServiceNetwork.String(), ipNet.String()))
}
if ni.ServiceNetwork.Contains(ipNet.IP) {
errList = append(errList, fmt.Errorf("Error: Host network with IP: %s conflicts with service network: %s", ipNet.IP.String(), ni.ServiceNetwork.String()))
errList = append(errList, fmt.Errorf("host network with IP: %s conflicts with service network: %s", ipNet.IP.String(), ni.ServiceNetwork.String()))
}
}

// Ensure each host subnet is within the cluster network
subnets, err := master.osClient.HostSubnets().List(kapi.ListOptions{})
if err != nil {
return fmt.Errorf("Error in initializing/fetching subnets: %v", err)
return fmt.Errorf("error in initializing/fetching subnets: %v", err)
}
for _, sub := range subnets.Items {
subnetIP, _, _ := net.ParseCIDR(sub.Subnet)
if subnetIP == nil {
errList = append(errList, fmt.Errorf("Failed to parse network address: %s", sub.Subnet))
errList = append(errList, fmt.Errorf("failed to parse network address: %s", sub.Subnet))
continue
}
if !ni.ClusterNetwork.Contains(subnetIP) {
errList = append(errList, fmt.Errorf("Error: Existing node subnet: %s is not part of cluster network: %s", sub.Subnet, ni.ClusterNetwork.String()))
errList = append(errList, fmt.Errorf("existing node subnet: %s is not part of cluster network: %s", sub.Subnet, ni.ClusterNetwork.String()))
}
}

Expand All @@ -156,7 +156,7 @@ func (master *OsdnMaster) validateNetworkConfig() error {
}
for _, svc := range services.Items {
if !ni.ServiceNetwork.Contains(net.ParseIP(svc.Spec.ClusterIP)) {
errList = append(errList, fmt.Errorf("Error: Existing service with IP: %s is not part of service network: %s", svc.Spec.ClusterIP, ni.ServiceNetwork.String()))
errList = append(errList, fmt.Errorf("existing service with IP: %s is not part of service network: %s", svc.Spec.ClusterIP, ni.ServiceNetwork.String()))
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/sdn/plugin/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,12 +198,12 @@ func (node *OsdnNode) Start() error {
var err error
node.networkInfo, err = getNetworkInfo(node.osClient)
if err != nil {
return fmt.Errorf("Failed to get network information: %v", err)
return fmt.Errorf("failed to get network information: %v", err)
}

nodeIPTables := newNodeIPTables(node.networkInfo.ClusterNetwork.String(), node.iptablesSyncPeriod)
if err = nodeIPTables.Setup(); err != nil {
return fmt.Errorf("Failed to set up iptables: %v", err)
return fmt.Errorf("failed to set up iptables: %v", err)
}

node.localSubnetCIDR, err = node.getLocalSubnet()
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdn/plugin/node_iptables.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (n *NodeIPTables) syncIPTableRules() error {
for _, rule := range rules {
_, err := n.ipt.EnsureRule(iptables.Prepend, iptables.Table(rule.table), iptables.Chain(rule.chain), rule.args...)
if err != nil {
return fmt.Errorf("Failed to ensure rule %v exists: %v", rule, err)
return fmt.Errorf("failed to ensure rule %v exists: %v", rule, err)
}
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdn/plugin/pod_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func (m *podManager) getNonExitedPods() ([]*kcontainer.Pod, error) {
ret := []*kcontainer.Pod{}
pods, err := m.host.GetRuntime().GetPods(true)
if err != nil {
return nil, fmt.Errorf("Failed to retrieve pods from runtime: %v", err)
return nil, fmt.Errorf("failed to retrieve pods from runtime: %v", err)
}
for _, p := range pods {
if podIsExited(p) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdn/plugin/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (proxy *OsdnProxy) Start(baseHandler pconfig.EndpointsConfigHandler) error

policies, err := proxy.osClient.EgressNetworkPolicies(kapi.NamespaceAll).List(kapi.ListOptions{})
if err != nil {
return fmt.Errorf("Could not get EgressNetworkPolicies: %s", err)
return fmt.Errorf("could not get EgressNetworkPolicies: %s", err)
}
for _, policy := range policies.Items {
proxy.updateNetworkPolicy(policy)
Expand Down
16 changes: 8 additions & 8 deletions pkg/sdn/plugin/subnets.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (master *OsdnMaster) addNode(nodeName string, nodeIP string, hsAnnotations
sub.HostIP = nodeIP
sub, err = master.osClient.HostSubnets().Update(sub)
if err != nil {
return fmt.Errorf("Error updating subnet %s for node %s: %v", sub.Subnet, nodeName, err)
return fmt.Errorf("error updating subnet %s for node %s: %v", sub.Subnet, nodeName, err)
}
log.Infof("Updated HostSubnet %s", hostSubnetToString(sub))
return nil
Expand All @@ -74,7 +74,7 @@ func (master *OsdnMaster) addNode(nodeName string, nodeIP string, hsAnnotations
// Create new subnet
sn, err := master.subnetAllocator.GetNetwork()
if err != nil {
return fmt.Errorf("Error allocating network for node %s: %v", nodeName, err)
return fmt.Errorf("error allocating network for node %s: %v", nodeName, err)
}

sub = &osapi.HostSubnet{
Expand All @@ -87,7 +87,7 @@ func (master *OsdnMaster) addNode(nodeName string, nodeIP string, hsAnnotations
sub, err = master.osClient.HostSubnets().Create(sub)
if err != nil {
master.subnetAllocator.ReleaseNetwork(sn)
return fmt.Errorf("Error creating subnet %s for node %s: %v", sn.String(), nodeName, err)
return fmt.Errorf("error creating subnet %s for node %s: %v", sn.String(), nodeName, err)
}
log.Infof("Created HostSubnet %s", hostSubnetToString(sub))
return nil
Expand All @@ -96,11 +96,11 @@ func (master *OsdnMaster) addNode(nodeName string, nodeIP string, hsAnnotations
func (master *OsdnMaster) deleteNode(nodeName string) error {
sub, err := master.osClient.HostSubnets().Get(nodeName)
if err != nil {
return fmt.Errorf("Error fetching subnet for node %q for deletion: %v", nodeName, err)
return fmt.Errorf("error fetching subnet for node %q for deletion: %v", nodeName, err)
}
err = master.osClient.HostSubnets().Delete(nodeName)
if err != nil {
return fmt.Errorf("Error deleting subnet %v for node %q: %v", sub, nodeName, err)
return fmt.Errorf("error deleting subnet %v for node %q: %v", sub, nodeName, err)
}

log.Infof("Deleted HostSubnet %s", hostSubnetToString(sub))
Expand Down Expand Up @@ -158,7 +158,7 @@ func (master *OsdnMaster) clearInitialNodeNetworkUnavailableCondition(node *kapi
return err
})
if resultErr != nil {
utilruntime.HandleError(fmt.Errorf("Status update failed for local node: %v", resultErr))
utilruntime.HandleError(fmt.Errorf("status update failed for local node: %v", resultErr))
} else if cleared {
log.Infof("Cleared node NetworkUnavailable/NoRouteCreated condition for %s", node.ObjectMeta.Name)
}
Expand Down Expand Up @@ -197,7 +197,7 @@ func (master *OsdnMaster) watchNodes() {

err = master.deleteNode(name)
if err != nil {
return fmt.Errorf("Error deleting node %s: %v", name, err)
return fmt.Errorf("error deleting node %s: %v", name, err)
}
}
return nil
Expand Down Expand Up @@ -253,7 +253,7 @@ func (master *OsdnMaster) watchSubnets() {
// release the subnet
_, ipnet, err := net.ParseCIDR(subnet)
if err != nil {
return fmt.Errorf("Error parsing subnet %q for node %q for deletion: %v", subnet, name, err)
return fmt.Errorf("error parsing subnet %q for node %q for deletion: %v", subnet, name, err)
}
master.subnetAllocator.ReleaseNetwork(ipnet)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/sdn/plugin/vnids_master.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,11 @@ func (master *OsdnMaster) watchNamespaces() {
switch delta.Type {
case cache.Sync, cache.Added, cache.Updated:
if err := master.vnids.assignVNID(master.osClient, name); err != nil {
return fmt.Errorf("Error assigning netid: %v", err)
return fmt.Errorf("error assigning netid: %v", err)
}
case cache.Deleted:
if err := master.vnids.revokeVNID(master.osClient, name); err != nil {
return fmt.Errorf("Error revoking netid: %v", err)
return fmt.Errorf("error revoking netid: %v", err)
}
}
return nil
Expand All @@ -309,7 +309,7 @@ func (master *OsdnMaster) watchNetNamespaces() {
case cache.Sync, cache.Added, cache.Updated:
err := master.vnids.updateVNID(master.osClient, netns)
if err != nil {
return fmt.Errorf("Error updating netid: %v", err)
return fmt.Errorf("error updating netid: %v", err)
}
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions pkg/sdn/plugin/vnids_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (vmap *nodeVNIDMap) GetVNID(name string) (uint32, error) {
if id, ok := vmap.ids[name]; ok {
return id, nil
}
return 0, fmt.Errorf("Failed to find netid for namespace: %s in vnid map", name)
return 0, fmt.Errorf("failed to find netid for namespace: %s in vnid map", name)
}

func (vmap *nodeVNIDMap) GetMulticastEnabled(id uint32) bool {
Expand Down Expand Up @@ -113,7 +113,7 @@ func (vmap *nodeVNIDMap) WaitAndGetVNID(name string) (uint32, error) {
if err == nil {
return id, nil
} else {
return 0, fmt.Errorf("Failed to find netid for namespace: %s in vnid map", name)
return 0, fmt.Errorf("failed to find netid for namespace: %s in vnid map", name)
}
}

Expand All @@ -137,7 +137,7 @@ func (vmap *nodeVNIDMap) unsetVNID(name string) (id uint32, err error) {

id, found := vmap.ids[name]
if !found {
return 0, fmt.Errorf("Failed to find netid for namespace: %s in vnid map", name)
return 0, fmt.Errorf("failed to find netid for namespace: %s in vnid map", name)
}
vmap.removeNamespaceFromSet(name, id)
delete(vmap.ids, name)
Expand Down