Merge pull request #12991 from fabriziopandini/add-condition-filter-for-clusterctl-describe

✨ Add conditions filter for clusterctl describe

Author: k8s-ci-robot

cmd/clusterctl/client/describe.go

@@ -35,7 +35,7 @@ type DescribeClusterOptions struct {
 	ClusterName string
 
 	// ShowOtherConditions is a list of comma separated kind or kind/name for which we should add the ShowObjectConditionsAnnotation
-	// to signal to the presentation layer to show all the conditions for the objects.
+	// to signal to the presentation layer to show conditions for the objects.
 	ShowOtherConditions string
 
 	// ShowMachineSets instructs the discovery process to include machine sets in the ObjectTree.

cmd/clusterctl/client/tree/annotations.go

@@ -23,7 +23,8 @@ import (
 )
 
 const (
-	// ShowObjectConditionsAnnotation documents that the presentation layer should show all the conditions for the object.
+	// ShowObjectConditionsAnnotation documents that the presentation layer should show conditions for the object
+	// and the filter to select those conditions.
 	ShowObjectConditionsAnnotation = "tree.cluster.x-k8s.io.io/show-conditions"
 
 	// ObjectMetaNameAnnotation contains the meta name that should be used for the object in the presentation layer,
@@ -73,6 +74,26 @@ const (
 	ObjectZOrderAnnotation = "tree.cluster.x-k8s.io.io/z-order"
 )
 
+// ConditionFilterType defines the type for condition filters.
+type ConditionFilterType string
+
+const (
+	// ShownNoConditions should be used when no conditions must be used for an object.
+	ShownNoConditions ConditionFilterType = ""
+
+	// ShowAllConditions should be used when all the conditions for an object must be shown.
+	ShowAllConditions ConditionFilterType = "All"
+
+	// ShowNonZeroConditions should be used when only non-zero conditions for an object must be shown.
+	// Non-zero conditions are conditions with a message set or with status different from the normal state
+	// for a given condition polarity (e.g. for positive polarity normal state is True, so the non-zero
+	// status are Unknown and False).
+	ShowNonZeroConditions ConditionFilterType = "NonZero"
+)
+
+// ShowNonZeroConditionsSuffix defines the suffix to be used when the ShowNonZeroConditions filter should be applied.
+const ShowNonZeroConditionsSuffix = "+"
+
 // GetMetaName returns the object meta name that should be used for the object in the presentation layer, if defined.
 func GetMetaName(obj client.Object) string {
 	if val, ok := getAnnotation(obj, ObjectMetaNameAnnotation); ok {
@@ -181,12 +202,22 @@ func IsVirtualObject(obj client.Object) bool {
 	return false
 }
 
-// IsShowConditionsObject returns true if the presentation layer should show all the conditions for the object.
-func IsShowConditionsObject(obj client.Object) bool {
-	if val, ok := getBoolAnnotation(obj, ShowObjectConditionsAnnotation); ok {
-		return val
+// ShowConditionsFilter returns the filter to be used by the presentation layer when showing conditions
+// for an object.
+func ShowConditionsFilter(obj client.Object) ConditionFilterType {
+	switch val, _ := getAnnotation(obj, ShowObjectConditionsAnnotation); val {
+	case "All":
+		return ShowAllConditions
+	case "NonZero":
+		return ShowNonZeroConditions
 	}
-	return false
+	return ShownNoConditions
+}
+
+// IsShowConditionsObject returns true if the presentation layer should show all the conditions for the object
+// or a subset of them.
+func IsShowConditionsObject(obj client.Object) bool {
+	return ShowConditionsFilter(obj) != ShownNoConditions
 }
 
 func getAnnotation(obj client.Object, annotation string) (string, bool) {

cmd/clusterctl/client/tree/discovery.go

@@ -36,7 +36,7 @@ import (
 // DiscoverOptions define options for the discovery process.
 type DiscoverOptions struct {
 	// ShowOtherConditions is a list of comma separated kind or kind/name for which we should add the ShowObjectConditionsAnnotation
-	// to signal to the presentation layer to show all the conditions for the objects.
+	// to signal to the presentation layer to show conditions for the objects.
 	ShowOtherConditions string
 
 	// ShowMachineSets instructs the discovery process to include machine sets in the ObjectTree.

cmd/clusterctl/client/tree/tree.go

@@ -34,7 +34,7 @@ import (
 // ObjectTreeOptions defines the options for an ObjectTree.
 type ObjectTreeOptions struct {
 	// ShowOtherConditions is a list of comma separated kind or kind/name for which we should add   the ShowObjectConditionsAnnotation
-	// to signal to the presentation layer to show all the conditions for the objects.
+	// to signal to the presentation layer to show the conditions for the objects and also which filter to apply.
 	ShowOtherConditions string
 
 	// ShowMachineSets instructs the discovery process to include machine sets in the ObjectTree.
@@ -74,11 +74,9 @@ type ObjectTree struct {
 
 // NewObjectTree creates a new object tree with the given root and options.
 func NewObjectTree(root client.Object, options ObjectTreeOptions) *ObjectTree {
-	// If it is requested to show all the conditions for the root, add
+	// If it is requested to show conditions for the root, add
 	// the ShowObjectConditionsAnnotation to signal this to the presentation layer.
-	if isObjDebug(root, options.ShowOtherConditions) {
-		addAnnotation(root, ShowObjectConditionsAnnotation, "True")
-	}
+	addAnnotation(root, ShowObjectConditionsAnnotation, string(showConditions(root, options.ShowOtherConditions)))
 
 	return &ObjectTree{
 		root:       root,
@@ -116,11 +114,9 @@ func (od ObjectTree) Add(parent, obj client.Object, opts ...AddObjectOption) (ad
 		parentReady = GetReadyCondition(parent)
 	}
 
-	// If it is requested to show all the conditions for the object, add
+	// If it is requested to show conditions for the object, add
 	// the ShowObjectConditionsAnnotation to signal this to the presentation layer.
-	if isObjDebug(obj, od.options.ShowOtherConditions) {
-		addAnnotation(obj, ShowObjectConditionsAnnotation, "True")
-	}
+	addAnnotation(obj, ShowObjectConditionsAnnotation, string(showConditions(obj, od.options.ShowOtherConditions)))
 
 	// 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).
 	// 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
 	}
 }
 
-func isObjDebug(obj client.Object, debugFilter string) bool {
-	if debugFilter == "" {
-		return false
+func showConditions(obj client.Object, showOtherConditions string) ConditionFilterType {
+	if showOtherConditions == "" {
+		return ShownNoConditions
 	}
-	for _, filter := range strings.Split(strings.ToLower(debugFilter), ",") {
-		filter = strings.TrimSpace(filter)
+	for _, filter := range strings.Split(showOtherConditions, ",") {
 		if filter == "" {
 			continue
 		}
-		if strings.EqualFold(filter, "all") {
-			return true
+		if strings.EqualFold("all", strings.TrimSuffix(filter, ShowNonZeroConditionsSuffix)) {
+			if strings.HasSuffix(filter, ShowNonZeroConditionsSuffix) {
+				return ShowNonZeroConditions
+			}
+			return ShowAllConditions
 		}
 		kn := strings.Split(filter, "/")
 		if len(kn) == 2 {
-			if strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) == kn[0] && obj.GetName() == kn[1] {
-				return true
+			if strings.EqualFold(obj.GetObjectKind().GroupVersionKind().Kind, kn[0]) && strings.EqualFold(obj.GetName(), strings.TrimSuffix(kn[1], ShowNonZeroConditionsSuffix)) {
+				if strings.HasSuffix(kn[1], ShowNonZeroConditionsSuffix) {
+					return ShowNonZeroConditions
+				}
+				return ShowAllConditions
 			}
 			continue
 		}
-		if strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) == kn[0] {
-			return true
+		if strings.EqualFold(obj.GetObjectKind().GroupVersionKind().Kind, strings.TrimSuffix(kn[0], ShowNonZeroConditionsSuffix)) {
+			if strings.HasSuffix(kn[0], ShowNonZeroConditionsSuffix) {
+				return ShowNonZeroConditions
+			}
+			return ShowAllConditions
 		}
 	}
-	return false
+	return ShownNoConditions
 }

cmd/clusterctl/client/tree/tree_test.go

@@ -438,64 +438,85 @@ func Test_minLastTransitionTimeV1Beta1(t *testing.T) {
 	}
 }
 
-func Test_isObjDebug(t *testing.T) {
+func Test_showConditions(t *testing.T) {
 	obj := fakeMachine("my-machine")
 	type args struct {
 		filter string
 	}
 	tests := []struct {
 		name string
 		args args
-		want bool
+		want ConditionFilterType
 	}{
 		{
-			name: "empty filter should return false",
+			name: "empty filter should return empty string",
 			args: args{
 				filter: "",
 			},
-			want: false,
+			want: ShownNoConditions,
 		},
 		{
-			name: "all filter should return true",
+			name: "all filter should return All",
 			args: args{
 				filter: "all",
 			},
-			want: true,
+			want: ShowAllConditions,
 		},
 		{
-			name: "kind filter should return true",
+			name: "kind filter should return All",
 			args: args{
 				filter: "Machine",
 			},
-			want: true,
+			want: ShowAllConditions,
 		},
 		{
-			name: "another kind filter should return false",
+			name: "another kind filter should return empty string",
 			args: args{
 				filter: "AnotherKind",
 			},
-			want: false,
+			want: ShownNoConditions,
 		},
 		{
-			name: "kind/name filter should return true",
+			name: "kind/name filter should return All",
 			args: args{
 				filter: "Machine/my-machine",
 			},
-			want: true,
+			want: ShowAllConditions,
 		},
 		{
-			name: "kind/wrong name filter should return false",
+			name: "kind/wrong name filter should return empty string",
 			args: args{
 				filter: "Cluster/another-cluster",
 			},
-			want: false,
+			want: ShownNoConditions,
+		},
+		{
+			name: "all! filter should return NonZero",
+			args: args{
+				filter: "all" + ShowNonZeroConditionsSuffix,
+			},
+			want: ShowNonZeroConditions,
+		},
+		{
+			name: "kind! filter should return NonZero",
+			args: args{
+				filter: "Machine" + ShowNonZeroConditionsSuffix,
+			},
+			want: ShowNonZeroConditions,
+		},
+		{
+			name: "kind/name filter should return NonZero",
+			args: args{
+				filter: "Machine/my-machine" + ShowNonZeroConditionsSuffix,
+			},
+			want: ShowNonZeroConditions,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			got := isObjDebug(obj, tt.args.filter)
+			got := showConditions(obj, tt.args.filter)
 			g.Expect(got).To(Equal(tt.want))
 		})
 	}
@@ -842,21 +863,28 @@ func Test_Add_setsShowObjectConditionsAnnotation(t *testing.T) {
 	tests := []struct {
 		name string
 		args args
-		want bool
+		want string
 	}{
 		{
-			name: "filter selecting my machine should not add the annotation",
+			name: "filter selecting my machine should add the annotation with All",
 			args: args{
 				treeOptions: ObjectTreeOptions{ShowOtherConditions: "all"},
 			},
-			want: true,
+			want: "All",
 		},
 		{
-			name: "filter not selecting my machine should not add the annotation",
+			name: "filter selecting my machine should add the annotation with NonZero",
+			args: args{
+				treeOptions: ObjectTreeOptions{ShowOtherConditions: "all" + ShowNonZeroConditionsSuffix},
+			},
+			want: "NonZero",
+		},
+		{
+			name: "filter not selecting my machine should add the annotation with empty value",
 			args: args{
 				treeOptions: ObjectTreeOptions{ShowOtherConditions: ""},
 			},
-			want: false,
+			want: "",
 		},
 	}
 	for _, tt := range tests {
@@ -874,13 +902,7 @@ func Test_Add_setsShowObjectConditionsAnnotation(t *testing.T) {
 
 				gotObj := tree.GetObject("my-machine")
 				g.Expect(gotObj).ToNot(BeNil())
-				switch tt.want {
-				case true:
-					g.Expect(gotObj.GetAnnotations()).To(HaveKey(ShowObjectConditionsAnnotation))
-					g.Expect(gotObj.GetAnnotations()[ShowObjectConditionsAnnotation]).To(Equal("True"))
-				case false:
-					g.Expect(gotObj.GetAnnotations()).ToNot(HaveKey(ShowObjectConditionsAnnotation))
-				}
+				g.Expect(gotObj.GetAnnotations()).To(HaveKeyWithValue(ShowObjectConditionsAnnotation, tt.want))
 			})
 		}
 	}

cmd/clusterctl/cmd/describe_cluster.go

@@ -18,6 +18,7 @@ package cmd
 
 import (
 	"context"
+	"fmt"
 	"os"
 
 	"github.com/fatih/color"
@@ -26,6 +27,7 @@ import (
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client"
+	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/tree"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/cmd/internal/templates"
 	cmdtree "sigs.k8s.io/cluster-api/internal/util/tree"
 )
@@ -93,7 +95,7 @@ func init() {
 		"The namespace where the workload cluster is located. If unspecified, the current namespace will be used.")
 
 	describeClusterClusterCmd.Flags().StringVar(&dc.showOtherConditions, "show-conditions", "",
-		"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).")
+		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))
 	describeClusterClusterCmd.Flags().BoolVar(&dc.showMachineSets, "show-machinesets", false,
 		"Show MachineSet objects.")
 	describeClusterClusterCmd.Flags().BoolVar(&dc.showClusterResourceSets, "show-resourcesets", false,