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.

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

Stabilize map order in kubectl describe #26046

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 9 additions & 5 deletions pkg/kubectl/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -842,21 +842,25 @@ func describeContainers(label string, containers []api.Container, containerStatu
if len(resourceToQoS) > 0 {
fmt.Fprintf(out, " QoS Tier:\n")
}
for resource, qos := range resourceToQoS {
for _, resource := range SortedQoSResourceNames(resourceToQoS) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should stop reporting qos per resource in 1.3

@bgrant0607 @vishh - now that we have merged #14943 should we update the CLI to just output the QoS of the pod?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. We should do that. I can post a patch for it later today, unless we
want to make that change in this PR itself.

On Wed, May 25, 2016 at 2:43 PM, Derek Carr notifications@github.com
wrote:

In pkg/kubectl/describe.go
#26046 (comment)
:

@@ -842,21 +842,25 @@ func describeContainers(label string, containers []api.Container, containerStatu
if len(resourceToQoS) > 0 {
fmt.Fprintf(out, " QoS Tier:\n")
}

  •   for resource, qos := range resourceToQoS {
    
  •   for _, resource := range SortedQoSResourceNames(resourceToQoS) {
    

I think we should stop reporting qos per resource in 1.3

@bgrant0607 https://github.com/bgrant0607 @vishh
https://github.com/vishh - now that we have merged #14943
#14943 should we update
the CLI to just output the QoS of the pod?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/26046/files/c8a14e376e3df237d6d0d6a2d5266056cbfeab87#r64658693

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should stop reporting qos per resource in 1.3

Does this mean that if I have a deployment and describe it I won't be able to see its QoS anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could be wrong here, but my understanding is that QoS just moves one layer up from containers to pods.

Copy link
Member

Choose a reason for hiding this comment

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

Qos has changed this release to be at the pod level and not for individual
compute resources. A follow on PR was made that now reports the effective
QoS for a pod in describe.

On Thursday, May 26, 2016, Michail Kargakis notifications@github.com
wrote:

In pkg/kubectl/describe.go
#26046 (comment)
:

@@ -842,21 +842,25 @@ func describeContainers(label string, containers []api.Container, containerStatu
if len(resourceToQoS) > 0 {
fmt.Fprintf(out, " QoS Tier:\n")
}

  •   for resource, qos := range resourceToQoS {
    
  •   for _, resource := range SortedQoSResourceNames(resourceToQoS) {
    

I think we should stop reporting qos per resource in 1.3

Does this mean that if I have a deployment and describe it I won't be able
to see its QoS anymore?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/26046/files/c8a14e376e3df237d6d0d6a2d5266056cbfeab87#r64710897

qos := resourceToQoS[resource]
fmt.Fprintf(out, " %s:\t%s\n", resource, qos)
}

if len(container.Resources.Limits) > 0 {
resources := container.Resources
if len(resources.Limits) > 0 {
fmt.Fprintf(out, " Limits:\n")
}
for name, quantity := range container.Resources.Limits {
for _, name := range SortedResourceNames(resources.Limits) {
quantity := resources.Limits[name]
fmt.Fprintf(out, " %s:\t%s\n", name, quantity.String())
}

if len(container.Resources.Requests) > 0 {
if len(resources.Requests) > 0 {
fmt.Fprintf(out, " Requests:\n")
}
for name, quantity := range container.Resources.Requests {
for _, name := range SortedResourceNames(resources.Requests) {
quantity := resources.Requests[name]
fmt.Fprintf(out, " %s:\t%s\n", name, quantity.String())
}

Expand Down
28 changes: 28 additions & 0 deletions pkg/kubectl/describe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,19 @@ func TestDescribeContainers(t *testing.T) {
},
expectedElements: []string{"test", "State", "Waiting", "Ready", "True", "Restart Count", "7", "Image", "image", "time", "1000"},
},
// QoS classes
{
container: api.Container{
Name: "test",
Image: "image",
},
status: api.ContainerStatus{
Name: "test",
Ready: true,
RestartCount: 7,
},
expectedElements: []string{"cpu", "BestEffort", "memory", "BestEffort"},
},
// Using limits.
{
container: api.Container{
Expand All @@ -258,6 +271,21 @@ func TestDescribeContainers(t *testing.T) {
},
expectedElements: []string{"cpu", "1k", "memory", "4G", "storage", "20G"},
},
// Using requests.
{
container: api.Container{
Name: "test",
Image: "image",
Resources: api.ResourceRequirements{
Requests: api.ResourceList{
api.ResourceName(api.ResourceCPU): resource.MustParse("1000"),
api.ResourceName(api.ResourceMemory): resource.MustParse("4G"),
api.ResourceName(api.ResourceStorage): resource.MustParse("20G"),
},
},
},
expectedElements: []string{"cpu", "1k", "memory", "4G", "storage", "20G"},
},
}

for i, testCase := range testCases {
Expand Down
23 changes: 23 additions & 0 deletions pkg/kubectl/sorted_resource_name_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ limitations under the License.
package kubectl

import (
"sort"

"k8s.io/kubernetes/pkg/api"
qosutil "k8s.io/kubernetes/pkg/kubelet/qos/util"
)

type SortableResourceNames []api.ResourceName
Expand All @@ -34,6 +37,16 @@ func (list SortableResourceNames) Less(i, j int) bool {
return list[i] < list[j]
}

// SortedResourceNames returns the sorted resource names of a resource list.
func SortedResourceNames(list api.ResourceList) []api.ResourceName {
resources := make([]api.ResourceName, 0, len(list))
for res := range list {
resources = append(resources, res)
}
sort.Sort(SortableResourceNames(resources))
return resources
}

type SortableResourceQuotas []api.ResourceQuota

func (list SortableResourceQuotas) Len() int {
Expand All @@ -47,3 +60,13 @@ func (list SortableResourceQuotas) Swap(i, j int) {
func (list SortableResourceQuotas) Less(i, j int) bool {
return list[i].Name < list[j].Name
}

// SortedQoSResourceNames returns the sorted resource names of a QoS list.
func SortedQoSResourceNames(list qosutil.QoSList) []api.ResourceName {
resources := make([]api.ResourceName, 0, len(list))
for res := range list {
resources = append(resources, res)
}
sort.Sort(SortableResourceNames(resources))
return resources
}
50 changes: 50 additions & 0 deletions pkg/kubectl/sorted_resource_name_list_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright 2016 The Kubernetes Authors All rights reserved.

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"
"sort"
"testing"

"k8s.io/kubernetes/pkg/api"
)

func TestSortableResourceNamesSorting(t *testing.T) {
want := SortableResourceNames{
api.ResourceName(""),
api.ResourceName("42"),
api.ResourceName("bar"),
api.ResourceName("foo"),
api.ResourceName("foo"),
api.ResourceName("foobar"),
}

in := SortableResourceNames{
api.ResourceName("foo"),
api.ResourceName("42"),
api.ResourceName("foobar"),
api.ResourceName("foo"),
api.ResourceName("bar"),
api.ResourceName(""),
}

sort.Sort(in)
if !reflect.DeepEqual(in, want) {
t.Errorf("got %v, want %v", in, want)
}
}
9 changes: 6 additions & 3 deletions pkg/kubelet/qos/util/qos.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,12 @@ func GetPodQos(pod *api.Pod) string {
return Burstable
}

// GetQos returns a mapping of resource name to QoS class of a container
func GetQoS(container *api.Container) map[api.ResourceName]string {
resourceToQoS := map[api.ResourceName]string{}
// QoSList is a set of (resource name, QoS class) pairs.
type QoSList map[api.ResourceName]string

// GetQoS returns a mapping of resource name to QoS class of a container
func GetQoS(container *api.Container) QoSList {
resourceToQoS := QoSList{}
for resource := range allResources(container) {
switch {
case isResourceGuaranteed(container, resource):
Expand Down