Skip to content

Commit f18d3c9

Browse files
author
Franck Rupin
committed
Fix comments in datapath service
- This commit addresses some comments done. - It fixes typos - It reorganizes error messages - It removes unnecessary print code
1 parent fb85caa commit f18d3c9

File tree

1 file changed

+20
-13
lines changed

1 file changed

+20
-13
lines changed

ovs/datapath.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,20 @@ package ovs
1616

1717
import (
1818
"errors"
19-
"fmt"
2019
"regexp"
2120
"strconv"
2221
"strings"
2322
)
2423

24+
var (
25+
errMissingDataPathName = errors.New("datapath name argument is mandatory")
26+
errUninitializedClient = errors.New("client unitialized")
27+
errMissingMandatoryZone = errors.New("at least 1 zone is mandatory")
28+
errWrongArgumentNumber = errors.New("missing or too much arguments to setup ct limits")
29+
errWrongDefaultArgument = errors.New("wrong argument while setting default ct limits")
30+
errWrongZoneArgument = errors.New("wrong argument while setting zone ct limits")
31+
)
32+
2533
// Zone defines the type used to store a zone as it is returned
2634
// by ovs-dpctl ct-*-limits commands
2735
type Zone map[string]uint64
@@ -84,10 +92,10 @@ type CLI interface {
8492
Exec(args ...string) ([]byte, error)
8593
}
8694

87-
// DataPathService defines the conrete type used for DataPath operations
95+
// DataPathService defines the concrete type used for DataPath operations
8896
// supported by the ovs-dpctl command
8997
type DataPathService struct {
90-
// We define here a CLI interface making easier to mock pvs-dpctl command
98+
// We define here a CLI interface making easier to mock ovs-dpctl command
9199
// as in github.com/digitalocean/go-openvswitch/ovs/datapath_test.go
92100
CLI
93101
}
@@ -133,7 +141,7 @@ func (dp *DataPathService) AddDataPath(dpName string) ([]byte, error) {
133141
return dp.CLI.Exec(args...)
134142
}
135143

136-
// DelDataPath create a Datapath with the command 'ovs-dpctl add-dp <DP>'
144+
// DelDataPath create a Datapath with the command 'ovs-dpctl del-dp <DP>'
137145
// It takes one argument, the required DataPath Name and returns an error
138146
// if it failed
139147
func (dp *DataPathService) DelDataPath(dpName string) ([]byte, error) {
@@ -148,7 +156,7 @@ func (dp *DataPathService) GetCTLimits(dpName string, zones []uint64) (*ConnTrac
148156
// Start by building the args
149157
args := []string{"ct-get-limits"}
150158
if dpName == "" {
151-
return nil, errors.New("datapath name argument is mandatory")
159+
return nil, errMissingDataPathName
152160
}
153161

154162
args = append(args, dpName)
@@ -170,7 +178,6 @@ func (dp *DataPathService) GetCTLimits(dpName string, zones []uint64) (*ConnTrac
170178

171179
r, err := regexp.Compile(`default`)
172180
if err != nil {
173-
fmt.Println(err.Error())
174181
return nil, err
175182
}
176183

@@ -212,7 +219,7 @@ func (dp *DataPathService) GetCTLimits(dpName string, zones []uint64) (*ConnTrac
212219
func (dp *DataPathService) SetCTLimits(dpName string, zone map[string]uint64) (string, error) {
213220
// Sanitize the input
214221
if dpName == "" {
215-
return "", errors.New("datapath name is required")
222+
return "", errMissingDataPathName
216223
}
217224
argsStr, err := ctSetLimitsArgsToString(zone)
218225
if err != nil {
@@ -230,10 +237,10 @@ func (dp *DataPathService) SetCTLimits(dpName string, zone map[string]uint64) (s
230237
// sudo ovs-dpctl ct-del-limits system@ovs-system zone=40,4
231238
func (dp *DataPathService) DelCTLimits(dpName string, zones []uint64) (string, error) {
232239
if dpName == "" {
233-
return "", errors.New("datapath name is missing")
240+
return "", errMissingDataPathName
234241
}
235242
if len(zones) < 1 {
236-
return "", errors.New("at least 1 zone is mandatory")
243+
return "", errMissingMandatoryZone
237244
}
238245

239246
var firstZone uint64
@@ -268,18 +275,18 @@ func ctSetLimitsArgsToString(zone map[string]uint64) (string, error) {
268275

269276
// We need at most 2 arguments and at least 1
270277
if len(args) == 0 || len(args) > 2 {
271-
return "", errors.New("missing or too much arguments to setup ct limits")
278+
return "", errWrongArgumentNumber
272279

273280
}
274281
// if we setup the default global setting we only need a single parameter
275282
// like "default=100000" and nothing else
276283
if defaultSetup && len(args) != 1 {
277-
return "", errors.New("wrong argument while setting default ct limits")
284+
return "", errWrongDefaultArgument
278285
}
279286
// if we setup a limit for dedicated zone we need 2 params like
280287
// "zone=3" and "limit=50000"
281288
if !defaultSetup && len(args) != 2 {
282-
return "", errors.New("wrong argument while setting zone ct limits")
289+
return "", errWrongZoneArgument
283290
}
284291

285292
var argsStr string
@@ -328,7 +335,7 @@ type OvsCLI struct {
328335
// Exec executes 'ovs-dpctl' + args passed in argument
329336
func (cli *OvsCLI) Exec(args ...string) ([]byte, error) {
330337
if cli.c == nil {
331-
return nil, errors.New("client unitialized")
338+
return nil, errUninitializedClient
332339
}
333340

334341
return cli.c.exec("ovs-dpctl", args...)

0 commit comments

Comments
 (0)