Mark oc export deprecated and simplify what it does

Instead of calling the apiserver code, clear status. Mark deprecated in favor of oc get --export (which itself is now of questionable value). A future release should provide better logic for stripping out unnecessary fields.
openshift · Apr 26, 2018 · bfe70a3 · bfe70a3
1 parent 81635a2
commit bfe70a3
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 49 deletions.
diff --git a/hack/import-restrictions.json b/hack/import-restrictions.json
@@ -386,6 +386,9 @@
     "checkedPackageRoots": [
       "github.com/openshift/origin/pkg/oc"
     ],
+    "ignoredSubTrees": [
+      "github.com/openshift/origin/pkg/oc/admin/groups/examples"
+    ],
     "allowedImportPackageRoots": [
       "vendor/github.com/aws/aws-sdk-go",
       "vendor/github.com/containers/image",
@@ -452,7 +455,6 @@
       "github.com/openshift/origin/pkg/apps/apis/apps/test",
       "github.com/openshift/origin/pkg/apps/client/internalversion",
       "github.com/openshift/origin/pkg/apps/client/v1",
-      "github.com/openshift/origin/pkg/apps/registry/deployconfig",
       "github.com/openshift/origin/pkg/apps/util",
       "github.com/openshift/origin/pkg/authorization/apis/authorization",
       "github.com/openshift/origin/pkg/authorization/reaper",
@@ -462,8 +464,6 @@
       "github.com/openshift/origin/pkg/build/apis/build/install",
       "github.com/openshift/origin/pkg/build/client",
       "github.com/openshift/origin/pkg/build/client/internalversion",
-      "github.com/openshift/origin/pkg/build/registry/build",
-      "github.com/openshift/origin/pkg/build/registry/buildconfig",
       "github.com/openshift/origin/pkg/build/util",
       "github.com/openshift/origin/pkg/bulk",
       "github.com/openshift/origin/pkg/client/config",
@@ -472,7 +472,7 @@
       "github.com/openshift/origin/pkg/cmd/server/apis/config",
       "github.com/openshift/origin/pkg/cmd/server/apis/config/install",
       "github.com/openshift/origin/pkg/cmd/server/apis/config/latest",
-      "github.com/openshift/origin/pkg/cmd/server/apis/config/validation",
+      "github.com/openshift/origin/pkg/cmd/server/apis/config/validation/ldap",
       "github.com/openshift/origin/pkg/cmd/server/bootstrappolicy",
       "github.com/openshift/origin/pkg/cmd/server/etcd",
       "github.com/openshift/origin/pkg/cmd/templates",
@@ -550,16 +550,6 @@
       "vendor/k8s.io/kubernetes/pkg/serviceaccount",
       "vendor/k8s.io/kubernetes/pkg/version",
 
-      "vendor/k8s.io/kubernetes/pkg/registry/core/endpoint",
-      "vendor/k8s.io/kubernetes/pkg/registry/core/namespace",
-      "vendor/k8s.io/kubernetes/pkg/registry/core/node",
-      "vendor/k8s.io/kubernetes/pkg/registry/core/persistentvolume",
-      "vendor/k8s.io/kubernetes/pkg/registry/core/persistentvolumeclaim",
-      "vendor/k8s.io/kubernetes/pkg/registry/core/pod",
-      "vendor/k8s.io/kubernetes/pkg/registry/core/replicationcontroller",
-      "vendor/k8s.io/kubernetes/pkg/registry/core/resourcequota",
-      "vendor/k8s.io/kubernetes/pkg/registry/core/secret",
-      "vendor/k8s.io/kubernetes/pkg/registry/core/serviceaccount",
       "vendor/k8s.io/kubernetes/pkg/registry/rbac/reconciliation",
       "vendor/k8s.io/kubernetes/pkg/registry/rbac/validation"
     ]

diff --git a/pkg/oc/cli/cmd/export.go b/pkg/oc/cli/cmd/export.go
@@ -66,6 +66,8 @@ func NewCmdExport(fullName string, f *clientcmd.Factory, in io.Reader, out io.Wr
 			kcmdutil.CheckErr(err)
 		},
 	}
+	cmd.Deprecated = "Use the `get --export` command instead."
+
 	cmd.Flags().String("as-template", "", "Output a Template object with specified name instead of a List or single object.")
 	cmd.Flags().Bool("exact", false, "If true, preserve fields that may be cluster specific, such as service clusterIPs or generated names")
 	cmd.Flags().Bool("raw", false, "If true, do not alter the resources in any way after they are loaded.")

diff --git a/pkg/oc/cli/cmd/exporter.go b/pkg/oc/cli/cmd/exporter.go
@@ -11,24 +11,10 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	apirequest "k8s.io/apiserver/pkg/endpoints/request"
 	kapi "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/registry/core/endpoint"
-	"k8s.io/kubernetes/pkg/registry/core/namespace"
-	"k8s.io/kubernetes/pkg/registry/core/node"
-	"k8s.io/kubernetes/pkg/registry/core/persistentvolume"
-	"k8s.io/kubernetes/pkg/registry/core/persistentvolumeclaim"
-	"k8s.io/kubernetes/pkg/registry/core/pod"
-	"k8s.io/kubernetes/pkg/registry/core/replicationcontroller"
-	"k8s.io/kubernetes/pkg/registry/core/resourcequota"
-	"k8s.io/kubernetes/pkg/registry/core/secret"
-	"k8s.io/kubernetes/pkg/registry/core/serviceaccount"
 
 	appsapi "github.com/openshift/origin/pkg/apps/apis/apps"
-	deployrest "github.com/openshift/origin/pkg/apps/registry/deployconfig"
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
-	buildrest "github.com/openshift/origin/pkg/build/registry/build"
-	buildconfigrest "github.com/openshift/origin/pkg/build/registry/buildconfig"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	routeapi "github.com/openshift/origin/pkg/route/apis/route"
 	osautil "github.com/openshift/origin/pkg/serviceaccounts/util"
@@ -66,34 +52,29 @@ func (e *DefaultExporter) Export(obj runtime.Object, exact bool) error {
 	} else {
 		glog.V(4).Infof("Object of type %v does not have ObjectMeta: %v", reflect.TypeOf(obj), err)
 	}
-	ctx := apirequest.NewContext()
 
 	switch t := obj.(type) {
 	case *kapi.Endpoints:
-		endpoint.Strategy.PrepareForCreate(ctx, obj)
 	case *kapi.ResourceQuota:
-		resourcequota.Strategy.PrepareForCreate(ctx, obj)
+		t.Status = kapi.ResourceQuotaStatus{}
 	case *kapi.LimitRange:
 	// TODO: this needs to be fixed
 	//  limitrange.Strategy.PrepareForCreate(obj)
 	case *kapi.Node:
-		node.Strategy.PrepareForCreate(ctx, obj)
 		if exact {
 			return nil
 		}
 		// Nodes are the only resources that allow direct status edits, therefore
 		// we clear that without exact so that the node value can be reused.
 		t.Status = kapi.NodeStatus{}
 	case *kapi.Namespace:
-		namespace.Strategy.PrepareForCreate(ctx, obj)
 	case *kapi.PersistentVolumeClaim:
-		persistentvolumeclaim.Strategy.PrepareForCreate(ctx, obj)
+		t.Status = kapi.PersistentVolumeClaimStatus{}
 	case *kapi.PersistentVolume:
-		persistentvolume.Strategy.PrepareForCreate(ctx, obj)
 	case *kapi.ReplicationController:
-		replicationcontroller.Strategy.PrepareForCreate(ctx, obj)
+		t.Status = kapi.ReplicationControllerStatus{}
 	case *kapi.Pod:
-		pod.Strategy.PrepareForCreate(ctx, obj)
+		t.Status = kapi.PodStatus{}
 	case *kapi.PodTemplate:
 	case *kapi.Service:
 		// TODO: service does not yet have a strategy
@@ -110,7 +91,6 @@ func (e *DefaultExporter) Export(obj runtime.Object, exact bool) error {
 			}
 		}
 	case *kapi.Secret:
-		secret.Strategy.PrepareForCreate(ctx, obj)
 		if exact {
 			return nil
 		}
@@ -119,7 +99,6 @@ func (e *DefaultExporter) Export(obj runtime.Object, exact bool) error {
 			return ErrExportOmit
 		}
 	case *kapi.ServiceAccount:
-		serviceaccount.Strategy.PrepareForCreate(ctx, obj)
 		if exact {
 			return nil
 		}
@@ -146,22 +125,17 @@ func (e *DefaultExporter) Export(obj runtime.Object, exact bool) error {
 		t.Secrets = newMountableSecrets
 
 	case *appsapi.DeploymentConfig:
-		return deployrest.CommonStrategy.Export(ctx, obj, exact)
+		t.Status = appsapi.DeploymentConfigStatus{}
 
 	case *buildapi.BuildConfig:
-		// Use the legacy strategy to avoid setting prune defaults if
-		// the object wasn't created with them in the first place.
-		// TODO: use the exportstrategy pattern instead.
-		buildconfigrest.LegacyStrategy.PrepareForCreate(ctx, obj)
-		// TODO: should be handled by prepare for create
-		t.Status.LastVersion = 0
+		t.Status = buildapi.BuildConfigStatus{}
+
 		for i := range t.Spec.Triggers {
 			if p := t.Spec.Triggers[i].ImageChange; p != nil {
 				p.LastTriggeredImageID = ""
 			}
 		}
 	case *buildapi.Build:
-		buildrest.Strategy.PrepareForCreate(ctx, obj)
 		// TODO: should be handled by prepare for create
 		t.Status.Duration = 0
 		t.Status.Phase = buildapi.BuildPhaseNew

diff --git a/pkg/oc/cli/cmd/exporter_test.go b/pkg/oc/cli/cmd/exporter_test.go
@@ -6,6 +6,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/diff"
 	kapi "k8s.io/kubernetes/pkg/apis/core"
 
 	appsapi "github.com/openshift/origin/pkg/apps/apis/apps"
@@ -33,7 +34,7 @@ func TestExport(t *testing.T) {
 			expectedObj: &appsapi.DeploymentConfig{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:       "config",
-					Generation: 1,
+					Generation: 0,
 				},
 				Spec:   appstest.OkDeploymentConfigSpec(),
 				Status: appsapi.DeploymentConfigStatus{},
@@ -157,7 +158,7 @@ func TestExport(t *testing.T) {
 		}
 
 		if !reflect.DeepEqual(test.object, test.expectedObj) {
-			t.Errorf("%s: object mismatch: expected \n%#v\ngot \n%#v\n", test.name, test.expectedObj, test.object)
+			t.Errorf("%s: object mismatch: %s", test.name, diff.ObjectReflectDiff(test.expectedObj, test.object))
 		}
 	}
 }