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

De-lint everything and replace "go lint" with golangci-lint #4253

Merged
merged 16 commits into from
May 16, 2019
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: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ sudo: required
go:
- 1.x

before_install:
- sudo apt-get install -y libvirt-dev
install:
- echo "Don't run anything."
script:
Expand Down
20 changes: 18 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,25 @@ fmt:
vet:
@go vet $(SOURCE_PACKAGES)

# Once v1.16.1+ is released, replace with
# curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh \
# | bash -s -- -b out/linters v1.16.0

out/linters/golangci-lint:
mkdir -p out/linters \
&& cd out/linters \
&& test -f go.mod || go mod init linters \
&& go get -u github.com/golangci/golangci-lint/cmd/golangci-lint@692dacb773b703162c091c2d8c59f9cd2d6801db >/dev/null
cp -f $(GOPATH)/bin/golangci-lint out/linters/golangci-lint

.PHONY: lint
lint:
@golint -set_exit_status $(SOURCE_PACKAGES)
lint: pkg/minikube/assets/assets.go out/linters/golangci-lint
./out/linters/golangci-lint run \
--deadline 4m \
--build-tags "${MINIKUBE_INTEGRATION_BUILD_TAGS}" \
--enable goimports,gocritic,golint,gocyclo,interfacer,misspell,nakedret,stylecheck,unconvert,unparam \
--exclude 'variable on range scope.*in function literal|ifElseChain' \
./...

.PHONY: reportcard
reportcard:
Expand Down
8 changes: 4 additions & 4 deletions cmd/minikube/cmd/cache_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,16 @@ For the list of accessible variables for the template, see the struct values her
cacheCmd.AddCommand(listCacheCmd)
}

// cacheList returns a formatted list of images found within the local cache
func cacheList(images []string) error {
for _, image := range images {
tmpl, err := template.New("list").Parse(cacheListFormat)
if err != nil {
exit.WithError("Unable to parse template", err)
return err
}
listTmplt := CacheListTemplate{image}
err = tmpl.Execute(os.Stdout, listTmplt)
if err != nil {
exit.WithError("Unable to process template", err)
if err := tmpl.Execute(os.Stdout, listTmplt); err != nil {
return err
}
}
return nil
Expand Down
5 changes: 4 additions & 1 deletion cmd/minikube/cmd/config/addons.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package config

import (
"github.com/golang/glog"
"github.com/spf13/cobra"
)

Expand All @@ -26,6 +27,8 @@ var AddonsCmd = &cobra.Command{
Short: "Modify minikube's kubernetes addons",
Long: `addons modifies minikube addons files using subcommands like "minikube addons enable heapster"`,
Run: func(cmd *cobra.Command, args []string) {
cmd.Help()
if err := cmd.Help(); err != nil {
glog.Errorf("help: %v", err)
}
},
}
9 changes: 6 additions & 3 deletions cmd/minikube/cmd/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"os"
"strings"

"github.com/golang/glog"
"github.com/spf13/cobra"
"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/constants"
Expand Down Expand Up @@ -261,7 +262,9 @@ var ConfigCmd = &cobra.Command{
Long: `config modifies minikube config files using subcommands like "minikube config set vm-driver kvm"
Configurable fields: ` + "\n\n" + configurableFields(),
Run: func(cmd *cobra.Command, args []string) {
cmd.Help()
if err := cmd.Help(); err != nil {
glog.Errorf("help: %v", err)
}
},
}

Expand Down Expand Up @@ -344,12 +347,12 @@ func DeleteFromConfigMap(name string, images []string) error {
func WriteConfig(m config.MinikubeConfig) error {
f, err := os.Create(constants.ConfigFile)
if err != nil {
return fmt.Errorf("Could not open file %s: %s", constants.ConfigFile, err)
return fmt.Errorf("create %s: %s", constants.ConfigFile, err)
}
defer f.Close()
err = encode(f, m)
if err != nil {
return fmt.Errorf("Error encoding config %s: %s", constants.ConfigFile, err)
return fmt.Errorf("encode %s: %s", constants.ConfigFile, err)
}
return nil
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/minikube/cmd/config/open.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ You can add one by annotating a service with the label %s:%s`, key, addonName)
}
for i := range serviceList.Items {
svc := serviceList.Items[i].ObjectMeta.Name
service.WaitAndMaybeOpenService(api, namespace, svc, addonsURLTemplate,
addonsURLMode, https, wait, interval)

if err := service.WaitAndMaybeOpenService(api, namespace, svc, addonsURLTemplate, addonsURLMode, https, wait, interval); err != nil {
exit.WithCode(exit.Unavailable, "Wait failed: %v", err)
}
}
},
}
Expand Down
19 changes: 11 additions & 8 deletions cmd/minikube/cmd/config/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"os"
"strings"

"github.com/golang/glog"
"golang.org/x/crypto/ssh/terminal"
"k8s.io/minikube/pkg/minikube/console"
)
Expand All @@ -42,15 +43,13 @@ func AskForYesNoConfirmation(s string, posResponses, negResponses []string) bool
log.Fatal(err)
}

response = strings.ToLower(strings.TrimSpace(response))

if containsString(posResponses, response) {
switch r := strings.ToLower(strings.TrimSpace(response)); {
case containsString(posResponses, r):
return true
} else if containsString(negResponses, response) {
case containsString(negResponses, r):
return false
} else {
default:
console.Err("Please type yes or no:")
return AskForYesNoConfirmation(s, posResponses, negResponses)
}
}
}
Expand Down Expand Up @@ -112,7 +111,7 @@ func concealableAskForStaticValue(readWriter io.ReadWriter, promptString string,
response = strings.TrimSpace(response)
if len(response) == 0 {
console.Warning("Please enter a value:")
return concealableAskForStaticValue(readWriter, promptString, hidden)
continue
}
return response, nil
}
Expand All @@ -126,7 +125,11 @@ func AskForPasswordValue(s string) string {
if err != nil {
log.Fatal(err)
}
defer terminal.Restore(stdInFd, oldState)
defer func() {
if err := terminal.Restore(stdInFd, oldState); err != nil {
glog.Errorf("terminal restore failed: %v", err)
}
}()

result, err := concealableAskForStaticValue(os.Stdin, s, true)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/config/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func findSetting(name string) (Setting, error) {
return s, nil
}
}
return Setting{}, fmt.Errorf("Property name %s not found", name)
return Setting{}, fmt.Errorf("property name %q not found", name)
}

// Set Functions
Expand Down
27 changes: 14 additions & 13 deletions cmd/minikube/cmd/config/validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,24 @@ import (
"k8s.io/minikube/pkg/minikube/cruntime"
)

// containerdOnlyMsg is the message shown when a containerd-only addon is enabled
const containerdOnlyAddonMsg = `
This addon can only be enabled with the containerd runtime backend. To enable this backend, please first stop minikube with:

minikube stop

and then start minikube again with the following flags:

minikube start --container-runtime=containerd --docker-opt containerd=/var/run/containerd/containerd.sock`

// IsValidDriver checks if a driver is supported
func IsValidDriver(string, driver string) error {
for _, d := range constants.SupportedVMDrivers {
if driver == d {
return nil
}
}
return fmt.Errorf("Driver %s is not supported", driver)
return fmt.Errorf("driver %q is not supported", driver)
}

// RequiresRestartMsg returns the "requires restart" message
Expand All @@ -53,7 +63,7 @@ func RequiresRestartMsg(string, string) error {
func IsValidDiskSize(name string, disksize string) error {
_, err := units.FromHumanSize(disksize)
if err != nil {
return fmt.Errorf("Not valid disk size: %v", err)
return fmt.Errorf("invalid disk size: %v", err)
}
return nil
}
Expand Down Expand Up @@ -117,7 +127,7 @@ func IsPositive(name string, val string) error {
func IsValidCIDR(name string, cidr string) error {
_, _, err := net.ParseCIDR(cidr)
if err != nil {
return fmt.Errorf("Error parsing CIDR: %v", err)
return fmt.Errorf("invalid CIDR: %v", err)
}
return nil
}
Expand Down Expand Up @@ -151,16 +161,7 @@ func IsContainerdRuntime(_, _ string) error {
}
_, ok := r.(*cruntime.Containerd)
if !ok {
return fmt.Errorf(`This addon can only be enabled with the containerd runtime backend.

To enable this backend, please first stop minikube with:

minikube stop

and then start minikube again with the following flags:

minikube start --container-runtime=containerd --docker-opt containerd=/var/run/containerd/containerd.sock`)
return fmt.Errorf(containerdOnlyAddonMsg)
}

return nil
}
4 changes: 3 additions & 1 deletion cmd/minikube/cmd/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,9 @@ var dockerEnvCmd = &cobra.Command{
}
}

executeTemplateStdout(shellCfg)
if err := executeTemplateStdout(shellCfg); err != nil {
exit.WithError("Error executing template", err)
}
},
}

Expand Down
14 changes: 10 additions & 4 deletions cmd/minikube/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ var RootCmd = &cobra.Command{

logDir := pflag.Lookup("log_dir")
if !logDir.Changed {
logDir.Value.Set(constants.MakeMiniPath("logs"))
if err := logDir.Value.Set(constants.MakeMiniPath("logs")); err != nil {
exit.WithError("logdir set failed", err)
}
}

if enableUpdateNotification {
Expand Down Expand Up @@ -116,7 +118,9 @@ func setFlagsUsingViper() {
}
// Viper will give precedence first to calls to the Set command,
// then to values from the config.yml
a.Value.Set(viper.GetString(a.Name))
if err := a.Value.Set(viper.GetString(a.Name)); err != nil {
exit.WithError(fmt.Sprintf("failed to set value for %q", a.Name), err)
}
a.Changed = true
}
}
Expand All @@ -129,7 +133,9 @@ func init() {
RootCmd.AddCommand(configCmd.AddonsCmd)
RootCmd.AddCommand(configCmd.ProfileCmd)
pflag.CommandLine.AddGoFlagSet(goflag.CommandLine)
viper.BindPFlags(RootCmd.PersistentFlags())
if err := viper.BindPFlags(RootCmd.PersistentFlags()); err != nil {
exit.WithError("Unable to bind flags", err)
}

cobra.OnInitialize(initConfig)

Expand Down Expand Up @@ -176,7 +182,7 @@ func GetClusterBootstrapper(api libmachine.API, bootstrapperName string) (bootst
return nil, errors.Wrap(err, "getting kubeadm bootstrapper")
}
default:
return nil, fmt.Errorf("Unknown bootstrapper: %s", bootstrapperName)
return nil, fmt.Errorf("unknown bootstrapper: %s", bootstrapperName)
}

return b, nil
Expand Down
36 changes: 23 additions & 13 deletions cmd/minikube/cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"
"testing"

"github.com/pkg/errors"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/spf13/viper"
Expand Down Expand Up @@ -145,37 +146,43 @@ func getEnvVarName(name string) string {
return constants.MinikubeEnvPrefix + "_" + strings.ToUpper(name)
}

func setValues(t *testing.T, tt configTest) {
func setValues(tt configTest) error {
if tt.FlagValue != "" {
pflag.Set(tt.Name, tt.FlagValue)
if err := pflag.Set(tt.Name, tt.FlagValue); err != nil {
return errors.Wrap(err, "flag set")
}
}
if tt.EnvValue != "" {
s := strings.Replace(getEnvVarName(tt.Name), "-", "_", -1)
os.Setenv(s, tt.EnvValue)
}
if tt.ConfigValue != "" {
err := initTestConfig(tt.ConfigValue)
if err != nil {
t.Fatalf("Config %s not read correctly: %v", tt.ConfigValue, err)
if err := initTestConfig(tt.ConfigValue); err != nil {
return errors.Wrapf(err, "Config %s not read correctly", tt.ConfigValue)
}
}
return nil
}

func unsetValues(tt configTest) {
var f = pflag.Lookup(tt.Name)
f.Value.Set(f.DefValue)
func unsetValues(name string) error {
f := pflag.Lookup(name)
if err := f.Value.Set(f.DefValue); err != nil {
return errors.Wrapf(err, "set(%s)", f.DefValue)
}
f.Changed = false

os.Unsetenv(getEnvVarName(tt.Name))

os.Unsetenv(getEnvVarName(name))
viper.Reset()
return nil
}

func TestViperAndFlags(t *testing.T) {
restore := hideEnv(t)
defer restore(t)
for _, tt := range configTests {
setValues(t, tt)
err := setValues(tt)
if err != nil {
t.Fatalf("setValues: %v", err)
}
setupViper()
f := pflag.Lookup(tt.Name)
if f == nil {
Expand All @@ -185,6 +192,9 @@ func TestViperAndFlags(t *testing.T) {
if actual != tt.ExpectedValue {
t.Errorf("pflag.Value(%s) => %s, wanted %s [%+v]", tt.Name, actual, tt.ExpectedValue, tt)
}
unsetValues(tt)
// Some flag validation may not accept their default value, such as log_at_backtrace :(
if err := unsetValues(tt.Name); err != nil {
t.Logf("unsetValues(%s) failed: %v", tt.Name, err)
}
}
}
4 changes: 2 additions & 2 deletions cmd/minikube/cmd/service_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (

"github.com/olekukonko/tablewriter"
"github.com/spf13/cobra"
"k8s.io/api/core/v1"
core "k8s.io/api/core/v1"
"k8s.io/minikube/pkg/minikube/console"
"k8s.io/minikube/pkg/minikube/exit"
"k8s.io/minikube/pkg/minikube/machine"
Expand Down Expand Up @@ -69,6 +69,6 @@ var serviceListCmd = &cobra.Command{
}

func init() {
serviceListCmd.Flags().StringVarP(&serviceListNamespace, "namespace", "n", v1.NamespaceAll, "The services namespace")
serviceListCmd.Flags().StringVarP(&serviceListNamespace, "namespace", "n", core.NamespaceAll, "The services namespace")
serviceCmd.AddCommand(serviceListCmd)
}
Loading