Skip to content

Commit 232c1e0

Browse files
author
Ravi Sankar Penta
committed
Minor/boring change: Consistently return user facing errors in SDN
Follow the existing convention: errors should always start with lowercase
1 parent 2358aeb commit 232c1e0

23 files changed

+80
-80
lines changed

Diff for: ‎pkg/sdn/api/netid.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ func ValidVNID(vnid uint32) error {
3535
return nil
3636
}
3737
if vnid < MinVNID {
38-
return fmt.Errorf("VNID must be greater than or equal to %d", MinVNID)
38+
return fmt.Errorf("vnid must be greater than or equal to %d", MinVNID)
3939
}
4040
if vnid > MaxVNID {
41-
return fmt.Errorf("VNID must be less than or equal to %d", MaxVNID)
41+
return fmt.Errorf("vnid must be less than or equal to %d", MaxVNID)
4242
}
4343
return nil
4444
}

Diff for: ‎pkg/sdn/api/validation/validation.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func ValidateHostSubnet(hs *sdnapi.HostSubnet) field.ErrorList {
8888
if hs.Subnet == "" {
8989
// check if annotation exists, then let the Subnet field be empty
9090
if _, ok := hs.Annotations[sdnapi.AssignHostSubnetAnnotation]; !ok {
91-
allErrs = append(allErrs, field.Invalid(field.NewPath("subnet"), hs.Subnet, "Field cannot be empty"))
91+
allErrs = append(allErrs, field.Invalid(field.NewPath("subnet"), hs.Subnet, "field cannot be empty"))
9292
}
9393
} else {
9494
_, _, err := net.ParseCIDR(hs.Subnet)

Diff for: ‎pkg/sdn/plugin/bin/openshift-sdn-ovs

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ del_ovs_port() {
3333
add_ovs_flows() {
3434
ovs_port=$(ovs-vsctl get Interface ${veth_host} ofport)
3535
if [ -z "$ovs_port" ]; then
36-
echo "Could not find OVS port for veth device ${veth_host} of container ${net_container}"
36+
echo "could not find OVS port for veth device ${veth_host} of container ${net_container}"
3737
exit 1
3838
fi
3939

@@ -95,7 +95,7 @@ run() {
9595
;;
9696

9797
*)
98-
echo "Bad input: $@"
98+
echo "bad input: $@"
9999
exit 1
100100
esac
101101
}

Diff for: ‎pkg/sdn/plugin/cniserver/cniserver.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func (s *CNIServer) Start(requestFunc cniRequestFunc) error {
146146
s.SetKeepAlivesEnabled(false)
147147
go utilwait.Forever(func() {
148148
if err := s.Serve(l); err != nil {
149-
utilruntime.HandleError(fmt.Errorf("CNI server Serve() failed: %v", err))
149+
utilruntime.HandleError(fmt.Errorf("cni server Serve() failed: %v", err))
150150
}
151151
}, 0)
152152
return nil
@@ -177,12 +177,12 @@ func cniRequestToPodRequest(r *http.Request) (*PodRequest, error) {
177177
var cr CNIRequest
178178
b, _ := ioutil.ReadAll(r.Body)
179179
if err := json.Unmarshal(b, &cr); err != nil {
180-
return nil, fmt.Errorf("JSON unmarshal error: %v", err)
180+
return nil, fmt.Errorf("json unmarshal error: %v", err)
181181
}
182182

183183
cmd, ok := cr.Env["CNI_COMMAND"]
184184
if !ok {
185-
return nil, fmt.Errorf("Unexpected or missing CNI_COMMAND")
185+
return nil, fmt.Errorf("unexpected or missing CNI_COMMAND")
186186
}
187187

188188
req := &PodRequest{

Diff for: ‎pkg/sdn/plugin/cniserver/cniserver_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ func TestCNIServer(t *testing.T) {
172172
Config: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
173173
},
174174
result: nil,
175-
errorPrefix: "Unexpected or missing CNI_COMMAND",
175+
errorPrefix: "unexpected or missing CNI_COMMAND",
176176
},
177177
}
178178

Diff for: ‎pkg/sdn/plugin/common.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ type NetworkInfo struct {
4141
func parseNetworkInfo(clusterNetwork string, serviceNetwork string) (*NetworkInfo, error) {
4242
_, cn, err := net.ParseCIDR(clusterNetwork)
4343
if err != nil {
44-
return nil, fmt.Errorf("Failed to parse ClusterNetwork CIDR %s: %v", clusterNetwork, err)
44+
return nil, fmt.Errorf("failed to parse ClusterNetwork CIDR %s: %v", clusterNetwork, err)
4545
}
4646
_, sn, err := net.ParseCIDR(serviceNetwork)
4747
if err != nil {
48-
return nil, fmt.Errorf("Failed to parse ServiceNetwork CIDR %s: %v", serviceNetwork, err)
48+
return nil, fmt.Errorf("failed to parse ServiceNetwork CIDR %s: %v", serviceNetwork, err)
4949
}
5050

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

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

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

6969
if ni.ClusterNetwork.Contains(ipaddr) {
70-
return fmt.Errorf("Node IP %s conflicts with cluster network %s", nodeIP, ni.ClusterNetwork.String())
70+
return fmt.Errorf("node IP %s conflicts with cluster network %s", nodeIP, ni.ClusterNetwork.String())
7171
}
7272
if ni.ServiceNetwork.Contains(ipaddr) {
73-
return fmt.Errorf("Node IP %s conflicts with service network %s", nodeIP, ni.ServiceNetwork.String())
73+
return fmt.Errorf("node IP %s conflicts with service network %s", nodeIP, ni.ServiceNetwork.String())
7474
}
7575

7676
return nil

Diff for: ‎pkg/sdn/plugin/controller.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,11 @@ func (plugin *OsdnNode) getLocalSubnet() (string, error) {
7575
}
7676
})
7777
if err != nil {
78-
return "", fmt.Errorf("Failed to get subnet for this host: %s, error: %v", plugin.hostName, err)
78+
return "", fmt.Errorf("failed to get subnet for this host: %s, error: %v", plugin.hostName, err)
7979
}
8080

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

8585
return subnet.Subnet, nil
@@ -347,11 +347,11 @@ func (plugin *OsdnNode) SetupSDN() (bool, error) {
347347
// Enable IP forwarding for ipv4 packets
348348
err = sysctl.SetSysctl("net/ipv4/ip_forward", 1)
349349
if err != nil {
350-
return false, fmt.Errorf("Could not enable IPv4 forwarding: %s", err)
350+
return false, fmt.Errorf("could not enable IPv4 forwarding: %s", err)
351351
}
352352
err = sysctl.SetSysctl(fmt.Sprintf("net/ipv4/conf/%s/forwarding", TUN), 1)
353353
if err != nil {
354-
return false, fmt.Errorf("Could not enable IPv4 forwarding on %s: %s", TUN, err)
354+
return false, fmt.Errorf("could not enable IPv4 forwarding on %s: %s", TUN, err)
355355
}
356356

357357
// Table 253: rule version; note action is hex bytes separated by '.'

Diff for: ‎pkg/sdn/plugin/egress_network_policy.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func (plugin *OsdnNode) watchEgressNetworkPolicies() {
4141

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

4747
policies := plugin.egressPolicies[vnid]

Diff for: ‎pkg/sdn/plugin/eventqueue_test.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -620,18 +620,18 @@ func TestEventQueueDeletedFinalStateUnknown(t *testing.T) {
620620
called = true
621621
if _, ok := delta.Object.(cache.DeletedFinalStateUnknown); !ok {
622622
// Capture error that Pop() logs the error but doesn't return
623-
processErr = fmt.Errorf("Unexpected item type %T", delta.Object)
623+
processErr = fmt.Errorf("unexpected item type %T", delta.Object)
624624
return processErr
625625
}
626626
return nil
627627
}, nil); err != nil {
628628
t.Fatalf(fmt.Sprintf("%v", err))
629629
}
630630
if !called {
631-
t.Fatalf("Delta pop function wasn't called")
631+
t.Fatalf("delta pop function wasn't called")
632632
}
633633
if processErr != nil {
634-
t.Fatalf("Delta pop function returned error %v", processErr)
634+
t.Fatalf("delta pop function returned error %v", processErr)
635635
}
636636

637637
// Repeat but this time make sure we get the objects we want, not DeletedFinalStateUnknown
@@ -649,17 +649,17 @@ func TestEventQueueDeletedFinalStateUnknown(t *testing.T) {
649649
called = true
650650
if _, ok := delta.Object.(*api.ObjectMeta); !ok {
651651
// Capture error that Pop() logs the error but doesn't return
652-
processErr = fmt.Errorf("Unexpected item type %T", delta.Object)
652+
processErr = fmt.Errorf("unexpected item type %T", delta.Object)
653653
return processErr
654654
}
655655
return nil
656656
}, &api.ObjectMeta{}); err != nil {
657657
t.Fatalf(fmt.Sprintf("%v", err))
658658
}
659659
if !called {
660-
t.Fatalf("Delta pop function wasn't called")
660+
t.Fatalf("delta pop function wasn't called")
661661
}
662662
if processErr != nil {
663-
t.Fatalf("Delta pop function returned error %v", processErr)
663+
t.Fatalf("delta pop function returned error %v", processErr)
664664
}
665665
}

Diff for: ‎pkg/sdn/plugin/master.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -120,32 +120,32 @@ func (master *OsdnMaster) validateNetworkConfig() error {
120120
// Ensure cluster and service network don't overlap with host networks
121121
for _, ipNet := range hostIPNets {
122122
if ipNet.Contains(ni.ClusterNetwork.IP) {
123-
errList = append(errList, fmt.Errorf("Error: Cluster IP: %s conflicts with host network: %s", ni.ClusterNetwork.IP.String(), ipNet.String()))
123+
errList = append(errList, fmt.Errorf("error: Cluster IP: %s conflicts with host network: %s", ni.ClusterNetwork.IP.String(), ipNet.String()))
124124
}
125125
if ni.ClusterNetwork.Contains(ipNet.IP) {
126-
errList = append(errList, fmt.Errorf("Error: Host network with IP: %s conflicts with cluster network: %s", ipNet.IP.String(), ni.ClusterNetwork.String()))
126+
errList = append(errList, fmt.Errorf("error: Host network with IP: %s conflicts with cluster network: %s", ipNet.IP.String(), ni.ClusterNetwork.String()))
127127
}
128128
if ipNet.Contains(ni.ServiceNetwork.IP) {
129-
errList = append(errList, fmt.Errorf("Error: Service IP: %s conflicts with host network: %s", ni.ServiceNetwork.String(), ipNet.String()))
129+
errList = append(errList, fmt.Errorf("error: Service IP: %s conflicts with host network: %s", ni.ServiceNetwork.String(), ipNet.String()))
130130
}
131131
if ni.ServiceNetwork.Contains(ipNet.IP) {
132-
errList = append(errList, fmt.Errorf("Error: Host network with IP: %s conflicts with service network: %s", ipNet.IP.String(), ni.ServiceNetwork.String()))
132+
errList = append(errList, fmt.Errorf("error: Host network with IP: %s conflicts with service network: %s", ipNet.IP.String(), ni.ServiceNetwork.String()))
133133
}
134134
}
135135

136136
// Ensure each host subnet is within the cluster network
137137
subnets, err := master.osClient.HostSubnets().List(kapi.ListOptions{})
138138
if err != nil {
139-
return fmt.Errorf("Error in initializing/fetching subnets: %v", err)
139+
return fmt.Errorf("error in initializing/fetching subnets: %v", err)
140140
}
141141
for _, sub := range subnets.Items {
142142
subnetIP, _, _ := net.ParseCIDR(sub.Subnet)
143143
if subnetIP == nil {
144-
errList = append(errList, fmt.Errorf("Failed to parse network address: %s", sub.Subnet))
144+
errList = append(errList, fmt.Errorf("failed to parse network address: %s", sub.Subnet))
145145
continue
146146
}
147147
if !ni.ClusterNetwork.Contains(subnetIP) {
148-
errList = append(errList, fmt.Errorf("Error: Existing node subnet: %s is not part of cluster network: %s", sub.Subnet, ni.ClusterNetwork.String()))
148+
errList = append(errList, fmt.Errorf("error: Existing node subnet: %s is not part of cluster network: %s", sub.Subnet, ni.ClusterNetwork.String()))
149149
}
150150
}
151151

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

Diff for: ‎pkg/sdn/plugin/networkpolicy.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func (np *networkPolicyPlugin) initNamespaces() error {
120120
policies, err := np.node.kClient.Extensions().NetworkPolicies(kapi.NamespaceAll).List(kapi.ListOptions{})
121121
if err != nil {
122122
if kapierrs.IsForbidden(err) {
123-
glog.Errorf("Unable to query NetworkPolicies (%v) - please ensure your nodes have access to view NetworkPolicy (eg, 'oadm policy reconcile-cluster-roles')", err)
123+
glog.Errorf("unable to query NetworkPolicies (%v) - please ensure your nodes have access to view NetworkPolicy (eg, 'oadm policy reconcile-cluster-roles')", err)
124124
}
125125
return err
126126
}
@@ -333,7 +333,7 @@ func (np *networkPolicyPlugin) selectNamespaces(lsel *unversioned.LabelSelector)
333333
sel, err := unversioned.LabelSelectorAsSelector(lsel)
334334
if err != nil {
335335
// Shouldn't happen
336-
glog.Errorf("ValidateNetworkPolicy() failure! Invalid NamespaceSelector: %v", err)
336+
glog.Errorf("Validating network policy failed, invalid NamespaceSelector: %v", err)
337337
return vnids
338338
}
339339
for vnid, ns := range np.namespaces {
@@ -351,7 +351,7 @@ func (np *networkPolicyPlugin) selectPods(npns *npNamespace, lsel *unversioned.L
351351
sel, err := unversioned.LabelSelectorAsSelector(lsel)
352352
if err != nil {
353353
// Shouldn't happen
354-
glog.Errorf("ValidateNetworkPolicy() failure! Invalid PodSelector: %v", err)
354+
glog.Errorf("Validating network policy failed, invalid PodSelector: %v", err)
355355
return ips
356356
}
357357
for _, pod := range npns.pods {

Diff for: ‎pkg/sdn/plugin/node.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,12 @@ func (node *OsdnNode) Start() error {
198198
var err error
199199
node.networkInfo, err = getNetworkInfo(node.osClient)
200200
if err != nil {
201-
return fmt.Errorf("Failed to get network information: %v", err)
201+
return fmt.Errorf("failed to get network information: %v", err)
202202
}
203203

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

209209
node.localSubnetCIDR, err = node.getLocalSubnet()
@@ -318,7 +318,7 @@ func (node *OsdnNode) IsPodNetworkReady() error {
318318
case <-node.podNetworkReady:
319319
return nil
320320
default:
321-
return fmt.Errorf("SDN pod network is not ready")
321+
return fmt.Errorf("sdn pod network is not ready")
322322
}
323323
}
324324

Diff for: ‎pkg/sdn/plugin/node_iptables.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (n *NodeIPTables) syncIPTableRules() error {
8282
for _, rule := range rules {
8383
_, err := n.ipt.EnsureRule(iptables.Prepend, iptables.Table(rule.table), iptables.Chain(rule.chain), rule.args...)
8484
if err != nil {
85-
return fmt.Errorf("Failed to ensure rule %v exists: %v", rule, err)
85+
return fmt.Errorf("failed to ensure rule %v exists: %v", rule, err)
8686
}
8787
}
8888
return nil

Diff for: ‎pkg/sdn/plugin/pod_linux.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ func (m *podManager) getNonExitedPods() ([]*kcontainer.Pod, error) {
287287
ret := []*kcontainer.Pod{}
288288
pods, err := m.host.GetRuntime().GetPods(true)
289289
if err != nil {
290-
return nil, fmt.Errorf("Failed to retrieve pods from runtime: %v", err)
290+
return nil, fmt.Errorf("failed to retrieve pods from runtime: %v", err)
291291
}
292292
for _, p := range pods {
293293
if podIsExited(p) {

Diff for: ‎pkg/sdn/plugin/pod_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -518,18 +518,18 @@ func TestUpdateMulticastFlows(t *testing.T) {
518518

519519
outputs := localMulticastOutputs(pods, 0)
520520
if outputs != "" {
521-
t.Fatalf("Unexpected outputs for vnid 0: %s", outputs)
521+
t.Fatalf("unexpected outputs for vnid 0: %s", outputs)
522522
}
523523
outputs = localMulticastOutputs(pods, 5)
524524
if outputs != "output:2,output:7,output:8" {
525-
t.Fatalf("Unexpected outputs for vnid 5: %s", outputs)
525+
t.Fatalf("unexpected outputs for vnid 5: %s", outputs)
526526
}
527527
outputs = localMulticastOutputs(pods, 6)
528528
if outputs != "output:3,output:9" {
529-
t.Fatalf("Unexpected outputs for vnid 6: %s", outputs)
529+
t.Fatalf("unexpected outputs for vnid 6: %s", outputs)
530530
}
531531
outputs = localMulticastOutputs(pods, 8)
532532
if outputs != "output:10" {
533-
t.Fatalf("Unexpected outputs for vnid 0: %s", outputs)
533+
t.Fatalf("unexpected outputs for vnid 0: %s", outputs)
534534
}
535535
}

Diff for: ‎pkg/sdn/plugin/proxy.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (proxy *OsdnProxy) Start(baseHandler pconfig.EndpointsConfigHandler) error
5858

5959
policies, err := proxy.osClient.EgressNetworkPolicies(kapi.NamespaceAll).List(kapi.ListOptions{})
6060
if err != nil {
61-
return fmt.Errorf("Could not get EgressNetworkPolicies: %s", err)
61+
return fmt.Errorf("could not get EgressNetworkPolicies: %s", err)
6262
}
6363
for _, policy := range policies.Items {
6464
proxy.updateNetworkPolicy(policy)

Diff for: ‎pkg/sdn/plugin/sdn-cni-plugin/openshift-sdn.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func (p *cniPlugin) doCNI(url string, req *cniserver.CNIRequest) ([]byte, error)
7474
}
7575

7676
if resp.StatusCode != 200 {
77-
return nil, fmt.Errorf("CNI request failed with status %v: '%s'", resp.StatusCode, string(body))
77+
return nil, fmt.Errorf("cni request failed with status %v: '%s'", resp.StatusCode, string(body))
7878
}
7979

8080
return body, nil

Diff for: ‎pkg/sdn/plugin/sdn-cni-plugin/sdn_cni_plugin_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func TestOpenshiftSdnCNIPlugin(t *testing.T) {
127127
Path: "/some/path",
128128
StdinData: []byte("{\"cniVersion\": \"0.1.0\",\"name\": \"openshift-sdn\",\"type\": \"openshift-sdn\"}"),
129129
},
130-
errorPrefix: "CNI request failed with status 400: 'invalid CNI_ARG",
130+
errorPrefix: "cni request failed with status 400: 'invalid CNI_ARG",
131131
},
132132
}
133133

0 commit comments

Comments
 (0)