Skip to content

Commit 27e2b18

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#12991 from fabriziopandini/add-condition-filter-for-clusterctl-describe
✨ Add conditions filter for clusterctl describe
2 parents 84e2bb6 + 32ea905 commit 27e2b18

File tree

8 files changed

+180
-80
lines changed

8 files changed

+180
-80
lines changed

‎cmd/clusterctl/client/describe.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type DescribeClusterOptions struct {
3535
ClusterName string
3636

3737
// ShowOtherConditions is a list of comma separated kind or kind/name for which we should add the ShowObjectConditionsAnnotation
38-
// to signal to the presentation layer to show all the conditions for the objects.
38+
// to signal to the presentation layer to show conditions for the objects.
3939
ShowOtherConditions string
4040

4141
// ShowMachineSets instructs the discovery process to include machine sets in the ObjectTree.

‎cmd/clusterctl/client/tree/annotations.go‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ import (
2323
)
2424

2525
const (
26-
// ShowObjectConditionsAnnotation documents that the presentation layer should show all the conditions for the object.
26+
// ShowObjectConditionsAnnotation documents that the presentation layer should show conditions for the object
27+
// and the filter to select those conditions.
2728
ShowObjectConditionsAnnotation = "tree.cluster.x-k8s.io.io/show-conditions"
2829

2930
// ObjectMetaNameAnnotation contains the meta name that should be used for the object in the presentation layer,
@@ -73,6 +74,26 @@ const (
7374
ObjectZOrderAnnotation = "tree.cluster.x-k8s.io.io/z-order"
7475
)
7576

77+
// ConditionFilterType defines the type for condition filters.
78+
type ConditionFilterType string
79+
80+
const (
81+
// ShownNoConditions should be used when no conditions must be used for an object.
82+
ShownNoConditions ConditionFilterType = ""
83+
84+
// ShowAllConditions should be used when all the conditions for an object must be shown.
85+
ShowAllConditions ConditionFilterType = "All"
86+
87+
// ShowNonZeroConditions should be used when only non-zero conditions for an object must be shown.
88+
// Non-zero conditions are conditions with a message set or with status different from the normal state
89+
// for a given condition polarity (e.g. for positive polarity normal state is True, so the non-zero
90+
// status are Unknown and False).
91+
ShowNonZeroConditions ConditionFilterType = "NonZero"
92+
)
93+
94+
// ShowNonZeroConditionsSuffix defines the suffix to be used when the ShowNonZeroConditions filter should be applied.
95+
const ShowNonZeroConditionsSuffix = "+"
96+
7697
// GetMetaName returns the object meta name that should be used for the object in the presentation layer, if defined.
7798
func GetMetaName(obj client.Object) string {
7899
if val, ok := getAnnotation(obj, ObjectMetaNameAnnotation); ok {
@@ -181,12 +202,22 @@ func IsVirtualObject(obj client.Object) bool {
181202
return false
182203
}
183204

184-
// IsShowConditionsObject returns true if the presentation layer should show all the conditions for the object.
185-
func IsShowConditionsObject(obj client.Object) bool {
186-
if val, ok := getBoolAnnotation(obj, ShowObjectConditionsAnnotation); ok {
187-
return val
205+
// ShowConditionsFilter returns the filter to be used by the presentation layer when showing conditions
206+
// for an object.
207+
func ShowConditionsFilter(obj client.Object) ConditionFilterType {
208+
switch val, _ := getAnnotation(obj, ShowObjectConditionsAnnotation); val {
209+
case "All":
210+
return ShowAllConditions
211+
case "NonZero":
212+
return ShowNonZeroConditions
188213
}
189-
return false
214+
return ShownNoConditions
215+
}
216+
217+
// IsShowConditionsObject returns true if the presentation layer should show all the conditions for the object
218+
// or a subset of them.
219+
func IsShowConditionsObject(obj client.Object) bool {
220+
return ShowConditionsFilter(obj) != ShownNoConditions
190221
}
191222

192223
func getAnnotation(obj client.Object, annotation string) (string, bool) {

‎cmd/clusterctl/client/tree/discovery.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import (
3636
// DiscoverOptions define options for the discovery process.
3737
type DiscoverOptions struct {
3838
// ShowOtherConditions is a list of comma separated kind or kind/name for which we should add the ShowObjectConditionsAnnotation
39-
// to signal to the presentation layer to show all the conditions for the objects.
39+
// to signal to the presentation layer to show conditions for the objects.
4040
ShowOtherConditions string
4141

4242
// ShowMachineSets instructs the discovery process to include machine sets in the ObjectTree.

‎cmd/clusterctl/client/tree/tree.go‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import (
3434
// ObjectTreeOptions defines the options for an ObjectTree.
3535
type ObjectTreeOptions struct {
3636
// ShowOtherConditions is a list of comma separated kind or kind/name for which we should add the ShowObjectConditionsAnnotation
37-
// to signal to the presentation layer to show all the conditions for the objects.
37+
// to signal to the presentation layer to show the conditions for the objects and also which filter to apply.
3838
ShowOtherConditions string
3939

4040
// ShowMachineSets instructs the discovery process to include machine sets in the ObjectTree.
@@ -74,11 +74,9 @@ type ObjectTree struct {
7474

7575
// NewObjectTree creates a new object tree with the given root and options.
7676
func NewObjectTree(root client.Object, options ObjectTreeOptions) *ObjectTree {
77-
// If it is requested to show all the conditions for the root, add
77+
// If it is requested to show conditions for the root, add
7878
// the ShowObjectConditionsAnnotation to signal this to the presentation layer.
79-
if isObjDebug(root, options.ShowOtherConditions) {
80-
addAnnotation(root, ShowObjectConditionsAnnotation, "True")
81-
}
79+
addAnnotation(root, ShowObjectConditionsAnnotation, string(showConditions(root, options.ShowOtherConditions)))
8280

8381
return &ObjectTree{
8482
root: root,
@@ -116,11 +114,9 @@ func (od ObjectTree) Add(parent, obj client.Object, opts ...AddObjectOption) (ad
116114
parentReady = GetReadyCondition(parent)
117115
}
118116

119-
// If it is requested to show all the conditions for the object, add
117+
// If it is requested to show conditions for the object, add
120118
// the ShowObjectConditionsAnnotation to signal this to the presentation layer.
121-
if isObjDebug(obj, od.options.ShowOtherConditions) {
122-
addAnnotation(obj, ShowObjectConditionsAnnotation, "True")
123-
}
119+
addAnnotation(obj, ShowObjectConditionsAnnotation, string(showConditions(obj, od.options.ShowOtherConditions)))
124120

125121
// If echo should be dropped from the ObjectTree, return if the object's ready condition is true, and it is the same it has of parent's object ready condition (it is an echo).
126122
// Note: the Echo option applies only for infrastructure machine or bootstrap config objects, and for those objects only Ready condition makes sense.
@@ -497,28 +493,36 @@ func updateV1Beta1GroupNode(groupObj client.Object, groupReady *clusterv1.Condit
497493
}
498494
}
499495

500-
func isObjDebug(obj client.Object, debugFilter string) bool {
501-
if debugFilter == "" {
502-
return false
496+
func showConditions(obj client.Object, showOtherConditions string) ConditionFilterType {
497+
if showOtherConditions == "" {
498+
return ShownNoConditions
503499
}
504-
for _, filter := range strings.Split(strings.ToLower(debugFilter), ",") {
505-
filter = strings.TrimSpace(filter)
500+
for _, filter := range strings.Split(showOtherConditions, ",") {
506501
if filter == "" {
507502
continue
508503
}
509-
if strings.EqualFold(filter, "all") {
510-
return true
504+
if strings.EqualFold("all", strings.TrimSuffix(filter, ShowNonZeroConditionsSuffix)) {
505+
if strings.HasSuffix(filter, ShowNonZeroConditionsSuffix) {
506+
return ShowNonZeroConditions
507+
}
508+
return ShowAllConditions
511509
}
512510
kn := strings.Split(filter, "/")
513511
if len(kn) == 2 {
514-
if strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) == kn[0] && obj.GetName() == kn[1] {
515-
return true
512+
if strings.EqualFold(obj.GetObjectKind().GroupVersionKind().Kind, kn[0]) && strings.EqualFold(obj.GetName(), strings.TrimSuffix(kn[1], ShowNonZeroConditionsSuffix)) {
513+
if strings.HasSuffix(kn[1], ShowNonZeroConditionsSuffix) {
514+
return ShowNonZeroConditions
515+
}
516+
return ShowAllConditions
516517
}
517518
continue
518519
}
519-
if strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) == kn[0] {
520-
return true
520+
if strings.EqualFold(obj.GetObjectKind().GroupVersionKind().Kind, strings.TrimSuffix(kn[0], ShowNonZeroConditionsSuffix)) {
521+
if strings.HasSuffix(kn[0], ShowNonZeroConditionsSuffix) {
522+
return ShowNonZeroConditions
523+
}
524+
return ShowAllConditions
521525
}
522526
}
523-
return false
527+
return ShownNoConditions
524528
}

‎cmd/clusterctl/client/tree/tree_test.go‎

Lines changed: 49 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -438,64 +438,85 @@ func Test_minLastTransitionTimeV1Beta1(t *testing.T) {
438438
}
439439
}
440440

441-
func Test_isObjDebug(t *testing.T) {
441+
func Test_showConditions(t *testing.T) {
442442
obj := fakeMachine("my-machine")
443443
type args struct {
444444
filter string
445445
}
446446
tests := []struct {
447447
name string
448448
args args
449-
want bool
449+
want ConditionFilterType
450450
}{
451451
{
452-
name: "empty filter should return false",
452+
name: "empty filter should return empty string",
453453
args: args{
454454
filter: "",
455455
},
456-
want: false,
456+
want: ShownNoConditions,
457457
},
458458
{
459-
name: "all filter should return true",
459+
name: "all filter should return All",
460460
args: args{
461461
filter: "all",
462462
},
463-
want: true,
463+
want: ShowAllConditions,
464464
},
465465
{
466-
name: "kind filter should return true",
466+
name: "kind filter should return All",
467467
args: args{
468468
filter: "Machine",
469469
},
470-
want: true,
470+
want: ShowAllConditions,
471471
},
472472
{
473-
name: "another kind filter should return false",
473+
name: "another kind filter should return empty string",
474474
args: args{
475475
filter: "AnotherKind",
476476
},
477-
want: false,
477+
want: ShownNoConditions,
478478
},
479479
{
480-
name: "kind/name filter should return true",
480+
name: "kind/name filter should return All",
481481
args: args{
482482
filter: "Machine/my-machine",
483483
},
484-
want: true,
484+
want: ShowAllConditions,
485485
},
486486
{
487-
name: "kind/wrong name filter should return false",
487+
name: "kind/wrong name filter should return empty string",
488488
args: args{
489489
filter: "Cluster/another-cluster",
490490
},
491-
want: false,
491+
want: ShownNoConditions,
492+
},
493+
{
494+
name: "all! filter should return NonZero",
495+
args: args{
496+
filter: "all" + ShowNonZeroConditionsSuffix,
497+
},
498+
want: ShowNonZeroConditions,
499+
},
500+
{
501+
name: "kind! filter should return NonZero",
502+
args: args{
503+
filter: "Machine" + ShowNonZeroConditionsSuffix,
504+
},
505+
want: ShowNonZeroConditions,
506+
},
507+
{
508+
name: "kind/name filter should return NonZero",
509+
args: args{
510+
filter: "Machine/my-machine" + ShowNonZeroConditionsSuffix,
511+
},
512+
want: ShowNonZeroConditions,
492513
},
493514
}
494515
for _, tt := range tests {
495516
t.Run(tt.name, func(t *testing.T) {
496517
g := NewWithT(t)
497518

498-
got := isObjDebug(obj, tt.args.filter)
519+
got := showConditions(obj, tt.args.filter)
499520
g.Expect(got).To(Equal(tt.want))
500521
})
501522
}
@@ -842,21 +863,28 @@ func Test_Add_setsShowObjectConditionsAnnotation(t *testing.T) {
842863
tests := []struct {
843864
name string
844865
args args
845-
want bool
866+
want string
846867
}{
847868
{
848-
name: "filter selecting my machine should not add the annotation",
869+
name: "filter selecting my machine should add the annotation with All",
849870
args: args{
850871
treeOptions: ObjectTreeOptions{ShowOtherConditions: "all"},
851872
},
852-
want: true,
873+
want: "All",
853874
},
854875
{
855-
name: "filter not selecting my machine should not add the annotation",
876+
name: "filter selecting my machine should add the annotation with NonZero",
877+
args: args{
878+
treeOptions: ObjectTreeOptions{ShowOtherConditions: "all" + ShowNonZeroConditionsSuffix},
879+
},
880+
want: "NonZero",
881+
},
882+
{
883+
name: "filter not selecting my machine should add the annotation with empty value",
856884
args: args{
857885
treeOptions: ObjectTreeOptions{ShowOtherConditions: ""},
858886
},
859-
want: false,
887+
want: "",
860888
},
861889
}
862890
for _, tt := range tests {
@@ -874,13 +902,7 @@ func Test_Add_setsShowObjectConditionsAnnotation(t *testing.T) {
874902

875903
gotObj := tree.GetObject("my-machine")
876904
g.Expect(gotObj).ToNot(BeNil())
877-
switch tt.want {
878-
case true:
879-
g.Expect(gotObj.GetAnnotations()).To(HaveKey(ShowObjectConditionsAnnotation))
880-
g.Expect(gotObj.GetAnnotations()[ShowObjectConditionsAnnotation]).To(Equal("True"))
881-
case false:
882-
g.Expect(gotObj.GetAnnotations()).ToNot(HaveKey(ShowObjectConditionsAnnotation))
883-
}
905+
g.Expect(gotObj.GetAnnotations()).To(HaveKeyWithValue(ShowObjectConditionsAnnotation, tt.want))
884906
})
885907
}
886908
}

‎cmd/clusterctl/cmd/describe_cluster.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package cmd
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"os"
2223

2324
"github.com/fatih/color"
@@ -26,6 +27,7 @@ import (
2627

2728
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
2829
"sigs.k8s.io/cluster-api/cmd/clusterctl/client"
30+
"sigs.k8s.io/cluster-api/cmd/clusterctl/client/tree"
2931
"sigs.k8s.io/cluster-api/cmd/clusterctl/cmd/internal/templates"
3032
cmdtree "sigs.k8s.io/cluster-api/internal/util/tree"
3133
)
@@ -93,7 +95,7 @@ func init() {
9395
"The namespace where the workload cluster is located. If unspecified, the current namespace will be used.")
9496

9597
describeClusterClusterCmd.Flags().StringVar(&dc.showOtherConditions, "show-conditions", "",
96-
"list of comma separated kind or kind/name for which the command should show all the object's conditions (use 'all' to show conditions for everything).")
98+
fmt.Sprintf("list of comma separated kind or kind/name for which the command should show all the object's conditions (use 'all' to show conditions for everything, use the %s suffix to show only non-zero conditions).", tree.ShowNonZeroConditionsSuffix))
9799
describeClusterClusterCmd.Flags().BoolVar(&dc.showMachineSets, "show-machinesets", false,
98100
"Show MachineSet objects.")
99101
describeClusterClusterCmd.Flags().BoolVar(&dc.showClusterResourceSets, "show-resourcesets", false,

0 commit comments

Comments
 (0)