diff --git a/cmd/clusterctl/client/describe.go b/cmd/clusterctl/client/describe.go index 03c77a287088..96b91d2102b1 100644 --- a/cmd/clusterctl/client/describe.go +++ b/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. diff --git a/cmd/clusterctl/client/tree/annotations.go b/cmd/clusterctl/client/tree/annotations.go index b8c2fa88bc1c..b287a52156c4 100644 --- a/cmd/clusterctl/client/tree/annotations.go +++ b/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) { diff --git a/cmd/clusterctl/client/tree/discovery.go b/cmd/clusterctl/client/tree/discovery.go index 05f44759d579..bc49f4df225f 100644 --- a/cmd/clusterctl/client/tree/discovery.go +++ b/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. diff --git a/cmd/clusterctl/client/tree/tree.go b/cmd/clusterctl/client/tree/tree.go index b1aa9db75f22..4de695c1bf08 100644 --- a/cmd/clusterctl/client/tree/tree.go +++ b/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 } diff --git a/cmd/clusterctl/client/tree/tree_test.go b/cmd/clusterctl/client/tree/tree_test.go index c9195e4155bc..f58f82683766 100644 --- a/cmd/clusterctl/client/tree/tree_test.go +++ b/cmd/clusterctl/client/tree/tree_test.go @@ -438,7 +438,7 @@ 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 @@ -446,56 +446,77 @@ func Test_isObjDebug(t *testing.T) { 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)) }) } } diff --git a/cmd/clusterctl/cmd/describe_cluster.go b/cmd/clusterctl/cmd/describe_cluster.go index ebe1a1c71edf..0ef994dafc5c 100644 --- a/cmd/clusterctl/cmd/describe_cluster.go +++ b/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, diff --git a/internal/util/tree/tree.go b/internal/util/tree/tree.go index 56e95876dae5..d34d7f1e8f4d 100644 --- a/internal/util/tree/tree.go +++ b/internal/util/tree/tree.go @@ -177,7 +177,7 @@ func addObjectRow(prefix string, tbl *tablewriter.Table, objectTree *tree.Object // NOTE: The object name gets manipulated in order to improve readability. name := getRowName(obj) - // If we are going to show all conditions from this object, let's drop the condition picked in the rowDescriptor. + // If we are going to show conditions from this object, let's drop the condition picked in the rowDescriptor. if tree.IsShowConditionsObject(obj) { rowDescriptor.status = "" rowDescriptor.reason = "" @@ -226,10 +226,8 @@ func addObjectRow(prefix string, tbl *tablewriter.Table, objectTree *tree.Object } // If it is required to show all the conditions for the object, add a row for each object's conditions. - if tree.IsShowConditionsObject(obj) { - if err := addOtherConditions(prefix, tbl, objectTree, obj); err != nil { - return errors.Wrap(err, "failed to add other conditions") - } + if err := addOtherConditions(prefix, tbl, objectTree, obj); err != nil { + return errors.Wrap(err, "failed to add other conditions") } // Add a row for each object's children, taking care of updating the tree view prefix. @@ -329,6 +327,11 @@ func addObjectRowV1Beta1(prefix string, tbl *tablewriter.Table, objectTree *tree // addOtherConditions adds a row for each object condition. func addOtherConditions(prefix string, tbl *tablewriter.Table, objectTree *tree.ObjectTree, obj ctrlclient.Object) error { + conditionFilter := tree.ShowConditionsFilter(obj) + if conditionFilter == tree.ShownNoConditions { + return nil + } + // Add a row for each other condition, taking care of updating the tree view prefix. // In this case the tree prefix get a filler, to indent conditions from objects, and eventually a // and additional pipe if the object has children that should be presented after the conditions. @@ -344,18 +347,38 @@ func addOtherConditions(prefix string, tbl *tablewriter.Table, objectTree *tree. clusterv1.RollingOutCondition, clusterv1.ScalingUpCondition, clusterv1.ScalingDownCondition, + clusterv1.MachineUpdatingCondition, clusterv1.RemediatingCondition, ) conditions := tree.GetConditions(obj) - for i := range conditions { - condition := conditions[i] + showConditions := conditions + if conditionFilter == tree.ShowNonZeroConditions { + showConditions = []metav1.Condition{} + for i := range conditions { + condition := conditions[i] + positivePolarity := true + if negativePolarityConditions.Has(condition.Type) { + positivePolarity = false + } + + if condition.Type != clusterv1.AvailableCondition && condition.Type != clusterv1.ReadyCondition { + if conditionIsZero(condition, positivePolarity) { + continue + } + } + showConditions = append(showConditions, condition) + } + } + + for i := range showConditions { + condition := showConditions[i] positivePolarity := true if negativePolarityConditions.Has(condition.Type) { positivePolarity = false } - childPrefix := getChildPrefix(prefix+childrenPipe+filler, i, len(conditions)) + childPrefix := getChildPrefix(prefix+childrenPipe+filler, i, len(showConditions)) c, status, age, reason, message := conditionInfo(condition, positivePolarity) // Add the row representing each condition. @@ -590,7 +613,7 @@ func newRowDescriptor(obj ctrlclient.Object) rowDescriptor { switch obj := obj.(type) { case *clusterv1.Cluster: // If the object is a cluster, returns all the replica counters (CP and worker replicas are summed for sake of simplicity); - // also, pick the available condition as the condition to show for this object in case not all the conditions are visualized. + // also, pick the Available condition as the condition to show for this object in case not all the conditions are visualized. cp := obj.Status.ControlPlane if cp == nil { cp = &clusterv1.ClusterControlPlaneStatus{} @@ -620,7 +643,7 @@ func newRowDescriptor(obj ctrlclient.Object) rowDescriptor { v.message = availableMessage } case *clusterv1.MachineDeployment: - // If the object is a MachineDeployment, returns all the replica counters and pick the available condition + // If the object is a MachineDeployment, returns all the replica counters and pick the Available condition // as the condition to show for this object in case not all the conditions are visualized. if obj.Spec.Replicas != nil { v.replicas = fmt.Sprintf("%d/%d", *obj.Spec.Replicas, ptr.Deref(obj.Status.Replicas, 0)) @@ -644,8 +667,8 @@ func newRowDescriptor(obj ctrlclient.Object) rowDescriptor { } case *clusterv1.MachineSet: - // If the object is a MachineSet, returns all the replica counters and pick the available condition - // as the condition to show for this object in case not all the conditions are visualized. + // If the object is a MachineSet, returns all the replica counters; no condition + // is shown for this object in case not all the conditions are visualized. if obj.Spec.Replicas != nil { v.replicas = fmt.Sprintf("%d/%d", *obj.Spec.Replicas, ptr.Deref(obj.Status.Replicas, 0)) } @@ -661,7 +684,7 @@ func newRowDescriptor(obj ctrlclient.Object) rowDescriptor { case *clusterv1.Machine: // If the object is a Machine, use Available, Ready and UpToDate conditions to infer replica counters; - // additionally pick the ready condition as the condition to show for this object in case not all the conditions are visualized. + // additionally pick the Ready condition as the condition to show for this object in case not all the conditions are visualized. v.replicas = "1" v.availableCounters = "0" @@ -691,7 +714,7 @@ func newRowDescriptor(obj ctrlclient.Object) rowDescriptor { } case *unstructured.Unstructured: - // If the object is a Unstructured, pick the ready condition as the condition to show for this object + // If the object is a Unstructured, pick the Ready condition as the condition to show for this object // in case not all the conditions are visualized. // Also, if the Unstructured object implements the Cluster API control plane contract, surface // corresponding replica counters. @@ -703,8 +726,9 @@ func newRowDescriptor(obj ctrlclient.Object) rowDescriptor { v.message = readyMessage } + // if the unstructured object is a ControlPlane, pick the condition with type Available if it exists (use it instead of ready) + // and also get replica counters if defined. if tree.GetObjectContract(obj) == "ControlPlane" { - // if a condition with type Available exist for a CP object, use it instead of ready. if available := tree.GetAvailableCondition(obj); available != nil { availableColor, availableStatus, availableAge, availableReason, availableMessage := conditionInfo(*available, true) v.status = availableColor.Sprintf("%s", availableStatus) @@ -734,7 +758,7 @@ func newRowDescriptor(obj ctrlclient.Object) rowDescriptor { case *tree.NodeObject: // If the object represent a group of objects, surface the corresponding replica counters. - // Also, pick the ready condition as the condition to show for this group. + // Also, pick the Ready condition as the condition to show for this group. if tree.IsGroupObject(obj) { v.readyCounters = fmt.Sprintf("%d", tree.GetGroupItemsReadyCounter(obj)) v.availableCounters = fmt.Sprintf("%d", tree.GetGroupItemsAvailableCounter(obj)) @@ -753,6 +777,22 @@ func newRowDescriptor(obj ctrlclient.Object) rowDescriptor { return v } +func conditionIsZero(c metav1.Condition, positivePolarity bool) bool { + switch c.Status { + case metav1.ConditionFalse: + if positivePolarity { + return false + } + case metav1.ConditionUnknown: + return false + case metav1.ConditionTrue: + if !positivePolarity { + return false + } + } + return c.Message == "" +} + func conditionInfo(c metav1.Condition, positivePolarity bool) (color *color.Color, status, age, reason, message string) { switch c.Status { case metav1.ConditionFalse: diff --git a/internal/util/tree/tree_test.go b/internal/util/tree/tree_test.go index 8422198d0dc5..2f22348f398f 100644 --- a/internal/util/tree/tree_test.go +++ b/internal/util/tree/tree_test.go @@ -210,15 +210,15 @@ func Test_V1Beta1TreePrefix(t *testing.T) { name: "Conditions should get the right prefix", objectTree: func() *tree.ObjectTree { root := fakeObject("root") - obectjTree := tree.NewObjectTree(root, tree.ObjectTreeOptions{}) + obectjTree := tree.NewObjectTree(root, tree.ObjectTreeOptions{ + ShowOtherConditions: "All", + }) o1 := fakeObject("child1", - withAnnotation(tree.ShowObjectConditionsAnnotation, "True"), withV1Beta1Condition(v1beta1conditions.TrueCondition("C1.1")), withV1Beta1Condition(v1beta1conditions.TrueCondition("C1.2")), ) o2 := fakeObject("child2", - withAnnotation(tree.ShowObjectConditionsAnnotation, "True"), withV1Beta1Condition(v1beta1conditions.TrueCondition("C2.1")), withV1Beta1Condition(v1beta1conditions.TrueCondition("C2.2")), ) @@ -240,17 +240,17 @@ func Test_V1Beta1TreePrefix(t *testing.T) { name: "Conditions should get the right prefix if the object has a child", objectTree: func() *tree.ObjectTree { root := fakeObject("root") - obectjTree := tree.NewObjectTree(root, tree.ObjectTreeOptions{}) + obectjTree := tree.NewObjectTree(root, tree.ObjectTreeOptions{ + ShowOtherConditions: "All", + }) o1 := fakeObject("child1", - withAnnotation(tree.ShowObjectConditionsAnnotation, "True"), withV1Beta1Condition(v1beta1conditions.TrueCondition("C1.1")), withV1Beta1Condition(v1beta1conditions.TrueCondition("C1.2")), ) o1_1 := fakeObject("child1.1") o2 := fakeObject("child2", - withAnnotation(tree.ShowObjectConditionsAnnotation, "True"), withV1Beta1Condition(v1beta1conditions.TrueCondition("C2.1")), withV1Beta1Condition(v1beta1conditions.TrueCondition("C2.2")), ) @@ -289,7 +289,8 @@ func Test_V1Beta1TreePrefix(t *testing.T) { // Compare the output with the expected prefix. // We only check whether the output starts with the expected prefix, // meaning expectPrefix does not contain the full expected output. - g.Expect(output.String()).Should(MatchTable(tt.expectPrefix)) + outputString := output.String() + g.Expect(outputString).Should(MatchTable(tt.expectPrefix)) }) } }