diff --git a/cmd/preupgradechecks/preupgradechecks.go b/cmd/preupgradechecks/preupgradechecks.go index fc28742f..63825f29 100644 --- a/cmd/preupgradechecks/preupgradechecks.go +++ b/cmd/preupgradechecks/preupgradechecks.go @@ -218,5 +218,4 @@ func (client *PreUpgradeTaskClient) SetupRoleBindings() { client.logger.Info("created rolebindings in default namespace", zap.Strings("role_bindings", []string{types.PackageGetterRB, types.SecretConfigMapGetterRB})) - return } diff --git a/environments/binary/env.go b/environments/binary/env.go index 4cfd5e4e..2e66c2cd 100644 --- a/environments/binary/env.go +++ b/environments/binary/env.go @@ -38,10 +38,8 @@ func (e *Env) ToStringEnv() []string { func NewEnv(stringEnv []string) *Env { env := &Env{} - if stringEnv != nil { - for _, rawEnvVar := range stringEnv { - env.SetEnv(FromString(rawEnvVar)) - } + for _, rawEnvVar := range stringEnv { + env.SetEnv(FromString(rawEnvVar)) } return env } diff --git a/hack/travis-kube-setup.sh b/hack/travis-kube-setup.sh index 54308e05..d2ccca67 100755 --- a/hack/travis-kube-setup.sh +++ b/hack/travis-kube-setup.sh @@ -14,12 +14,12 @@ then fi # Get staticcheck -STATICCHECK_VERSION=2019.1.1 -if [ ! -f $TOOL_DIR/staticcheck ] +STATICCHECK_VERSION=2019.2.3 +if [ ! -f $TOOL_DIR/staticcheck ] || (staticcheck -version | grep -v $STATICCHECK_VERSION) then - curl -LO https://github.com/dominikh/go-tools/releases/download/${STATICCHECK_VERSION}/staticcheck_linux_amd64 - chmod +x staticcheck_linux_amd64 - mv staticcheck_linux_amd64 $TOOL_DIR/staticcheck + curl -LO https://github.com/dominikh/go-tools/releases/download/${STATICCHECK_VERSION}/staticcheck_linux_amd64.tar.gz + tar xzvf staticcheck_linux_amd64.tar.gz + mv staticcheck/staticcheck $TOOL_DIR/staticcheck fi K8SCLI_DIR=$HOME/k8scli diff --git a/hack/verify-staticcheck.sh b/hack/verify-staticcheck.sh index 88cad13b..8f257c4e 100755 --- a/hack/verify-staticcheck.sh +++ b/hack/verify-staticcheck.sh @@ -6,12 +6,4 @@ set -o pipefail ROOT=`realpath $(dirname $0)/..` -# Currently here only lists the packages that already addressed all warnings for gradual code repair. -# That means we don't allow adding new warnings to any of package in list. And eventually, the -# list will be replaced by find command. -declare -a pkgs=("pkg/builder" "pkg/builder" "pkg/crd" "pkg/logger" "pkg/buildermgr" "pkg/fission-cli" "cmd/fission-cli") - -for pkg in "${pkgs[@]}" -do - find ${ROOT}/${pkg} -type d |grep -v influxdb | xargs -I@ staticcheck @ -done +go list ./...| grep -v vendor | grep -v "examples" | grep -v "demos" | xargs -I@ staticcheck @ diff --git a/pkg/apis/fission.io/v1/validation.go b/pkg/apis/fission.io/v1/validation.go index 351e9a49..88b7b7d2 100644 --- a/pkg/apis/fission.io/v1/validation.go +++ b/pkg/apis/fission.io/v1/validation.go @@ -37,7 +37,7 @@ const ( ) var ( - validAzureQueueName = regexp.MustCompile("^[a-z0-9][a-z0-9\\-]*[a-z0-9]$") + validAzureQueueName = regexp.MustCompile(`^[a-z0-9][a-z0-9\\-]*[a-z0-9]$`) // Need to use raw string to support escape sequence for - & . chars validKafkaTopicName = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9\-\._]*[a-zA-Z0-9]$`) ) diff --git a/pkg/builder/builder.go b/pkg/builder/builder.go index d1816152..044e992b 100644 --- a/pkg/builder/builder.go +++ b/pkg/builder/builder.go @@ -74,7 +74,7 @@ func MakeBuilder(logger *zap.Logger, sharedVolumePath string) *Builder { func (builder *Builder) VersionHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json; charset=utf-8") - fmt.Fprintf(w, info.BuildInfo().String()) + w.Write([]byte(info.BuildInfo().String())) } func (builder *Builder) Handler(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/buildermgr/pkgwatcher.go b/pkg/buildermgr/pkgwatcher.go index df9af006..280ee350 100644 --- a/pkg/buildermgr/pkgwatcher.go +++ b/pkg/buildermgr/pkgwatcher.go @@ -84,7 +84,7 @@ func (pkgw *packageWatcher) build(buildCache *cache.Cache, srcpkg *fv1.Package) // Ignore duplicate build requests key := fmt.Sprintf("%v-%v", srcpkg.Metadata.Name, srcpkg.Metadata.ResourceVersion) - err, _ := buildCache.Set(key, srcpkg) + _, err := buildCache.Set(key, srcpkg) if err != nil { return } diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 387a6092..f5140789 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -159,7 +159,7 @@ func (c *Cache) Get(key interface{}) (interface{}, error) { // if key exists in the cache, the new value is NOT set; instead an // error and the old value are returned -func (c *Cache) Set(key interface{}, value interface{}) (error, interface{}) { +func (c *Cache) Set(key interface{}, value interface{}) (interface{}, error) { respChannel := make(chan *response) c.requestChannel <- &request{ requestType: SET, @@ -168,7 +168,7 @@ func (c *Cache) Set(key interface{}, value interface{}) (error, interface{}) { responseChannel: respChannel, } resp := <-respChannel - return resp.error, resp.existingValue + return resp.existingValue, resp.error } func (c *Cache) Delete(key interface{}) error { diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 83528b64..9941bb2e 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -31,9 +31,9 @@ func checkErr(err error) { func TestCache(t *testing.T) { c := MakeCache(100*time.Millisecond, 100*time.Millisecond) - err, _ := c.Set("a", "b") + _, err := c.Set("a", "b") checkErr(err) - err, _ = c.Set("p", "q") + _, err = c.Set("p", "q") checkErr(err) val, err := c.Get("a") @@ -55,7 +55,7 @@ func TestCache(t *testing.T) { log.Panicf("found deleted element") } - err, _ = c.Set("expires", "42") + _, err = c.Set("expires", "42") checkErr(err) time.Sleep(150 * time.Millisecond) _, err = c.Get("expires") diff --git a/pkg/canaryconfigmgr/canaryConfigCache.go b/pkg/canaryconfigmgr/canaryConfigCache.go index 4098e386..75d01619 100644 --- a/pkg/canaryconfigmgr/canaryConfigCache.go +++ b/pkg/canaryconfigmgr/canaryConfigCache.go @@ -68,7 +68,7 @@ func (cancelFuncMap *canaryConfigCancelFuncMap) lookup(f *metav1.ObjectMeta) (*C func (cancelFuncMap *canaryConfigCancelFuncMap) assign(f *metav1.ObjectMeta, value *CanaryProcessingInfo) error { mk := keyFromMetadata(f) - err, _ := cancelFuncMap.cache.Set(mk, value) + _, err := cancelFuncMap.cache.Set(mk, value) return err } diff --git a/pkg/canaryconfigmgr/canaryConfigMgr.go b/pkg/canaryconfigmgr/canaryConfigMgr.go index 68bae94f..dfe6417e 100644 --- a/pkg/canaryconfigmgr/canaryConfigMgr.go +++ b/pkg/canaryconfigmgr/canaryConfigMgr.go @@ -443,6 +443,9 @@ func (canaryCfgMgr *canaryConfigMgr) rollback(canaryConfig *fv1.CanaryConfig, tr functionWeights[canaryConfig.Spec.OldFunction] = 100 err := canaryCfgMgr.updateHttpTriggerWithRetries(trigger.Metadata.Name, trigger.Metadata.Namespace, functionWeights) + if err != nil { + return err + } err = canaryCfgMgr.updateCanaryConfigStatusWithRetries(canaryConfig.Metadata.Name, canaryConfig.Metadata.Namespace, types.CanaryConfigStatusFailed) diff --git a/pkg/controller/api.go b/pkg/controller/api.go index 051b8043..8aab2875 100644 --- a/pkg/controller/api.go +++ b/pkg/controller/api.go @@ -53,7 +53,6 @@ type ( builderManagerUrl string workflowApiUrl string functionNamespace string - useIstio bool featureStatus map[string]string } @@ -167,7 +166,7 @@ func (api *API) getLogDBConfig(dbType string) logDBConfig { func (api *API) HomeHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json; charset=utf-8") - fmt.Fprintf(w, info.ApiInfo().String()) + w.Write([]byte(info.ApiInfo().String())) } func (api *API) ApiVersionMismatchHandler(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/controller/client/client.go b/pkg/controller/client/client.go index f8738a5f..6eb1ae09 100644 --- a/pkg/controller/client/client.go +++ b/pkg/controller/client/client.go @@ -20,13 +20,13 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io/ioutil" "net/http" "strings" "time" + "github.com/pkg/errors" "golang.org/x/net/context/ctxhttp" ferror "github.com/fission/fission/pkg/error" @@ -58,9 +58,9 @@ func (c *Client) delete(relativeUrl string) error { if resp.StatusCode != 200 { body, err := ioutil.ReadAll(resp.Body) if err != nil { - return errors.New("Delete failed") + return errors.Wrap(err, "error deleting") } else { - return errors.New("Delete failed: " + string(body)) + return errors.Errorf("failed to delete: %v", string(body)) } } diff --git a/pkg/controller/client/core.go b/pkg/controller/client/core.go index 585d2dc7..e0512145 100644 --- a/pkg/controller/client/core.go +++ b/pkg/controller/client/core.go @@ -22,6 +22,8 @@ import ( "io/ioutil" "net/http" + "github.com/pkg/errors" + apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -84,7 +86,7 @@ func (c *Client) GetSvcURL(label string) (string, error) { } if resp == nil { - return "", fmt.Errorf("Failed to find service for given label: %v", label) + return "", errors.Errorf("failed to find service for given label: %v", label) } defer resp.Body.Close() diff --git a/pkg/controller/functionApi.go b/pkg/controller/functionApi.go index b78244d4..6d3b467e 100644 --- a/pkg/controller/functionApi.go +++ b/pkg/controller/functionApi.go @@ -106,12 +106,6 @@ func RegisterFunctionRoute(ws *restful.WebService) { Returns(http.StatusOK, "Only HTTP status returned", nil)) } -func (a *API) getIstioServiceLabels(fnName string) map[string]string { - return map[string]string{ - "functionName": fnName, - } -} - func (a *API) FunctionApiList(w http.ResponseWriter, r *http.Request) { ns := a.extractQueryParamFromRequest(r, "namespace") if len(ns) == 0 { @@ -325,7 +319,7 @@ func (a *API) FunctionPodLogs(w http.ResponseWriter, r *http.Request) { if len(pods) > 0 { podLogsReq = a.kubernetesClient.CoreV1().Pods(ns).GetLogs(pods[0].ObjectMeta.Name, &podLogOpts) } else { - a.respondWithError(w, errors.New("No active pods found")) + a.respondWithError(w, errors.New("no active pods found")) return } @@ -341,5 +335,4 @@ func (a *API) FunctionPodLogs(w http.ResponseWriter, r *http.Request) { a.respondWithError(w, err) return } - return } diff --git a/pkg/executor/executor_test.go b/pkg/executor/executor_test.go index e80350a5..182422f9 100644 --- a/pkg/executor/executor_test.go +++ b/pkg/executor/executor_test.go @@ -25,10 +25,8 @@ package executor import ( "context" "fmt" - "io/ioutil" "log" "math/rand" - "net/http" "os" "testing" "time" @@ -99,19 +97,6 @@ func createSvc(kubeClient *kubernetes.Clientset, ns string, name string, targetP return svc } -func httpGet(url string) string { - resp, err := http.Get(url) - if err != nil { - log.Panicf("HTTP Get failed: URL %v: %v", url, err) - } - defer resp.Body.Close() - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - log.Panicf("HTTP Get failed to read body: URL %v: %v", url, err) - } - return string(body) -} - func TestExecutor(t *testing.T) { // run in a random namespace so we can have concurrent tests // on a given cluster diff --git a/pkg/executor/fscache/functionServiceCache.go b/pkg/executor/fscache/functionServiceCache.go index 0d0dcb43..85554e1d 100644 --- a/pkg/executor/fscache/functionServiceCache.go +++ b/pkg/executor/fscache/functionServiceCache.go @@ -68,15 +68,13 @@ type ( requestChannel chan *fscRequest } fscRequest struct { - requestType fscRequestType - address string - kubernetesObjects []apiv1.ObjectReference - age time.Duration - responseChannel chan *fscResponse + requestType fscRequestType + address string + age time.Duration + responseChannel chan *fscResponse } fscResponse struct { objects []*FuncSvc - deleted bool error } ) @@ -180,7 +178,7 @@ func (fsc *FunctionServiceCache) GetByFunctionUID(uid types.UID) (*FuncSvc, erro } func (fsc *FunctionServiceCache) Add(fsvc FuncSvc) (*FuncSvc, error) { - err, existing := fsc.byFunction.Set(crd.CacheKey(fsvc.Function), &fsvc) + existing, err := fsc.byFunction.Set(crd.CacheKey(fsvc.Function), &fsvc) if err != nil { if IsNameExistError(err) { f := existing.(*FuncSvc) @@ -199,7 +197,7 @@ func (fsc *FunctionServiceCache) Add(fsvc FuncSvc) (*FuncSvc, error) { // Add to byAddress cache. Ignore NameExists errors // because of multiple-specialization. See issue #331. - err, _ = fsc.byAddress.Set(fsvc.Address, *fsvc.Function) + _, err = fsc.byAddress.Set(fsvc.Address, *fsvc.Function) if err != nil { if IsNameExistError(err) { err = nil @@ -211,7 +209,7 @@ func (fsc *FunctionServiceCache) Add(fsvc FuncSvc) (*FuncSvc, error) { // Add to byFunctionUID cache. Ignore NameExists errors // because of multiple-specialization. See issue #331. - err, _ = fsc.byFunctionUID.Set(fsvc.Function.UID, *fsvc.Function) + _, err = fsc.byFunctionUID.Set(fsvc.Function.UID, *fsvc.Function) if err != nil { if IsNameExistError(err) { err = nil @@ -257,7 +255,7 @@ func (fsc *FunctionServiceCache) DeleteEntry(fsvc *FuncSvc) { fsc.byFunctionUID.Delete(fsvc.Function.UID) fsc.observeFuncRunningTime(fsvc.Function.Name, string(fsvc.Function.UID), fsvc.Atime.Sub(fsvc.Ctime).Seconds()) - fsc.observeFuncAliveTime(fsvc.Function.Name, string(fsvc.Function.UID), time.Now().Sub(fsvc.Ctime).Seconds()) + fsc.observeFuncAliveTime(fsvc.Function.Name, string(fsvc.Function.UID), time.Since(fsvc.Ctime).Seconds()) fsc.setFuncAlive(fsvc.Function.Name, string(fsvc.Function.UID), false) } diff --git a/pkg/executor/fscache/functionServiceCache_test.go b/pkg/executor/fscache/functionServiceCache_test.go index 1f3deb89..e0211805 100644 --- a/pkg/executor/fscache/functionServiceCache_test.go +++ b/pkg/executor/fscache/functionServiceCache_test.go @@ -74,12 +74,12 @@ func TestFunctionServiceCache(t *testing.T) { log.Panicf("Failed to add fsvc: %v", err) } - f, err := fsc.GetByFunction(fsvc.Function) + _, err = fsc.GetByFunction(fsvc.Function) if err != nil { fsc.Log() log.Panicf("Failed to get fsvc: %v", err) } - f, err = fsc.GetByFunctionUID(fsvc.Function.UID) + f, err := fsc.GetByFunctionUID(fsvc.Function.UID) if err != nil { fsc.Log() log.Panicf("Failed to get fsvc by function uid: %v", err) diff --git a/pkg/executor/fscache/metrics.go b/pkg/executor/fscache/metrics.go index a66e1689..cc8a7b39 100644 --- a/pkg/executor/fscache/metrics.go +++ b/pkg/executor/fscache/metrics.go @@ -5,8 +5,6 @@ import ( ) var ( - metricAddr = ":8080" - // funcname: the function's name // funcuid: the function's version id coldStarts = prometheus.NewCounterVec( diff --git a/pkg/executor/newdeploy/newdeploy.go b/pkg/executor/newdeploy/newdeploy.go index 6f872419..8ab24d46 100644 --- a/pkg/executor/newdeploy/newdeploy.go +++ b/pkg/executor/newdeploy/newdeploy.go @@ -143,10 +143,6 @@ func (deploy *NewDeploy) setupRBACObjs(deployNamespace string, fn *fv1.Function) return nil } -func (deploy *NewDeploy) getDeployment(ns, name string) (*appsv1.Deployment, error) { - return deploy.kubernetesClient.AppsV1().Deployments(ns).Get(name, metav1.GetOptions{}) -} - func (deploy *NewDeploy) updateDeployment(deployment *appsv1.Deployment, ns string) error { _, err := deploy.kubernetesClient.AppsV1().Deployments(ns).Update(deployment) return err diff --git a/pkg/executor/newdeploy/newdeploymgr.go b/pkg/executor/newdeploy/newdeploymgr.go index 71e7fbb9..5583b2e7 100644 --- a/pkg/executor/newdeploy/newdeploymgr.go +++ b/pkg/executor/newdeploy/newdeploymgr.go @@ -61,7 +61,6 @@ type ( runtimeImagePullPolicy apiv1.PullPolicy namespace string useIstio bool - collectorEndpoint string fsCache *fscache.FunctionServiceCache // cache funcSvc's by function, address and pod name @@ -523,7 +522,7 @@ func (deploy *NewDeploy) updateFunction(oldFn *fv1.Function, newFn *fv1.Function } } - if deployChanged == true { + if deployChanged { env, err := deploy.fissionClient.Environments(newFn.Spec.Environment.Namespace). Get(newFn.Spec.Environment.Name) if err != nil { @@ -624,20 +623,6 @@ func (deploy *NewDeploy) getDeployLabels(fnMeta metav1.ObjectMeta, envMeta metav } } -// updateKubeObjRefRV update the resource version of kubeObjectRef with -// given kind and return error if failed to find the reference. -func (deploy *NewDeploy) updateKubeObjRefRV(fsvc *fscache.FuncSvc, objKind string, rv string) error { - kubeObjs := fsvc.KubernetesObjects - for i, obj := range kubeObjs { - if obj.Kind == objKind { - kubeObjs[i].ResourceVersion = rv - return nil - } - } - fsvc.KubernetesObjects = kubeObjs - return fmt.Errorf("error finding kubernetes object reference with kind: %v", objKind) -} - // updateStatus is a function which updates status of update. // Current implementation only logs messages, in future it will update function status func (deploy *NewDeploy) updateStatus(fn *fv1.Function, err error, message string) { diff --git a/pkg/executor/poolmgr/gp.go b/pkg/executor/poolmgr/gp.go index 2befc087..f90657d8 100644 --- a/pkg/executor/poolmgr/gp.go +++ b/pkg/executor/poolmgr/gp.go @@ -154,16 +154,13 @@ func (gp *GenericPool) getDeployLabels() map[string]string { // choosePodService serializes the choosing of pods func (gp *GenericPool) choosePodService() { - for { - select { - case req := <-gp.requestChannel: - pod, err := gp._choosePod(req.newLabels) - if err != nil { - req.responseChannel <- &choosePodResponse{error: err} - continue - } - req.responseChannel <- &choosePodResponse{pod: pod} + for req := range gp.requestChannel { + pod, err := gp._choosePod(req.newLabels) + if err != nil { + req.responseChannel <- &choosePodResponse{error: err} + continue } + req.responseChannel <- &choosePodResponse{pod: pod} } } @@ -287,11 +284,13 @@ func (gp *GenericPool) getFetcherUrl(podIP string) string { } isv6 := IsIPv6(podIP) var baseUrl string - if isv6 == false { - baseUrl = fmt.Sprintf("http://%v:8000/", podIP) - } else if isv6 == true { // We use bracket if the IP is in IPv6. + + if isv6 { // We use bracket if the IP is in IPv6. baseUrl = fmt.Sprintf("http://[%v]:8000/", podIP) + } else { + baseUrl = fmt.Sprintf("http://%v:8000/", podIP) } + return baseUrl } @@ -477,7 +476,7 @@ func (gp *GenericPool) waitForReadyPod() error { pod := podList.Items[0] multierr := &multierror.Error{} for _, cStatus := range pod.Status.ContainerStatuses { - if cStatus.Ready != true { + if !cStatus.Ready { multierr = multierror.Append(multierr, errors.New(fmt.Sprintf("%v: %v", cStatus.State.Waiting.Reason, cStatus.State.Waiting.Message))) } } diff --git a/pkg/executor/util/merge.go b/pkg/executor/util/merge.go index 85bbab6b..e69144ae 100644 --- a/pkg/executor/util/merge.go +++ b/pkg/executor/util/merge.go @@ -120,17 +120,9 @@ func MergePodSpec(srcPodSpec *apiv1.PodSpec, targetPodSpec *apiv1.PodSpec) (*api srcPodSpec.Hostname = targetPodSpec.Hostname } - for _, obj := range targetPodSpec.ImagePullSecrets { - srcPodSpec.ImagePullSecrets = append(srcPodSpec.ImagePullSecrets, obj) - } - - for _, obj := range targetPodSpec.Tolerations { - srcPodSpec.Tolerations = append(srcPodSpec.Tolerations, obj) - } - - for _, obj := range targetPodSpec.HostAliases { - srcPodSpec.HostAliases = append(srcPodSpec.HostAliases, obj) - } + srcPodSpec.ImagePullSecrets = append(srcPodSpec.ImagePullSecrets, targetPodSpec.ImagePullSecrets...) + srcPodSpec.Tolerations = append(srcPodSpec.Tolerations, targetPodSpec.Tolerations...) + srcPodSpec.HostAliases = append(srcPodSpec.HostAliases, targetPodSpec.HostAliases...) err = mergo.Merge(&srcPodSpec.NodeSelector, targetPodSpec.NodeSelector) if err != nil { diff --git a/pkg/fetcher/config/config.go b/pkg/fetcher/config/config.go index 15533cbc..fd3c69e6 100644 --- a/pkg/fetcher/config/config.go +++ b/pkg/fetcher/config/config.go @@ -7,8 +7,7 @@ import ( "os" "path/filepath" - "github.com/fission/fission/pkg/types" - "github.com/fission/fission/pkg/utils" + "github.com/pkg/errors" apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -16,6 +15,8 @@ import ( "k8s.io/client-go/kubernetes" fv1 "github.com/fission/fission/pkg/apis/fission.io/v1" + "github.com/fission/fission/pkg/types" + "github.com/fission/fission/pkg/utils" ) type Config struct { @@ -30,9 +31,9 @@ type Config struct { sharedSecretPath string sharedCfgMapPath string - dockerRegistryAuthDomain string - dockerRegistryUsername string - dockerRegistryPassword string + // dockerRegistryAuthDomain string + // dockerRegistryUsername string + // dockerRegistryPassword string serviceAccount string @@ -289,7 +290,7 @@ func (cfg *Config) addFetcherToPodSpecWithCommand(podSpec *apiv1.PodSpec, mainCo for _, existingContainer := range podSpec.Containers { existingContainerNames = append(existingContainerNames, existingContainer.Name) } - return fmt.Errorf("Could not find main container '%s' in given PodSpec. Found: %v", + return errors.Errorf("could not find main container '%s' in given PodSpec. Found: %v", mainContainerName, existingContainerNames) } diff --git a/pkg/fetcher/fetcher.go b/pkg/fetcher/fetcher.go index 0531dbcf..4bb5b0be 100644 --- a/pkg/fetcher/fetcher.go +++ b/pkg/fetcher/fetcher.go @@ -151,7 +151,7 @@ func writeSecretOrConfigMap(dataMap map[string][]byte, dirPath string) error { func (fetcher *Fetcher) VersionHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json; charset=utf-8") - fmt.Fprintf(w, info.BuildInfo().String()) + w.Write([]byte(info.BuildInfo().String())) } func (fetcher *Fetcher) FetchHandler(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/kubewatcher/kubewatcher.go b/pkg/kubewatcher/kubewatcher.go index 7e05e6e2..bf44db7e 100644 --- a/pkg/kubewatcher/kubewatcher.go +++ b/pkg/kubewatcher/kubewatcher.go @@ -54,7 +54,6 @@ type ( kubernetesClient *kubernetes.Clientset requestChannel chan *kubeWatcherRequest publisher publisher.Publisher - routerUrl string } watchSubscription struct { diff --git a/pkg/mqtrigger/messageQueue/kafka.go b/pkg/mqtrigger/messageQueue/kafka.go index 32114a86..6f91f79d 100644 --- a/pkg/mqtrigger/messageQueue/kafka.go +++ b/pkg/mqtrigger/messageQueue/kafka.go @@ -69,7 +69,7 @@ func makeKafkaMessageQueue(logger *zap.Logger, routerUrl string, mqCfg MessageQu version: kafkaVersion, } - if tls, _ := strconv.ParseBool(os.Getenv("TLS_ENABLED")); tls == true { + if tls, _ := strconv.ParseBool(os.Getenv("TLS_ENABLED")); tls { kafka.tls = true authKeys := make(map[string][]byte) diff --git a/pkg/mqtrigger/messageQueue/messageQueue.go b/pkg/mqtrigger/messageQueue/messageQueue.go index 9f9597c4..79303428 100644 --- a/pkg/mqtrigger/messageQueue/messageQueue.go +++ b/pkg/mqtrigger/messageQueue/messageQueue.go @@ -55,7 +55,6 @@ type ( MessageQueueTriggerManager struct { logger *zap.Logger reqChan chan request - mqCfg MessageQueueConfig triggers map[string]*triggerSubscription fissionClient *crd.FissionClient messageQueue MessageQueue diff --git a/pkg/router/functionHandler.go b/pkg/router/functionHandler.go index 1bf6c68b..1e05d9cb 100644 --- a/pkg/router/functionHandler.go +++ b/pkg/router/functionHandler.go @@ -154,8 +154,7 @@ func (roundTripper RetryingRoundTripper) RoundTrip(req *http.Request) (*http.Res roundTripper.addForwardedHostHeader(req) // TODO: Keep? --> Needed for queries encoded in URL before they're stripped by the proxy - var originalUrl url.URL - originalUrl = *req.URL + originalUrl := *req.URL // Iff this request needs to be recorded, we save the body var postedBody string @@ -245,7 +244,7 @@ func (roundTripper RetryingRoundTripper) RoundTrip(req *http.Request) (*http.Res Body: ioutil.NopCloser(bytes.NewBufferString(errMsg)), ContentLength: int64(len(errMsg)), Request: req, - Header: make(http.Header, 0), + Header: make(http.Header), }, nil } return nil, ferror.MakeError(http.StatusInternalServerError, err.Error()) diff --git a/pkg/router/functionRecorderMap.go b/pkg/router/functionRecorderMap.go index a20a2a08..880cfbc7 100644 --- a/pkg/router/functionRecorderMap.go +++ b/pkg/router/functionRecorderMap.go @@ -51,7 +51,7 @@ func (frmap *functionRecorderMap) lookup(function string) (*fv1.Recorder, error) } func (frmap *functionRecorderMap) assign(function string, recorder *fv1.Recorder) { - err, _ := frmap.cache.Set(function, recorder) + _, err := frmap.cache.Set(function, recorder) if err != nil { if e, ok := err.(ferror.Error); ok && e.Code == ferror.ErrorNameExists { return diff --git a/pkg/router/functionReferenceResolver.go b/pkg/router/functionReferenceResolver.go index 0f5c970f..7a760828 100644 --- a/pkg/router/functionReferenceResolver.go +++ b/pkg/router/functionReferenceResolver.go @@ -20,11 +20,8 @@ import ( "fmt" "time" + "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/fields" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/watch" - "k8s.io/client-go/rest" k8sCache "k8s.io/client-go/tools/cache" fv1 "github.com/fission/fission/pkg/apis/fission.io/v1" @@ -37,9 +34,7 @@ type ( functionReferenceResolver struct { // FunctionReference -> function metadata refCache *cache.Cache - - stopCh chan struct{} - store k8sCache.Store + store k8sCache.Store } resolveResultType int @@ -81,21 +76,6 @@ func makeFunctionReferenceResolver(store k8sCache.Store) *functionReferenceResol return frr } -func makeK8SCache(crdClient *rest.RESTClient) (k8sCache.Store, k8sCache.Controller) { - watchlist := k8sCache.NewListWatchFromClient(crdClient, "functions", metav1.NamespaceDefault, fields.Everything()) - listWatch := &k8sCache.ListWatch{ - ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - return watchlist.List(options) - }, - WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - return watchlist.Watch(options) - }, - } - resyncPeriod := 30 * time.Second - return k8sCache.NewInformer(listWatch, &fv1.Function{}, resyncPeriod, - k8sCache.ResourceEventHandlerFuncs{}) -} - // resolve translates a trigger's function reference to a resolveResult. func (frr *functionReferenceResolver) resolve(trigger fv1.HTTPTrigger) (*resolveResult, error) { nfr := namespacedTriggerReference{ @@ -128,7 +108,7 @@ func (frr *functionReferenceResolver) resolve(trigger fv1.HTTPTrigger) (*resolve } default: - return nil, fmt.Errorf("Unrecognized function reference type %v", trigger.Spec.FunctionReference.Type) + return nil, errors.Errorf("unrecognized function reference type %v", trigger.Spec.FunctionReference.Type) } // cache resolve result @@ -150,7 +130,7 @@ func (frr *functionReferenceResolver) resolveByName(namespace, name string) (*re return nil, err } if !isExist { - return nil, fmt.Errorf("function %v does not exist", name) + return nil, errors.Errorf("function %v does not exist", name) } f := obj.(*fv1.Function) @@ -167,7 +147,7 @@ func (frr *functionReferenceResolver) resolveByName(namespace, name string) (*re func (frr *functionReferenceResolver) resolveByFunctionWeights(namespace string, fr *fv1.FunctionReference) (*resolveResult, error) { - functionMetadataMap := make(map[string]*metav1.ObjectMeta, 0) + functionMetadataMap := make(map[string]*metav1.ObjectMeta) fnWtDistrList := make([]FunctionWeightDistribution, 0) sumPrefix := 0 diff --git a/pkg/router/functionServiceMap.go b/pkg/router/functionServiceMap.go index 9e99c79d..2cae359a 100644 --- a/pkg/router/functionServiceMap.go +++ b/pkg/router/functionServiceMap.go @@ -68,7 +68,7 @@ func (fmap *functionServiceMap) lookup(f *metav1.ObjectMeta) (*url.URL, error) { func (fmap *functionServiceMap) assign(f *metav1.ObjectMeta, serviceUrl *url.URL) { mk := keyFromMetadata(f) - err, old := fmap.cache.Set(*mk, serviceUrl) + old, err := fmap.cache.Set(*mk, serviceUrl) if err != nil { if *serviceUrl == *(old.(*url.URL)) { return diff --git a/pkg/router/ingress.go b/pkg/router/ingress.go index fca28620..9bd5401d 100644 --- a/pkg/router/ingress.go +++ b/pkg/router/ingress.go @@ -72,12 +72,12 @@ func deleteIngress(logger *zap.Logger, trigger *fv1.HTTPTrigger, kubeClient *kub func updateIngress(logger *zap.Logger, oldT *fv1.HTTPTrigger, newT *fv1.HTTPTrigger, kubeClient *kubernetes.Clientset) { - if oldT.Spec.CreateIngress == false && newT.Spec.CreateIngress == true { + if !oldT.Spec.CreateIngress && newT.Spec.CreateIngress { createIngress(logger, newT, kubeClient) return } - if newT.Spec.CreateIngress == false && oldT.Spec.CreateIngress == true { + if !newT.Spec.CreateIngress && oldT.Spec.CreateIngress { deleteIngress(logger, oldT, kubeClient) return } diff --git a/pkg/router/mutablemux_test.go b/pkg/router/mutablemux_test.go index 97ff9dd8..70edb394 100644 --- a/pkg/router/mutablemux_test.go +++ b/pkg/router/mutablemux_test.go @@ -47,7 +47,7 @@ func spamServer(quit chan bool) { for { select { case <-quit: - break + return default: i = i + 1 resp, err := http.Get("http://localhost:3333") diff --git a/pkg/router/recorderController.go b/pkg/router/recorderController.go index e51e55f1..a299af79 100644 --- a/pkg/router/recorderController.go +++ b/pkg/router/recorderController.go @@ -113,7 +113,7 @@ func (rs *RecorderSet) disableRecorder(r *fv1.Recorder) { } func (rs *RecorderSet) updateRecorder(old *fv1.Recorder, newer *fv1.Recorder) { - if newer.Spec.Enabled == true { + if newer.Spec.Enabled { rs.newRecorder(newer) // TODO: Test this } else { rs.disableRecorder(old) diff --git a/pkg/router/triggerRecorderMap.go b/pkg/router/triggerRecorderMap.go index 8284b2d2..447fb1e0 100644 --- a/pkg/router/triggerRecorderMap.go +++ b/pkg/router/triggerRecorderMap.go @@ -50,7 +50,7 @@ func (trmap *triggerRecorderMap) lookup(trigger string) (*fv1.Recorder, error) { } func (trmap *triggerRecorderMap) assign(trigger string, recorder *fv1.Recorder) { - err, _ := trmap.cache.Set(trigger, recorder) + _, err := trmap.cache.Set(trigger, recorder) if err != nil { if e, ok := err.(ferror.Error); ok && e.Code == ferror.ErrorNameExists { return diff --git a/pkg/storagesvc/archivePruner.go b/pkg/storagesvc/archivePruner.go index db5aedbf..c1ec23a3 100644 --- a/pkg/storagesvc/archivePruner.go +++ b/pkg/storagesvc/archivePruner.go @@ -28,7 +28,7 @@ import ( type ArchivePruner struct { logger *zap.Logger crdClient *crd.FissionClient - archiveChan chan (string) + archiveChan chan string stowClient *StowClient pruneInterval time.Duration } @@ -53,18 +53,15 @@ func MakeArchivePruner(logger *zap.Logger, stowClient *StowClient, pruneInterval // pruneArchives listens to archiveChannel for archive ids that need to be deleted func (pruner *ArchivePruner) pruneArchives() { pruner.logger.Debug("listening to archiveChannel to prune archives") - for { - select { - case archiveID := <-pruner.archiveChan: - pruner.logger.Info("sending delete request for archive", + for archiveID := range pruner.archiveChan { + pruner.logger.Info("sending delete request for archive", + zap.String("archive_id", archiveID)) + if err := pruner.stowClient.removeFileByID(archiveID); err != nil { + // logging the error and continuing with other deletions. + // hopefully this archive will be deleted in the next iteration. + pruner.logger.Error("ignoring error while deleting archive", + zap.Error(err), zap.String("archive_id", archiveID)) - if err := pruner.stowClient.removeFileByID(archiveID); err != nil { - // logging the error and continuing with other deletions. - // hopefully this archive will be deleted in the next iteration. - pruner.logger.Error("ignoring error while deleting archive", - zap.Error(err), - zap.String("archive_id", archiveID)) - } } } } @@ -134,8 +131,6 @@ func (pruner *ArchivePruner) getOrphanArchives() { for _, archiveID = range orphanedArchives { pruner.insertArchive(archiveID) } - - return } // Start starts a go routine that listens to a channel for archive IDs that need to deleted. @@ -144,12 +139,9 @@ func (pruner *ArchivePruner) getOrphanArchives() { func (pruner *ArchivePruner) Start() { ticker := time.NewTicker(pruner.pruneInterval * time.Minute) go pruner.pruneArchives() - for { - select { - case <-ticker.C: - // This method fetches unused archive IDs and sends them to archiveChannel for deletion - // silencing the errors, hoping they go away in next iteration. - pruner.getOrphanArchives() - } + for range ticker.C { + // This method fetches unused archive IDs and sends them to archiveChannel for deletion + // silencing the errors, hoping they go away in next iteration. + pruner.getOrphanArchives() } } diff --git a/pkg/storagesvc/client/client.go b/pkg/storagesvc/client/client.go index 6ffab97c..40814cf4 100644 --- a/pkg/storagesvc/client/client.go +++ b/pkg/storagesvc/client/client.go @@ -20,7 +20,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io" "io/ioutil" @@ -30,6 +29,7 @@ import ( "os" "strings" + "github.com/pkg/errors" "go.opencensus.io/plugin/ochttp" "golang.org/x/net/context/ctxhttp" @@ -127,7 +127,7 @@ func (c *Client) Download(ctx context.Context, id string, filePath string) error // quit if file exists _, err := os.Stat(filePath) if err == nil || !os.IsNotExist(err) { - return errors.New(fmt.Sprintf("file already exists: %v", filePath)) + return errors.Errorf("file already exists: %v", filePath) } // create @@ -175,7 +175,7 @@ func (c *Client) Delete(ctx context.Context, id string) error { defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return errors.New(fmt.Sprintf("HTTP error %v", resp.StatusCode)) + return errors.Errorf("HTTP error %v", resp.StatusCode) } return nil diff --git a/pkg/storagesvc/storagesvc.go b/pkg/storagesvc/storagesvc.go index 94e719c0..d1422143 100644 --- a/pkg/storagesvc/storagesvc.go +++ b/pkg/storagesvc/storagesvc.go @@ -18,7 +18,6 @@ package storagesvc import ( "encoding/json" - "errors" "fmt" "net/http" "os" @@ -27,6 +26,7 @@ import ( "github.com/gorilla/mux" _ "github.com/graymeta/stow/local" + "github.com/pkg/errors" "go.opencensus.io/plugin/ochttp" "go.uber.org/zap" ) @@ -109,7 +109,7 @@ func (ss *StorageService) getIdFromRequest(r *http.Request) (string, error) { values := r.URL.Query() ids, ok := values["id"] if !ok || len(ids) == 0 { - return "", errors.New("Missing `id' query param") + return "", errors.New("missing `id' query param") } return ids[0], nil } diff --git a/pkg/utils/rbacutils.go b/pkg/utils/rbacutils.go index 8a370abe..34bca0d9 100644 --- a/pkg/utils/rbacutils.go +++ b/pkg/utils/rbacutils.go @@ -136,7 +136,7 @@ func AddSaToRoleBindingWithRetries(logger *zap.Logger, k8sClient *kubernetes.Cli // someone may have deleted the object between us checking if the object is present and deciding to patch // so just create the object again rbObj := makeRoleBindingObj(roleBinding, roleBindingNs, role, roleKind, sa, saNamespace) - rbObj, err = k8sClient.RbacV1beta1().RoleBindings(roleBindingNs).Create(rbObj) + _, err = k8sClient.RbacV1beta1().RoleBindings(roleBindingNs).Create(rbObj) if err == nil { logger.Debug("created rolebinding", zap.String("role_binding", roleBinding), @@ -263,7 +263,7 @@ func SetupRoleBinding(logger *zap.Logger, k8sClient *kubernetes.Clientset, roleB zap.String("role_binding", roleBinding), zap.String("role_binding_namespace", roleBindingNs)) rbObj = makeRoleBindingObj(roleBinding, roleBindingNs, role, roleKind, sa, saNamespace) - rbObj, err = k8sClient.RbacV1beta1().RoleBindings(roleBindingNs).Create(rbObj) + _, err = k8sClient.RbacV1beta1().RoleBindings(roleBindingNs).Create(rbObj) if k8serrors.IsAlreadyExists(err) { logger.Debug("rolebinding already exists in namespace - adding service account to rolebinding", zap.String("service_account_name", sa), diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index fec6fad4..d896032a 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -19,7 +19,6 @@ package utils import ( "crypto/sha256" "encoding/hex" - "errors" "fmt" "io" "io/ioutil" @@ -29,6 +28,7 @@ import ( "strings" "github.com/mholt/archiver" + "github.com/pkg/errors" uuid "github.com/satori/go.uuid" apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -105,7 +105,7 @@ func FindAllGlobs(inputList []string) ([]string, error) { for _, glob := range inputList { f, err := filepath.Glob(glob) if err != nil { - return nil, fmt.Errorf("Invalid glob %v: %v", glob, err) + return nil, errors.Errorf("invalid glob %v: %v", glob, err) } files = append(files, f...) } diff --git a/pkg/v1/types.go b/pkg/v1/types.go index 8e000cb2..ad3db03e 100644 --- a/pkg/v1/types.go +++ b/pkg/v1/types.go @@ -99,24 +99,24 @@ type ( errorCode int ) -const ( - ErrorInternal = iota - - ErrorNotAuthorized - ErrorNotFound - ErrorNameExists - ErrorInvalidArgument - ErrorNoSpace - ErrorNotImplmented -) - -// must match order and len of the above const -var errorDescriptions = []string{ - "Internal error", - "Not authorized", - "Resource not found", - "Resource exists", - "Invalid argument", - "No space", - "Not implemented", -} +//const ( +// ErrorInternal = iota +// +// ErrorNotAuthorized +// ErrorNotFound +// ErrorNameExists +// ErrorInvalidArgument +// ErrorNoSpace +// ErrorNotImplmented +//) +// +//// must match order and len of the above const +//var errorDescriptions = []string{ +// "Internal error", +// "Not authorized", +// "Resource not found", +// "Resource exists", +// "Invalid argument", +// "No space", +// "Not implemented", +//} diff --git a/test/benchmark/picasso.go b/test/benchmark/picasso.go index 90948981..1e1eff6e 100644 --- a/test/benchmark/picasso.go +++ b/test/benchmark/picasso.go @@ -72,7 +72,6 @@ func generateContinuousSeries(file string) chart.Series { reader := bufio.NewReader(f) - var points []*MetricPoint var xVals []float64 var yVals []float64 @@ -95,8 +94,6 @@ func generateContinuousSeries(file string) chart.Series { continue } - points = append(points, point) - if initTime == nil { initTime = &point.Data.Time } @@ -119,7 +116,7 @@ func generateContinuousSeries(file string) chart.Series { func generateChart(title string, file string, format chart.RendererProvider, series []chart.Series) error { if series == nil { - return errors.New("Series cannot be nil") + return errors.New("series cannot be nil") } cs := chart.ConcatSeries(series) @@ -190,16 +187,13 @@ func listJsonFiles(path string) ([]string, error) { if err != nil { return err } - if strings.HasSuffix(path, ".json") && !info.IsDir() { files = append(files, path) - return nil } - return nil }) - return files, nil + return files, err } func main() {