Extend all to more resources

Added more things from the list here:
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/cmd.go#L159

Update the devel/kubectl-conventions.md with the rules mentioned by
a few folks on which resources could be added to the special 'all' alias
This commit is contained in:
Davanum Srinivas 2016-07-14 07:48:32 -04:00
parent 87b40782c9
commit e1373cf516
8 changed files with 147 additions and 17 deletions

View File

@ -43,6 +43,7 @@ Updated: 8/27/2015
- [Principles](#principles)
- [Command conventions](#command-conventions)
- [Create commands](#create-commands)
- [Rules for extending special resource alias - "all"](#rules-for-extending-special-resource-alias---all)
- [Flag conventions](#flag-conventions)
- [Output conventions](#output-conventions)
- [Documentation conventions](#documentation-conventions)
@ -118,6 +119,21 @@ creating tls secrets. You create these as separate commands to get distinct
flags and separate help that is tailored for the particular usage.
### Rules for extending special resource alias - "all"
Here are the rules to add a new resource to the `kubectl get all` output.
* No cluster scoped resources
* No namespace admin level resources (limits, quota, policy, authorization
rules)
* No resources that are potentially unrecoverable (secrets and pvc)
* Resources that are considered "similar" to #3 should be grouped
the same (configmaps)
## Flag conventions
* Flags are all lowercase, with words separated by hyphens

View File

@ -1048,6 +1048,19 @@ __EOF__
exit 1
fi
### Test kubectl get all
output_message=$(kubectl --v=6 --namespace default get all 2>&1 "${kube_flags[@]}")
# Post-condition: Check if we get 200 OK from all the url(s)
kube::test::if_has_string "${output_message}" "/api/v1/namespaces/default/pods 200 OK"
kube::test::if_has_string "${output_message}" "/api/v1/namespaces/default/replicationcontrollers 200 OK"
kube::test::if_has_string "${output_message}" "/api/v1/namespaces/default/services 200 OK"
kube::test::if_has_string "${output_message}" "/apis/apps/v1alpha1/namespaces/default/petsets 200 OK"
kube::test::if_has_string "${output_message}" "/apis/autoscaling/v1/namespaces/default/horizontalpodautoscalers 200"
kube::test::if_has_string "${output_message}" "/apis/batch/v1/namespaces/default/jobs 200 OK"
kube::test::if_has_string "${output_message}" "/apis/extensions/v1beta1/namespaces/default/deployments 200 OK"
kube::test::if_has_string "${output_message}" "/apis/extensions/v1beta1/namespaces/default/replicasets 200 OK"
#####################################
# Third Party Resources #
#####################################

View File

@ -299,7 +299,7 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec, runtime.Neg
mapper := discovery.NewRESTMapper(groupResources, meta.InterfacesForUnstructured)
typer := discovery.NewUnstructuredObjectTyper(groupResources)
return cmdutil.NewShortcutExpander(mapper), typer, nil
return cmdutil.NewShortcutExpander(mapper, nil), typer, nil
},
ClientSet: func() (*internalclientset.Clientset, error) {
// Swap out the HTTP client out of the client with the fake's version.

View File

@ -302,9 +302,10 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
}
mapper := registered.RESTMapper()
discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg)
// if we can find the server version and it's current enough to have discovery information, use it. Otherwise,
// fallback to our hardcoded list
if discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg); err == nil {
if err == nil {
if serverVersion, err := discoveryClient.ServerVersion(); err == nil && useDiscoveryRESTMapper(serverVersion.GitVersion) {
// register third party resources with the api machinery groups. This probably should be done, but
// its consistent with old code, so we'll start with it.
@ -326,7 +327,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
}
// wrap with shortcuts
mapper = NewShortcutExpander(mapper)
mapper = NewShortcutExpander(mapper, discoveryClient)
// wrap with output preferences
mapper = kubectl.OutputVersionMapper{RESTMapper: mapper, OutputVersions: []unversioned.GroupVersion{cmdApiVersion}}
return mapper, api.Scheme
@ -363,7 +364,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
typer := discovery.NewUnstructuredObjectTyper(groupResources)
return NewShortcutExpander(mapper), typer, nil
return NewShortcutExpander(mapper, dc), typer, nil
},
RESTClient: func() (*restclient.RESTClient, error) {
clientConfig, err := clients.ClientConfigForVersion(nil)

View File

@ -32,6 +32,7 @@ import (
"time"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/meta"
"k8s.io/kubernetes/pkg/api/testapi"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/api/validation"
@ -43,6 +44,7 @@ import (
manualfake "k8s.io/kubernetes/pkg/client/unversioned/fake"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/resource"
"k8s.io/kubernetes/pkg/labels"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/pkg/util/flag"
@ -712,3 +714,43 @@ func TestMakePortsString(t *testing.T) {
}
}
}
func fakeClient() resource.ClientMapper {
return resource.ClientMapperFunc(func(*meta.RESTMapping) (resource.RESTClient, error) {
return &manualfake.RESTClient{}, nil
})
}
func TestDiscoveryReplaceAliases(t *testing.T) {
tests := []struct {
name string
arg string
expected string
}{
{
name: "no-replacement",
arg: "service",
expected: "service",
},
{
name: "all-replacement",
arg: "all",
expected: "pods,replicationcontrollers,services,petsets,horizontalpodautoscalers,jobs,deployments,replicasets",
},
{
name: "alias-in-comma-separated-arg",
arg: "all,secrets",
expected: "pods,replicationcontrollers,services,petsets,horizontalpodautoscalers,jobs,deployments,replicasets,secrets",
},
}
mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
b := resource.NewBuilder(mapper, api.Scheme, fakeClient(), testapi.Default.Codec())
for _, test := range tests {
replaced := b.ReplaceAliases(test.arg)
if replaced != test.expected {
t.Errorf("%s: unexpected argument: expected %s, got %s", test.name, test.expected, replaced)
}
}
}

View File

@ -17,8 +17,11 @@ limitations under the License.
package util
import (
"strings"
"k8s.io/kubernetes/pkg/api/meta"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/client/typed/discovery"
"k8s.io/kubernetes/pkg/kubectl"
)
@ -26,13 +29,52 @@ import (
type ShortcutExpander struct {
RESTMapper meta.RESTMapper
All []string
All []unversioned.GroupResource
discoveryClient *discovery.DiscoveryClient
}
var _ meta.RESTMapper = &ShortcutExpander{}
func NewShortcutExpander(delegate meta.RESTMapper) ShortcutExpander {
return ShortcutExpander{All: userResources, RESTMapper: delegate}
func NewShortcutExpander(delegate meta.RESTMapper, client *discovery.DiscoveryClient) ShortcutExpander {
return ShortcutExpander{All: userResources, RESTMapper: delegate, discoveryClient: client}
}
func (e ShortcutExpander) getAll() []unversioned.GroupResource {
if e.discoveryClient == nil {
return e.All
}
// Check if we have access to server resources
apiResources, err := e.discoveryClient.ServerResources()
if err != nil {
return e.All
}
availableResources := []unversioned.GroupVersionResource{}
for groupVersionString, resourceList := range apiResources {
currVersion, err := unversioned.ParseGroupVersion(groupVersionString)
if err != nil {
return e.All
}
for _, resource := range resourceList.APIResources {
availableResources = append(availableResources, currVersion.WithResource(resource.Name))
}
}
availableAll := []unversioned.GroupResource{}
for _, requestedResource := range e.All {
for _, availableResource := range availableResources {
if requestedResource.Group == availableResource.Group &&
requestedResource.Resource == availableResource.Resource {
availableAll = append(availableAll, requestedResource)
break
}
}
}
return availableAll
}
func (e ShortcutExpander) KindFor(resource unversioned.GroupVersionResource) (unversioned.GroupVersionKind, error) {
@ -65,14 +107,30 @@ func (e ShortcutExpander) RESTMappings(gk unversioned.GroupKind) ([]*meta.RESTMa
// userResources are the resource names that apply to the primary, user facing resources used by
// client tools. They are in deletion-first order - dependent resources should be last.
var userResources = []string{"rc", "svc", "pods", "pvc"}
var userResources = []unversioned.GroupResource{
{Group: "", Resource: "pods"},
{Group: "", Resource: "replicationcontrollers"},
{Group: "", Resource: "services"},
{Group: "apps", Resource: "petsets"},
{Group: "autoscaling", Resource: "horizontalpodautoscalers"},
{Group: "extensions", Resource: "jobs"},
{Group: "extensions", Resource: "deployments"},
{Group: "extensions", Resource: "replicasets"},
}
// AliasesForResource returns whether a resource has an alias or not
func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) {
if resource == "all" {
return e.All, true
if strings.ToLower(resource) == "all" {
var resources []unversioned.GroupResource
if resources = e.getAll(); len(resources) == 0 {
resources = userResources
}
aliases := []string{}
for _, r := range resources {
aliases = append(aliases, r.Resource)
}
return aliases, true
}
expanded := expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource
return []string{expanded}, (expanded != resource)
}

View File

@ -37,16 +37,16 @@ func TestReplaceAliases(t *testing.T) {
{
name: "all-replacement",
arg: "all",
expected: "rc,svc,pods,pvc",
expected: "pods,replicationcontrollers,services,petsets,horizontalpodautoscalers,jobs,deployments,replicasets",
},
{
name: "alias-in-comma-separated-arg",
arg: "all,secrets",
expected: "rc,svc,pods,pvc,secrets",
expected: "pods,replicationcontrollers,services,petsets,horizontalpodautoscalers,jobs,deployments,replicasets,secrets",
},
}
mapper := NewShortcutExpander(testapi.Default.RESTMapper())
mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
for _, test := range tests {
resources := []string{}

View File

@ -343,7 +343,7 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string
}
if len(args) > 0 {
// Try replacing aliases only in types
args[0] = b.replaceAliases(args[0])
args[0] = b.ReplaceAliases(args[0])
}
switch {
case len(args) > 2:
@ -364,9 +364,9 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string
return b
}
// replaceAliases accepts an argument and tries to expand any existing
// ReplaceAliases accepts an argument and tries to expand any existing
// aliases found in it
func (b *Builder) replaceAliases(input string) string {
func (b *Builder) ReplaceAliases(input string) string {
replaced := []string{}
for _, arg := range strings.Split(input, ",") {
if aliases, ok := b.mapper.AliasesForResource(arg); ok {