Merge pull request #55257 from mahdix/history_visitor

Automatic merge from submit-queue (batch tested with PRs 55044, 55257, 55334). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Refactor HistoryViewerFor to use Visitor design pattern

Resolves https://github.com/kubernetes/kubectl/issues/73

This is a follow up from https://github.com/kubernetes/kubernetes/pull/54456 which is already reviewed.

pkg/kubectl/BUILD

@@ -16,6 +16,7 @@ go_test(
         "deployment_test.go",
         "env_file_test.go",
         "generate_test.go",
+        "history_test.go",
         "namespace_test.go",
         "pdb_test.go",
         "quota_test.go",
@@ -133,6 +134,7 @@ go_library(
         "//pkg/controller/deployment/util:go_default_library",
         "//pkg/controller/statefulset:go_default_library",
         "//pkg/credentialprovider:go_default_library",
+        "//pkg/kubectl/apps:go_default_library",
         "//pkg/kubectl/resource:go_default_library",
         "//pkg/kubectl/util:go_default_library",
         "//pkg/kubectl/util/hash:go_default_library",

pkg/kubectl/history.go

@@ -37,8 +37,8 @@ import (
 	clientextv1beta1 "k8s.io/client-go/kubernetes/typed/extensions/v1beta1"
 	"k8s.io/kubernetes/pkg/api"
 	apiv1 "k8s.io/kubernetes/pkg/api/v1"
-	"k8s.io/kubernetes/pkg/apis/apps"
 	deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"
+	kapps "k8s.io/kubernetes/pkg/kubectl/apps"
 	sliceutil "k8s.io/kubernetes/pkg/kubectl/util/slice"
 	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
 )
@@ -52,16 +52,47 @@ type HistoryViewer interface {
 	ViewHistory(namespace, name string, revision int64) (string, error)
 }
 
+type HistoryVisitor struct {
+	clientset kubernetes.Interface
+	result    HistoryViewer
+}
+
+func (v *HistoryVisitor) VisitDeployment(elem kapps.GroupKindElement) {
+	v.result = &DeploymentHistoryViewer{v.clientset}
+}
+
+func (v *HistoryVisitor) VisitStatefulSet(kind kapps.GroupKindElement) {
+	v.result = &StatefulSetHistoryViewer{v.clientset}
+}
+
+func (v *HistoryVisitor) VisitDaemonSet(kind kapps.GroupKindElement) {
+	v.result = &DaemonSetHistoryViewer{v.clientset}
+}
+
+func (v *HistoryVisitor) VisitJob(kind kapps.GroupKindElement)                   {}
+func (v *HistoryVisitor) VisitPod(kind kapps.GroupKindElement)                   {}
+func (v *HistoryVisitor) VisitReplicaSet(kind kapps.GroupKindElement)            {}
+func (v *HistoryVisitor) VisitReplicationController(kind kapps.GroupKindElement) {}
+
+// HistoryViewerFor returns an implementation of HistoryViewer interface for the given schema kind
 func HistoryViewerFor(kind schema.GroupKind, c kubernetes.Interface) (HistoryViewer, error) {
-	switch kind {
-	case extensionsv1beta1.SchemeGroupVersion.WithKind("Deployment").GroupKind(), apps.Kind("Deployment"):
-		return &DeploymentHistoryViewer{c}, nil
-	case apps.Kind("StatefulSet"):
-		return &StatefulSetHistoryViewer{c}, nil
-	case extensionsv1beta1.SchemeGroupVersion.WithKind("DaemonSet").GroupKind(), apps.Kind("DaemonSet"):
-		return &DaemonSetHistoryViewer{c}, nil
+	elem := kapps.GroupKindElement(kind)
+	visitor := &HistoryVisitor{
+		clientset: c,
 	}
-	return nil, fmt.Errorf("no history viewer has been implemented for %q", kind)
+
+	// Determine which HistoryViewer we need here
+	err := elem.Accept(visitor)
+
+	if err != nil {
+		return nil, fmt.Errorf("error retrieving history for %q, %v", kind.String(), err)
+	}
+
+	if visitor.result == nil {
+		return nil, fmt.Errorf("no history viewer has been implemented for %q", kind.String())
+	}
+
+	return visitor.result, nil
 }
 
 type DeploymentHistoryViewer struct {

pkg/kubectl/history_test.go

@@ -0,0 +1,46 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package kubectl
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+var historytests = map[schema.GroupKind]reflect.Type{
+	{Group: "apps", Kind: "DaemonSet"}:   reflect.TypeOf(&DaemonSetHistoryViewer{}),
+	{Group: "apps", Kind: "StatefulSet"}: reflect.TypeOf(&StatefulSetHistoryViewer{}),
+	{Group: "apps", Kind: "Deployment"}:  reflect.TypeOf(&DeploymentHistoryViewer{}),
+}
+
+func TestHistoryViewerFor(t *testing.T) {
+	fakeClientset := &fake.Clientset{}
+
+	for kind, expectedType := range historytests {
+		result, err := HistoryViewerFor(kind, fakeClientset)
+		if err != nil {
+			t.Fatalf("error getting HistoryViewer for a %v: %v", kind.String(), err)
+		}
+
+		if reflect.TypeOf(result) != expectedType {
+			t.Fatalf("unexpected output type (%v was expected but got %v)", expectedType, reflect.TypeOf(result))
+		}
+	}
+}