Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Fix golangci issues on master branch #78

Merged
merged 5 commits into from
Aug 13, 2020
Merged

Fix golangci issues on master branch #78

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3b0b7b1
Fix golangci issues on master branch
edwardstudy Aug 11, 2020
298b315
Updated comments; Ignore errcheck for viper.BindPFlag
edwardstudy Aug 12, 2020
27488f5
Merge branch 'master' into ed/fix-lint
mudler Aug 13, 2020
73fd1c3
Drop interfacer
Aug 13, 2020
3f885bc
debugJSON logs debug message
Aug 13, 2020
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 go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7
github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
github.com/coreos/pkg v0.0.0-20180108230652-97fdf19511ea/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
github.com/cpuguy83/go-md2man v1.0.10 h1:BSKMNlYxDvnunlTymqtgONjNnaRV1sTpcovwwjF22jk=
github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE=
github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
github.com/daaku/go.zipexe v1.0.0/go.mod h1:z8IiR6TsVLEYKwXAoE/I+8ys/sDkgTzSL0CLnGVd57E=
Expand Down Expand Up @@ -314,6 +315,7 @@ github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsT
github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/russross/blackfriday v1.5.2 h1:HyvC0ARfnZBqnXwABFeSZHpKvJHJJfPz81GNueLj0oo=
github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/apply_crd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ func ApplyCRDs(ctx context.Context, apply ApplyFn, restConfig *rest.Config) erro
// ApplyCRDsFlags adds to viper flags
func ApplyCRDsFlags(pf *flag.FlagSet, argToEnv map[string]string) {
pf.Bool("apply-crd", true, "If true, apply CRDs on start")
viper.BindPFlag("apply-crd", pf.Lookup("apply-crd"))
viper.BindPFlag("apply-crd", pf.Lookup("apply-crd")) //nolint:errcheck
argToEnv["apply-crd"] = "APPLY_CRD"
}
1 change: 1 addition & 0 deletions pkg/cmd/argtoenv.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ func AddEnvToUsage(cmd *cobra.Command, argToEnv map[string]string) {
flagSet := make(map[string]bool)

for arg, env := range argToEnv {
//nolint:errcheck
manno marked this conversation as resolved.
Show resolved Hide resolved
viper.BindEnv(arg, env)
flag := cmd.Flag(arg)

Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/ctx_timeout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func CtxTimeOut(cfg *config.Config) {
// CtxTimeOutFlags adds to viper flags
func CtxTimeOutFlags(pf *flag.FlagSet, argToEnv map[string]string) {
pf.Int("ctx-timeout", 300, "context timeout for each k8s API request in seconds")
//nolint:errcheck
viper.BindPFlag("ctx-timeout", pf.Lookup("ctx-timeout"))
argToEnv["ctx-timeout"] = "CTX_TIMEOUT"
}
9 changes: 5 additions & 4 deletions pkg/cmd/docker_image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@ func DockerImageFlags(pf *flag.FlagSet, argToEnv map[string]string, repo string,
pf.StringP("docker-image-tag", "t", tag, "Tag of the operator docker image")
pf.StringP("docker-image-pull-policy", "", string(corev1.PullIfNotPresent), "Image pull policy")

viper.BindPFlag("docker-image-org", pf.Lookup("docker-image-org"))
viper.BindPFlag("docker-image-repository", pf.Lookup("docker-image-repository"))
viper.BindPFlag("docker-image-tag", pf.Lookup("docker-image-tag"))
viper.BindPFlag("docker-image-pull-policy", pf.Lookup("docker-image-pull-policy"))
// TODO Check all of viper.BindPFlag on project level
viper.BindPFlag("docker-image-org", pf.Lookup("docker-image-org")) //nolint:errcheck
viper.BindPFlag("docker-image-repository", pf.Lookup("docker-image-repository")) //nolint:errcheck
viper.BindPFlag("docker-image-tag", pf.Lookup("docker-image-tag")) //nolint:errcheck
viper.BindPFlag("docker-image-pull-policy", pf.Lookup("docker-image-pull-policy")) //nolint:errcheck

argToEnv["docker-image-org"] = "DOCKER_IMAGE_ORG"
argToEnv["docker-image-repository"] = "DOCKER_IMAGE_REPOSITORY"
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/kubeconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func KubeConfig(log *zap.SugaredLogger) (*rest.Config, error) {
// KubeConfigFlags adds to viper flags
func KubeConfigFlags(pf *flag.FlagSet, argToEnv map[string]string) {
pf.StringP("kubeconfig", "c", "", "Path to a kubeconfig, not required in-cluster")
//nolint:errcheck
viper.BindPFlag("kubeconfig", pf.Lookup("kubeconfig"))
argToEnv["kubeconfig"] = "KUBECONFIG"
}
1 change: 1 addition & 0 deletions pkg/cmd/logger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ func LogLevel() string {
// LoggerFlags adds to viper flags
func LoggerFlags(pf *flag.FlagSet, argToEnv map[string]string) {
pf.StringP("log-level", "l", "debug", "Only print log messages from this level onward (trace,debug,info,warn)")
//nolint:errcheck
viper.BindPFlag("log-level", pf.Lookup("log-level"))
argToEnv["log-level"] = "LOG_LEVEL"
}
1 change: 1 addition & 0 deletions pkg/cmd/meltdown.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func MeltdownFlags(pf *flag.FlagSet, argToEnv map[string]string) {
pf.Int("meltdown-duration", 60, "Duration (in seconds) of the meltdown period, in which we postpone further reconciles for the same resource")
pf.Int("meltdown-requeue-after", 30, "Duration (in seconds) for which we delay the requeuing of the reconcile")
for _, opt := range []string{"meltdown-duration", "meltdown-requeue-after"} {
//nolint:errcheck
viper.BindPFlag(opt, pf.Lookup(opt))

argToEnv[opt] = envName(opt)
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/monitored_id.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func MonitoredID(cfg *config.Config) {
// MonitoredIDFlags adds to viper flags
func MonitoredIDFlags(pf *flag.FlagSet, argToEnv map[string]string) {
pf.String("monitored-id", "default", "only monitor namespaces with this id in their namespace label")
//nolint:errcheck
viper.BindPFlag("monitored-id", pf.Lookup("monitored-id"))
argToEnv["monitored-id"] = "MONITORED_ID"
}
1 change: 1 addition & 0 deletions pkg/cmd/namespaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func OperatorNamespace(cfg *config.Config, log *zap.SugaredLogger, name string)
func OperatorNamespaceFlags(pf *flag.FlagSet, argToEnv map[string]string, name string) {
pf.StringP(name, "n", "default", "The operator namespace, for the webhook service")

//nolint:errcheck
viper.BindPFlag(name, pf.Lookup(name))

argToEnv[name] = envName(name)
Expand Down
4 changes: 2 additions & 2 deletions pkg/crd/crd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

// ApplyCRD creates or updates the CRD
func ApplyCRD(ctx context.Context, client extv1client.ApiextensionsV1beta1Interface, crdName, kind, plural string, shortNames []string, groupVersion schema.GroupVersion, validation *extv1.CustomResourceValidation) error {
func ApplyCRD(ctx context.Context, client extv1client.ApiextensionsV1beta1Interface, crdName, kind, plural string, shortNames []string, groupVersion schema.GroupVersion, validation *extv1.CustomResourceValidation) error { // nolint:interfacer
crd := &extv1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: crdName,
Expand Down Expand Up @@ -68,7 +68,7 @@ func ApplyCRD(ctx context.Context, client extv1client.ApiextensionsV1beta1Interf
}

// WaitForCRDReady blocks until the CRD is ready.
func WaitForCRDReady(ctx context.Context, client extv1client.ApiextensionsV1beta1Interface, crdName string) error {
func WaitForCRDReady(ctx context.Context, client extv1client.ApiextensionsV1beta1Interface, crdName string) error { // nolint:interfacer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anyone intend to use the output of interfacer? Maybe we drop that linter

Copy link
Member Author

@edwardstudy edwardstudy Aug 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manno I had same feeling.
BTW, interface is out of maintain: https://github.com/mvdan/interfacer

Deprecated: A tool that suggests interfaces is prone to bad suggestions, so its usefulness in real code is limited.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, added a commit to drop it

err := wait.ExponentialBackoff(
wait.Backoff{
Duration: time.Second,
Expand Down
6 changes: 5 additions & 1 deletion pkg/ctxlog/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,11 @@ func (ev Event) debugJSON(ctx context.Context, objectInfo interface{}) {

// treat JSON data as a string map and extract message
var result map[string]string
json.Unmarshal([]byte(jsonData), &result)
err := json.Unmarshal([]byte(jsonData), &result)
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err != nil {
if err := json.Unmarshal([]byte(jsonData), &result); err != nil {

return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be helpful to print the error to debug if it fails?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it was silent for a reason, but then a few lines done it does print a debug msg. So why not here, too.

}

if msg, ok := result["message"]; ok {
log := ExtractLoggerWithOptions(ctx, zap.AddCallerSkip(1))
log.Debug(msg)
Expand Down
6 changes: 5 additions & 1 deletion pkg/kubeconfig/checker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ var _ = Describe("Checker", func() {
"Check method",
func(c checkCase) {
logger := zap.NewNop()
defer logger.Sync()
defer func() {
serr := logger.Sync()
Expect(serr).To(BeNil())
}()

c.checker.log = logger.Sugar()

actualErr := c.checker.Check(c.cfg)
Expand Down
6 changes: 5 additions & 1 deletion pkg/kubeconfig/getter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ var _ = Describe("Getter", func() {
"Get method",
func(c getCase) {
logger := zap.NewNop()
defer logger.Sync()
defer func() {
serr := logger.Sync()
Expect(serr).To(BeNil())
}()

c.getter.log = logger.Sugar()

actualConfig, actualErr := c.getter.Get(c.configPath)
Expand Down
5 changes: 4 additions & 1 deletion pkg/logger/logger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ func newLogger(level string, options ...zap.Option) *zap.Logger {
}

l := zap.DebugLevel
l.Set(level)
err := l.Set(level)
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err != nil {
if err := l.Set(level); err != nil {

golog.Fatalf("cannot sets the level for ZAP logger: %v", err)
}

cfg := zap.NewDevelopmentConfig()
cfg.Development = false
Expand Down
10 changes: 8 additions & 2 deletions pkg/webhook/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ func (f *Config) CreateValidationWebhookServerConfig(ctx context.Context, webhoo
}
}
ctxlog.Debugf(ctx, "Creating validation webhook config '%s'", config.Name)
f.client.Delete(ctx, config)
err := f.client.Delete(ctx, config)
if err != nil && !apierrors.IsNotFound(err) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err != nil && !apierrors.IsNotFound(err) {
if err := f.client.Delete(ctx, config); err != nil && !apierrors.IsNotFound(err) {

ctxlog.Debugf(ctx, "Trying to deleting existing validatingWebhookConfiguration %s: %s", config.Name, err.Error())
}
return f.client.Create(ctx, config)
}

Expand Down Expand Up @@ -251,7 +254,10 @@ func (f *Config) CreateMutationWebhookServerConfig(ctx context.Context, webhooks
}

ctxlog.Debugf(ctx, "Creating mutating webhook config '%s'", config.Name)
f.client.Delete(ctx, &config)
err := f.client.Delete(ctx, &config)
if err != nil && !apierrors.IsNotFound(err) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err != nil && !apierrors.IsNotFound(err) {
if err := f.client.Delete(ctx, &config); err != nil && !apierrors.IsNotFound(err) {

ctxlog.Debugf(ctx, "Trying to deleting existing mutatingWebhookConfiguration %s: %s", config.Name, err.Error())
}
return f.client.Create(ctx, &config)
}

Expand Down