Skip to content

Commit

Permalink
Merge pull request kubernetes#68851 from seans3/rollout-cleanup
Browse files Browse the repository at this point in the history
Remove unused client in rollout status
  • Loading branch information
k8s-ci-robot authored Sep 27, 2018
2 parents 3611c5c + eadb6b1 commit b637ae6
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 44 deletions.
12 changes: 5 additions & 7 deletions pkg/kubectl/cmd/rollout/rollout_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ type RolloutStatusOptions struct {
Revision int64
Timeout time.Duration

StatusViewer func(*meta.RESTMapping) (kubectl.StatusViewer, error)
Builder func() *resource.Builder
DynamicClient dynamic.Interface
StatusViewerFn func(*meta.RESTMapping) (kubectl.StatusViewer, error)
Builder func() *resource.Builder
DynamicClient dynamic.Interface

FilenameOptions *resource.FilenameOptions
genericclioptions.IOStreams
Expand Down Expand Up @@ -127,9 +127,7 @@ func (o *RolloutStatusOptions) Complete(f cmdutil.Factory, args []string) error
}

o.BuilderArgs = args
o.StatusViewer = func(mapping *meta.RESTMapping) (kubectl.StatusViewer, error) {
return polymorphichelpers.StatusViewerFn(f, mapping)
}
o.StatusViewerFn = polymorphichelpers.StatusViewerFn

clientConfig, err := f.ToRESTConfig()
if err != nil {
Expand Down Expand Up @@ -180,7 +178,7 @@ func (o *RolloutStatusOptions) Run() error {
info := infos[0]
mapping := info.ResourceMapping()

statusViewer, err := o.StatusViewer(mapping)
statusViewer, err := o.StatusViewerFn(mapping)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubectl/polymorphichelpers/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type HistoryViewerFunc func(restClientGetter genericclioptions.RESTClientGetter,
var HistoryViewerFn HistoryViewerFunc = historyViewer

// StatusViewerFunc is a function type that can tell you how to print rollout status
type StatusViewerFunc func(restClientGetter genericclioptions.RESTClientGetter, mapping *meta.RESTMapping) (kubectl.StatusViewer, error)
type StatusViewerFunc func(mapping *meta.RESTMapping) (kubectl.StatusViewer, error)

// StatusViewerFn gives a way to easily override the function for unit testing if needed
var StatusViewerFn StatusViewerFunc = statusViewer
Expand Down
14 changes: 2 additions & 12 deletions pkg/kubectl/polymorphichelpers/statusviewer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,10 @@ package polymorphichelpers

import (
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/cli-runtime/pkg/genericclioptions"
"k8s.io/client-go/kubernetes"
"k8s.io/kubernetes/pkg/kubectl"
)

// statusViewer returns a StatusViewer for printing rollout status.
func statusViewer(restClientGetter genericclioptions.RESTClientGetter, mapping *meta.RESTMapping) (kubectl.StatusViewer, error) {
clientConfig, err := restClientGetter.ToRESTConfig()
if err != nil {
return nil, err
}
clientset, err := kubernetes.NewForConfig(clientConfig)
if err != nil {
return nil, err
}
return kubectl.StatusViewerFor(mapping.GroupVersionKind.GroupKind(), clientset)
func statusViewer(mapping *meta.RESTMapping) (kubectl.StatusViewer, error) {
return kubectl.StatusViewerFor(mapping.GroupVersionKind.GroupKind())
}
22 changes: 7 additions & 15 deletions pkg/kubectl/rollout_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import (
extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/kubernetes"
clientappsv1 "k8s.io/client-go/kubernetes/typed/apps/v1"
"k8s.io/kubernetes/pkg/controller/deployment/util"
"k8s.io/kubernetes/pkg/kubectl/scheme"
)
Expand All @@ -35,34 +33,28 @@ type StatusViewer interface {
}

// StatusViewerFor returns a StatusViewer for the resource specified by kind.
func StatusViewerFor(kind schema.GroupKind, c kubernetes.Interface) (StatusViewer, error) {
func StatusViewerFor(kind schema.GroupKind) (StatusViewer, error) {
switch kind {
case extensionsv1beta1.SchemeGroupVersion.WithKind("Deployment").GroupKind(),
appsv1.SchemeGroupVersion.WithKind("Deployment").GroupKind():
return &DeploymentStatusViewer{c.AppsV1()}, nil
return &DeploymentStatusViewer{}, nil
case extensionsv1beta1.SchemeGroupVersion.WithKind("DaemonSet").GroupKind(),
appsv1.SchemeGroupVersion.WithKind("DaemonSet").GroupKind():
return &DaemonSetStatusViewer{c.AppsV1()}, nil
return &DaemonSetStatusViewer{}, nil
case appsv1.SchemeGroupVersion.WithKind("StatefulSet").GroupKind():
return &StatefulSetStatusViewer{c.AppsV1()}, nil
return &StatefulSetStatusViewer{}, nil
}
return nil, fmt.Errorf("no status viewer has been implemented for %v", kind)
}

// DeploymentStatusViewer implements the StatusViewer interface.
type DeploymentStatusViewer struct {
c clientappsv1.DeploymentsGetter
}
type DeploymentStatusViewer struct{}

// DaemonSetStatusViewer implements the StatusViewer interface.
type DaemonSetStatusViewer struct {
c clientappsv1.DaemonSetsGetter
}
type DaemonSetStatusViewer struct{}

// StatefulSetStatusViewer implements the StatusViewer interface.
type StatefulSetStatusViewer struct {
c clientappsv1.StatefulSetsGetter
}
type StatefulSetStatusViewer struct{}

// Status returns a message describing deployment status, and a bool value indicating if the status is considered done.
func (s *DeploymentStatusViewer) Status(obj runtime.Unstructured, revision int64) (string, bool, error) {
Expand Down
13 changes: 4 additions & 9 deletions pkg/kubectl/rollout_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
api "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/kubernetes/pkg/kubectl/scheme"
)

Expand Down Expand Up @@ -134,8 +133,7 @@ func TestDeploymentStatusViewerStatus(t *testing.T) {
t.Fatal(err)
}

client := fake.NewSimpleClientset(d).Apps()
dsv := &DeploymentStatusViewer{c: client}
dsv := &DeploymentStatusViewer{}
msg, done, err := dsv.Status(unstructuredD, 0)
if err != nil {
t.Fatalf("DeploymentStatusViewer.Status(): %v", err)
Expand Down Expand Up @@ -240,8 +238,7 @@ func TestDaemonSetStatusViewerStatus(t *testing.T) {
t.Fatal(err)
}

client := fake.NewSimpleClientset(d).Apps()
dsv := &DaemonSetStatusViewer{c: client}
dsv := &DaemonSetStatusViewer{}
msg, done, err := dsv.Status(unstructuredD, 0)
if err != nil {
t.Fatalf("unexpected error: %v", err)
Expand Down Expand Up @@ -392,8 +389,7 @@ func TestStatefulSetStatusViewerStatus(t *testing.T) {
t.Fatal(err)
}

client := fake.NewSimpleClientset(s).AppsV1()
dsv := &StatefulSetStatusViewer{c: client}
dsv := &StatefulSetStatusViewer{}
msg, done, err := dsv.Status(unstructuredS, 0)
if test.err && err == nil {
t.Fatalf("%s: expected error", test.name)
Expand Down Expand Up @@ -431,8 +427,7 @@ func TestDaemonSetStatusViewerStatusWithWrongUpdateStrategyType(t *testing.T) {
t.Fatal(err)
}

client := fake.NewSimpleClientset(d).Apps()
dsv := &DaemonSetStatusViewer{c: client}
dsv := &DaemonSetStatusViewer{}
msg, done, err := dsv.Status(unstructuredD, 0)
errMsg := "rollout status is only available for RollingUpdate strategy type"
if err == nil || err.Error() != errMsg {
Expand Down

0 comments on commit b637ae6

Please sign in to comment.