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

Fix error format strings according to best practices from CodeReviewComments #574

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 cmd/e2e-test/security_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func verifySecurityPolicy(t *testing.T, gclb *fuzz.GCLB, svcNamespace, svcName,
}

if bs.Beta == nil {
return fmt.Errorf("Beta BackendService resource not found: %v", bs)
return fmt.Errorf("beta BackendService resource not found: %v", bs)
}
if bs.Beta.SecurityPolicy != policyLink {
return fmt.Errorf("backend service %q has security policy %q, want %q", bs.Beta.Name, bs.Beta.SecurityPolicy, policyLink)
Expand Down
2 changes: 1 addition & 1 deletion pkg/backends/ig_linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (l *instanceGroupLinker) Link(sp utils.ServicePort, groups []GroupKey) erro
for _, group := range groups {
ig, err := l.instancePool.Get(l.namer.InstanceGroup(), group.Zone)
if err != nil {
return fmt.Errorf("Error retrieving IG for linking with backend %+v: %v", sp, err)
return fmt.Errorf("error retrieving IG for linking with backend %+v: %v", sp, err)
}
igLinks = append(igLinks, ig.SelfLink)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/crd/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ func validation(typeSource string, fn common.GetOpenAPIDefinitions) (*apiextensi
jsonSchemaProps := &apiextensionsv1beta1.JSONSchemaProps{}
bytes, err := json.Marshal(condensedSchema)
if err != nil {
return nil, fmt.Errorf("Error marshalling OpenAPI schema to JSON: %v", err)
return nil, fmt.Errorf("error marshalling OpenAPI schema to JSON: %v", err)
}
err = json.Unmarshal(bytes, jsonSchemaProps)
if err != nil {
return nil, fmt.Errorf("Error unmarshalling OpenAPI JSON: %v", err)
return nil, fmt.Errorf("error unmarshalling OpenAPI JSON: %v", err)
}
return &apiextensionsv1beta1.CustomResourceValidation{OpenAPIV3Schema: jsonSchemaProps}, nil
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/e2e/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func WaitForIngress(s *Sandbox, ing *v1beta1.Ingress, options *WaitForIngressOpt
if options == nil || options.ExpectUnreachable {
return false, nil
}
return true, fmt.Errorf("Unexpected error from validation: %v", result.Err)
return true, fmt.Errorf("unexpected error from validation: %v", result.Err)
}
})
return ing, err
Expand All @@ -74,7 +74,7 @@ func WaitForIngress(s *Sandbox, ing *v1beta1.Ingress, options *WaitForIngressOpt
// resources associated with it to be deleted.
func WaitForIngressDeletion(ctx context.Context, g *fuzz.GCLB, s *Sandbox, ing *v1beta1.Ingress, options *fuzz.GCLBDeleteOptions) error {
if err := s.f.Clientset.Extensions().Ingresses(s.Namespace).Delete(ing.Name, &metav1.DeleteOptions{}); err != nil {
return fmt.Errorf("Delete(%q) = %v, want nil", ing.Name, err)
return fmt.Errorf("delete(%q) = %v, want nil", ing.Name, err)
}
glog.Infof("Waiting for GCLB resources to be deleted (%s/%s), IngressDeletionOptions=%+v", s.Namespace, ing.Name, options)
if err := WaitForGCLBDeletion(ctx, s.f.Cloud, g, options); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/fuzz/features/affinity.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,17 @@ func (v *affinityValidator) CheckResponse(host, path string, resp *http.Response

if backendConfig.Spec.SessionAffinity.AffinityType == "GENERATED_COOKIE" && !haveCookie {
return fuzz.CheckResponseContinue,
fmt.Errorf("Cookie based affinity is turned on but response did not contain a GCLB cookie")
fmt.Errorf("cookie based affinity is turned on but response did not contain a GCLB cookie")
}

if backendConfig.Spec.SessionAffinity.AffinityType == "NONE" && haveCookie {
return fuzz.CheckResponseContinue,
fmt.Errorf("Affinity is NONE but response contains a GCLB cookie")
fmt.Errorf("affinity is NONE but response contains a GCLB cookie")
}

if backendConfig.Spec.SessionAffinity.AffinityType == "CLIENT_IP" && haveCookie {
return fuzz.CheckResponseContinue,
fmt.Errorf("Affinity is CLIENT_IP but response contains a GCLB cookie")
fmt.Errorf("affinity is CLIENT_IP but response contains a GCLB cookie")
}

return fuzz.CheckResponseContinue, nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/fuzz/features/backendconfig_example.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (v *backendConfigExampleValidator) CheckResponse(host, path string, resp *h
hp := fuzz.HostPath{Host: host, Path: path}
b, ok := sm[hp]
if !ok {
return fuzz.CheckResponseContinue, fmt.Errorf("HostPath %v not found in Ingress", hp)
return fuzz.CheckResponseContinue, fmt.Errorf("hostPath %v not found in Ingress", hp)
}

serviceMap, err := v.env.Services()
Expand All @@ -80,7 +80,7 @@ func (v *backendConfigExampleValidator) CheckResponse(host, path string, resp *h

service, ok := serviceMap[b.ServiceName]
if !ok {
return fuzz.CheckResponseContinue, fmt.Errorf("Service %q not found in environment", b.ServiceName)
return fuzz.CheckResponseContinue, fmt.Errorf("service %q not found in environment", b.ServiceName)
}

anno := annotations.FromService(service)
Expand Down
4 changes: 2 additions & 2 deletions pkg/fuzz/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ func BackendConfigForPath(host, path string, ing *v1beta1.Ingress, env Validator
}
service, ok := serviceMap[b.ServiceName]
if !ok {
return nil, fmt.Errorf("Service %q not found in environment", b.ServiceName)
return nil, fmt.Errorf("service %q not found in environment", b.ServiceName)
}
servicePort := translatorutil.ServicePort(*service, b.ServicePort)
if servicePort == nil {
return nil, fmt.Errorf("Port %+v in Service %q not found", b.ServicePort, b.ServiceName)
return nil, fmt.Errorf("port %+v in Service %q not found", b.ServicePort, b.ServiceName)
}
bc, err := annotations.FromService(service).GetBackendConfigs()
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/healthchecks/healthchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func (h *HealthChecks) create(hc *HealthCheck) error {
}
return h.cloud.CreateHealthCheck(v1hc)
default:
return fmt.Errorf("Unknown Version: %q", hc.Version())
return fmt.Errorf("unknown Version: %q", hc.Version())
}
}

Expand All @@ -177,7 +177,7 @@ func (h *HealthChecks) update(oldHC, newHC *HealthCheck) error {
}
return h.cloud.UpdateHealthCheck(v1hc)
default:
return fmt.Errorf("Unknown Version: %q", newHC.Version())
return fmt.Errorf("unknown Version: %q", newHC.Version())

}
}
Expand Down Expand Up @@ -239,7 +239,7 @@ func (h *HealthChecks) Get(name string, version meta.Version) (*HealthCheck, err
}
hc, err = v1ToAlphaHealthCheck(v1hc)
default:
return nil, fmt.Errorf("Unknown version %v", version)
return nil, fmt.Errorf("unknown version %v", version)
}
return NewHealthCheck(hc), err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/instances/fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func (f *FakeInstanceGroups) SetNamedPortsOfInstanceGroup(igName, zone string, n
}
}
if ig == nil {
return fmt.Errorf("Failed to find instance group %q in zone %q", igName, zone)
return fmt.Errorf("failed to find instance group %q in zone %q", igName, zone)
}

ig.NamedPorts = namedPorts
Expand Down
2 changes: 1 addition & 1 deletion pkg/loadbalancers/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (l *L7) getSslCertificates(names []string) ([]*compute.SslCertificate, erro
result = append(result, cert)
}
if len(failedCerts) != 0 {
return result, fmt.Errorf("Errors - %s", strings.Join(failedCerts, ","))
return result, fmt.Errorf("errors - %s", strings.Join(failedCerts, ","))
}

return result, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/loadbalancers/fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ func (f *FakeLoadBalancers) CreateSslCertificate(cert *compute.SslCertificate) (
cert.SelfLink = cloud.NewSslCertificatesResourceID("mock-project", cert.Name).SelfLink(meta.VersionGA)
if len(f.Certs) == FakeCertLimit {
// Simulate cert creation failure
return nil, fmt.Errorf("Unable to create cert, Exceeded cert limit of %d.", FakeCertLimit)
return nil, fmt.Errorf("unable to create cert, Exceeded cert limit of %d.", FakeCertLimit)
}
f.Certs = append(f.Certs, cert)
return cert, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/neg/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (manager *syncerManager) GC() error {

// Garbage collect NEGs
if err := manager.garbageCollectNEG(); err != nil {
return fmt.Errorf("Failed to garbage collect negs: %v", err)
return fmt.Errorf("failed to garbage collect negs: %v", err)
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/neg/syncers/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ func (s *batchSyncer) toNetworkEndpointBatch(endpoints sets.String) ([]*compute.
ip, instance, port := decodeEndpoint(enc)
portNum, err := strconv.Atoi(port)
if err != nil {
return nil, fmt.Errorf("Failed to decode endpoint %q: %v", enc, err)
return nil, fmt.Errorf("failed to decode endpoint %q: %v", enc, err)
}
networkEndpointList[i] = &compute.NetworkEndpoint{
Instance: instance,
Expand Down
2 changes: 1 addition & 1 deletion pkg/neg/syncers/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func makeEndpointBatch(endpoints sets.String) (map[string]*compute.NetworkEndpoi
ip, instance, port := decodeEndpoint(encodedEndpoint)
portNum, err := strconv.Atoi(port)
if err != nil {
return nil, fmt.Errorf("Failed to decode endpoint %q: %v", encodedEndpoint, err)
return nil, fmt.Errorf("failed to decode endpoint %q: %v", encodedEndpoint, err)
}

endpointBatch[encodedEndpoint] = &compute.NetworkEndpoint{
Expand Down
2 changes: 1 addition & 1 deletion pkg/neg/types/fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func NewFakeNetworkEndpointGroupCloud(subnetwork, network string) NetworkEndpoin
}
}

var NotFoundError = fmt.Errorf("Not Found")
var NotFoundError = fmt.Errorf("not Found")

func (f *FakeNetworkEndpointGroupCloud) GetNetworkEndpointGroup(name string, zone string) (*computebeta.NetworkEndpointGroup, error) {
f.mu.Lock()
Expand Down
12 changes: 6 additions & 6 deletions pkg/ratelimit/ratelimit.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func NewGCERateLimiter(specs []string, operationPollInterval time.Duration) (*GC
for _, spec := range specs {
params := strings.Split(spec, ",")
if len(params) < 2 {
return nil, fmt.Errorf("Must at least specify operation and rate limiter type.")
return nil, fmt.Errorf("must at least specify operation and rate limiter type.")
}
// params[0] should consist of the operation to rate limit.
key, err := constructRateLimitKey(params[0])
Expand Down Expand Up @@ -121,7 +121,7 @@ func constructRateLimitKey(param string) (cloud.RateLimitKey, error) {
var retVal cloud.RateLimitKey
params := strings.Split(param, ".")
if len(params) != 3 {
return retVal, fmt.Errorf("Must specify rate limit in [version].[service].[operation] format: %v", param)
return retVal, fmt.Errorf("must specify rate limit in [version].[service].[operation] format: %v", param)
}
// TODO(rramkumar): Add another layer of validation here?
version := meta.Version(params[0])
Expand All @@ -144,17 +144,17 @@ func constructRateLimitImpl(params []string) (flowcontrol.RateLimiter, error) {
implArgs := params[1:]
if rlType == "qps" {
if len(implArgs) != 2 {
return nil, fmt.Errorf("Invalid number of args for rate limiter type %v. Expected %d, Got %v", rlType, 2, len(implArgs))
return nil, fmt.Errorf("invalid number of args for rate limiter type %v. Expected %d, Got %v", rlType, 2, len(implArgs))
}
qps, err := strconv.ParseFloat(implArgs[0], 32)
if err != nil || qps <= 0 {
return nil, fmt.Errorf("Invalid argument for rate limiter type %v. Either %v is not a float or not greater than 0.", rlType, implArgs[0])
return nil, fmt.Errorf("invalid argument for rate limiter type %v. Either %v is not a float or not greater than 0.", rlType, implArgs[0])
}
burst, err := strconv.Atoi(implArgs[1])
if err != nil {
return nil, fmt.Errorf("Invalid argument for rate limiter type %v. Expected %v to be a int.", rlType, implArgs[1])
return nil, fmt.Errorf("invalid argument for rate limiter type %v. Expected %v to be a int.", rlType, implArgs[1])
}
return flowcontrol.NewTokenBucketRateLimiter(float32(qps), burst), nil
}
return nil, fmt.Errorf("Invalid rate limiter type provided: %v", rlType)
return nil, fmt.Errorf("invalid rate limiter type provided: %v", rlType)
}
10 changes: 5 additions & 5 deletions pkg/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ func (s *IngressSyncer) Sync(state interface{}) error {
if err == ErrSkipBackendsSync {
return nil
}
return fmt.Errorf("Error running backend syncing routine: %v", err)
return fmt.Errorf("error running backend syncing routine: %v", err)
}

if err := s.controller.SyncLoadBalancer(state); err != nil {
return fmt.Errorf("Error running load balancer syncing routine: %v", err)
return fmt.Errorf("error running load balancer syncing routine: %v", err)
}

if err := s.controller.PostProcess(state); err != nil {
return fmt.Errorf("Error running post-process routine: %v", err)
return fmt.Errorf("error running post-process routine: %v", err)
}

return nil
Expand All @@ -61,10 +61,10 @@ func (s *IngressSyncer) GC(state interface{}) error {
lbErr := s.controller.GCLoadBalancers(state)
beErr := s.controller.GCBackends(state)
if lbErr != nil {
return fmt.Errorf("Error running load balancer garbage collection routine: %v", lbErr)
return fmt.Errorf("error running load balancer garbage collection routine: %v", lbErr)
}
if beErr != nil {
return fmt.Errorf("Error running backend garbage collection routine: %v", beErr)
return fmt.Errorf("error running backend garbage collection routine: %v", beErr)
}
return nil
}