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.

Sign up for GitHub

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

Jump to bottom

Switch ints to time.Duration #427

Merged
merged 7 commits into from
Aug 28, 2018
Merged

Switch ints to time.Duration #427

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c4ed30f
Switch ints to time.duration
theishshah Aug 24, 2018
3ec5596
update templates
theishshah Aug 24, 2018
616e702
line spacing
theishshah Aug 24, 2018
2587191
fix import order
theishshah Aug 24, 2018
4cbc006
updated godoc and removed multiplier
theishshah Aug 27, 2018
b994d20
move from ns to seconds
theishshah Aug 27, 2018
6875f80
update godoc
theishshah Aug 28, 2018
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
3 changes: 2 additions & 1 deletion pkg/generator/generator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ const mainExp = `package main
import (
"context"
"runtime"
"time"

stub "github.com/example-inc/app-operator/pkg/stub"
sdk "github.com/operator-framework/operator-sdk/pkg/sdk"
Expand Down Expand Up @@ -445,7 +446,7 @@ func main() {
if err != nil {
logrus.Fatalf("failed to get watch namespace: %v", err)
}
resyncPeriod := 5
resyncPeriod := time.Duration(5)
logrus.Infof("Watching %s, %s, %s, %d", resource, kind, namespace, resyncPeriod)
sdk.Watch(resource, kind, namespace, resyncPeriod)
sdk.Handle(stub.NewHandler())
Expand Down
3 changes: 2 additions & 1 deletion pkg/generator/templates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ const mainTmpl = `package main
import (
"context"
"runtime"
"time"

stub "{{.StubImport}}"
sdk "{{.OperatorSDKImport}}"
Expand Down Expand Up @@ -159,7 +160,7 @@ func main() {
if err != nil {
logrus.Fatalf("failed to get watch namespace: %v", err)
}
resyncPeriod := 5
resyncPeriod := time.Duration(5)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't' this be a default of 5 seconds:

resyncPeriod := time.Duration(5) * time.Second

Otherwise simply casting it to time.Duration means this is 5ns.
Duration is just an int64 nanosecond count.
https://golang.org/pkg/time/#Duration

logrus.Infof("Watching %s, %s, %s, %d", resource, kind, namespace, resyncPeriod)
sdk.Watch(resource, kind, namespace, resyncPeriod)
sdk.Handle(stub.NewHandler())
Expand Down
3 changes: 2 additions & 1 deletion pkg/sdk/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package sdk

import (
"context"
"time"

"github.com/operator-framework/operator-sdk/pkg/k8sclient"
"github.com/operator-framework/operator-sdk/pkg/sdk/internal/metrics"
Expand All @@ -40,7 +41,7 @@ var (
// Consult the API reference for the Group, Version and Kind of a resource: https://kubernetes.io/docs/reference/
// namespace is the Namespace to watch for the resource
// TODO: support opts for specifying label selector
func Watch(apiVersion, kind, namespace string, resyncPeriod int, opts ...watchOption) {
func Watch(apiVersion, kind, namespace string, resyncPeriod time.Duration, opts ...watchOption) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we update the godoc for this change as well?

resourceClient, resourcePluralName, err := k8sclient.GetResourceClient(apiVersion, kind, namespace)
// TODO: Better error handling, e.g retry
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/sdk/informer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type informer struct {
numWorkers int
}

func NewInformer(resourcePluralName, namespace string, resourceClient dynamic.ResourceInterface, resyncPeriod int, c *metrics.Collector, n int) Informer {
func NewInformer(resourcePluralName, namespace string, resourceClient dynamic.ResourceInterface, resyncPeriod time.Duration, c *metrics.Collector, n int) Informer {
i := &informer{
resourcePluralName: resourcePluralName,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), resourcePluralName),
Expand All @@ -56,7 +56,7 @@ func NewInformer(resourcePluralName, namespace string, resourceClient dynamic.Re
numWorkers: n,
}

resyncDuration := time.Duration(resyncPeriod) * time.Second
resyncDuration := resyncPeriod * time.Second
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to multiply by time.Second here.
The user is allowed to pass in any Duration to sdk.Watch().

i.sharedIndexInformer = cache.NewSharedIndexInformer(
newListWatcherFromResourceClient(resourceClient), &unstructured.Unstructured{}, resyncDuration, cache.Indexers{},
)
Expand Down