🌱 Bumping tablewriter to v1.0.9 - latest #12781

irapandey · 2025-09-18T13:29:32Z

What this PR does / why we need it:
This PR bumps up the github.com/olekukonko/tablewriter to latest (v1.0.9)

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #12318

/area misc

k8s-ci-robot · 2025-09-18T13:29:42Z

Hi @irapandey. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

irapandey · 2025-09-18T13:41:16Z

// tbl.SetCenterSeparator("") 
// tbl.SetRowSeparator("")
// tbl.SetRowSeparator("")

These need not to be set separately as they are set by default as "" - https://github.com/olekukonko/tablewriter/blob/e520aed3bd36fcbc5935d6c4f778c38627788d1d/tw/symbols.go#L885

irapandey · 2025-09-19T10:37:52Z

ira@Iras-MacBook-Pro cluster-api % go test -v ./internal/util/tree
=== RUN   Test_getRowName
=== RUN   Test_getRowName/Row_name_for_objects_should_be_kind/name
=== RUN   Test_getRowName/Row_name_for_a_deleting_object_should_have_deleted_prefix
=== RUN   Test_getRowName/Row_name_for_objects_with_meta_name_should_be_meta-name_-_kind/name
=== RUN   Test_getRowName/Row_name_for_virtual_objects_should_be_name
=== RUN   Test_getRowName/Row_name_for_group_objects_should_be_#-of-items_kind
--- PASS: Test_getRowName (0.00s)
    --- PASS: Test_getRowName/Row_name_for_objects_should_be_kind/name (0.00s)
    --- PASS: Test_getRowName/Row_name_for_a_deleting_object_should_have_deleted_prefix (0.00s)
    --- PASS: Test_getRowName/Row_name_for_objects_with_meta_name_should_be_meta-name_-_kind/name (0.00s)
    --- PASS: Test_getRowName/Row_name_for_virtual_objects_should_be_name (0.00s)
    --- PASS: Test_getRowName/Row_name_for_group_objects_should_be_#-of-items_kind (0.00s)
=== RUN   Test_newConditionDescriptor_readyColor
=== RUN   Test_newConditionDescriptor_readyColor/True_condition_should_be_green
=== RUN   Test_newConditionDescriptor_readyColor/Unknown_condition_should_be_white
=== RUN   Test_newConditionDescriptor_readyColor/False_condition,_severity_error_should_be_red
=== RUN   Test_newConditionDescriptor_readyColor/False_condition,_severity_warning_should_be_yellow
=== RUN   Test_newConditionDescriptor_readyColor/False_condition,_severity_info_should_be_white
=== RUN   Test_newConditionDescriptor_readyColor/Condition_without_status_should_be_gray
--- PASS: Test_newConditionDescriptor_readyColor (0.00s)
    --- PASS: Test_newConditionDescriptor_readyColor/True_condition_should_be_green (0.00s)
    --- PASS: Test_newConditionDescriptor_readyColor/Unknown_condition_should_be_white (0.00s)
    --- PASS: Test_newConditionDescriptor_readyColor/False_condition,_severity_error_should_be_red (0.00s)
    --- PASS: Test_newConditionDescriptor_readyColor/False_condition,_severity_warning_should_be_yellow (0.00s)
    --- PASS: Test_newConditionDescriptor_readyColor/False_condition,_severity_info_should_be_white (0.00s)
    --- PASS: Test_newConditionDescriptor_readyColor/Condition_without_status_should_be_gray (0.00s)
=== RUN   Test_newConditionDescriptor_truncateMessages
=== RUN   Test_newConditionDescriptor_truncateMessages/Short_messages_are_not_changed
=== RUN   Test_newConditionDescriptor_truncateMessages/Long_message_are_truncated
--- PASS: Test_newConditionDescriptor_truncateMessages (0.00s)
    --- PASS: Test_newConditionDescriptor_truncateMessages/Short_messages_are_not_changed (0.00s)
    --- PASS: Test_newConditionDescriptor_truncateMessages/Long_message_are_truncated (0.00s)
=== RUN   Test_V1Beta1TreePrefix
=== RUN   Test_V1Beta1TreePrefix/First_level_child_should_get_the_right_prefix
=== RUN   Test_V1Beta1TreePrefix/Second_level_child_should_get_the_right_prefix
=== RUN   Test_V1Beta1TreePrefix/Conditions_should_get_the_right_prefix
=== RUN   Test_V1Beta1TreePrefix/Conditions_should_get_the_right_prefix_if_the_object_has_a_child
--- PASS: Test_V1Beta1TreePrefix (0.00s)
    --- PASS: Test_V1Beta1TreePrefix/First_level_child_should_get_the_right_prefix (0.00s)
    --- PASS: Test_V1Beta1TreePrefix/Second_level_child_should_get_the_right_prefix (0.00s)
    --- PASS: Test_V1Beta1TreePrefix/Conditions_should_get_the_right_prefix (0.00s)
    --- PASS: Test_V1Beta1TreePrefix/Conditions_should_get_the_right_prefix_if_the_object_has_a_child (0.00s)
=== RUN   Test_TreePrefix
=== RUN   Test_TreePrefix/Conditions_should_get_the_right_prefix_with_multiline_message
=== RUN   Test_TreePrefix/Conditions_should_get_the_right_prefix_with_multiline_message_and_a_child
=== RUN   Test_TreePrefix/Multiple_nested_childs_should_get_the_right_multiline_prefix
=== RUN   Test_TreePrefix/Nested_childs_should_get_the_right_prefix_with_multiline_message
=== RUN   Test_TreePrefix/Conditions_should_get_the_right_prefix_with_childs
--- PASS: Test_TreePrefix (0.00s)
    --- PASS: Test_TreePrefix/Conditions_should_get_the_right_prefix_with_multiline_message (0.00s)
    --- PASS: Test_TreePrefix/Conditions_should_get_the_right_prefix_with_multiline_message_and_a_child (0.00s)
    --- PASS: Test_TreePrefix/Multiple_nested_childs_should_get_the_right_multiline_prefix (0.00s)
    --- PASS: Test_TreePrefix/Nested_childs_should_get_the_right_prefix_with_multiline_message (0.00s)
    --- PASS: Test_TreePrefix/Conditions_should_get_the_right_prefix_with_childs (0.00s)
=== RUN   Test_formatParagraph
=== RUN   Test_formatParagraph/0
=== RUN   Test_formatParagraph/1
=== RUN   Test_formatParagraph/2
--- PASS: Test_formatParagraph (0.00s)
    --- PASS: Test_formatParagraph/0 (0.00s)
    --- PASS: Test_formatParagraph/1 (0.00s)
    --- PASS: Test_formatParagraph/2 (0.00s)
PASS
ok  	sigs.k8s.io/cluster-api/internal/util/tree	0.443s
ira@Iras-MacBook-Pro cluster-api %

irapandey · 2025-09-19T10:39:12Z

The changes in go.sum and go.mod apart from the tablewriter originated from go mod tidy

irapandey · 2025-09-19T10:45:47Z

Used this guide to migrate from v0.0.5 to v1.0.0+ - https://github.com/olekukonko/tablewriter/blob/e520aed3bd36fcbc5935d6c4f778c38627788d1d/MIGRATION.md

irapandey · 2025-09-19T11:09:43Z

neolit123

best to show the before and after output in text / screenehots.

internal/util/tree/tree.go

neolit123 · 2025-09-19T14:03:04Z

/ok-to-test

irapandey · 2025-09-22T04:17:47Z

Hey @neolit123

I did tilt-up with main branch and my feature branch

With tablewriter v0.0.5

With tablewriter v1.0.9

The expected result for unit tests are unchanged and tests are passing successfully

irapandey · 2025-09-22T05:25:09Z

WIP - linting with error message Error return value of ---- is not checked (errcheck)

neolit123

added some comments. thanks for the updates.
please keep the commits squashed to 1.

neolit123 · 2025-09-22T08:37:32Z

internal/util/tree/tree.go

 	// Prints the output table
-	tbl.Render()
+	if err := tbl.Render(); err != nil {
+		fmt.Printf("Error rendering table: %v", err)


fprint to w. same for other cases below.

neolit123 · 2025-09-22T08:38:45Z

internal/util/tree/tree.go

+	return tbl
+}
+
+// CreateObjectTreeV1Beta1 creates a new tablewriter.Table for the object tree.


can there be a single function?
if not best to clarify this in the comment.

I suppose they can be clubbed in - there is a difference for an extra space
But they are different versions and clubbing together doesn't seem to be a great idea IMO

Thoughts? @sbueringer

cc @fabriziopandini

i'm still in favor to have a single function.
function arguments can determine its behavior.

this part can be reused

// Creates the output table tbl := tablewriter.NewTable(w, tablewriter.WithConfig(cfg), tablewriter.WithRendition(tw.Rendition{ Settings: tw.Settings{ Separators: tw.SeparatorsNone, Lines: tw.LinesNone, }, Borders: tw.BorderNone, }))

but looks like there are is further separation with addObjectRowV1Beta1 vs addObjectRow, so maybe just keep these separate functions for now.

In general v1beta1 code will be deleted once we drop v1beta1 in a year or so

neolit123 · 2025-09-22T08:40:43Z

internal/util/tree/tree.go

-	tbl.Render()
+	if err := tbl.Render(); err != nil {
+		fmt.Printf("Error rendering table: %v", err)
+		os.Exit(1)


exiting from printing functions does not seem right. better to return errors.

sure - I'll exit the function with return err message

Hey @neolit123 - I was working on returning the err instead of os.Exit - I found that returning error from these functions would impact whole lot of code in terms of error handling

Will have to catch the error as well - I suppose printing the error message should suffice - I am still confused though

I think last time @sivchari also mentioned to handle the error #12320 (comment),
I think may be you can handle error and and keep those changes in seperate commit, based on the further review we can decide later?

how did it work before this PR?
i did not see any exit calls in the old code or maybe i missed them.

it's an indication that the application is not organized well.
errors should propagate up to callers and ideally there shouldn't be any os.Exit calls in the code base other than the main.go. i suggest you do the necessary refactor in this PR in separate commits as @Karthik-K-N suggested ^

how did it work before this PR?
i did not see any exit calls in the old code or maybe i missed them.

I think in the previous version of tablewriter t.Render() never used to return error, Its new in this update.

Yup - the functions append() and render() have now started to return errors - it's one of the major changes in the v1.0.0+ release pf tablewriter package

I'll do the error handling in a separate commit as suggested

Thanks 😄

sbueringer · 2025-09-22T10:20:06Z

@neolit123

please keep the commits squashed to 1.

Usually we don't enforce this in CAPI and just use the squash merge method with tide. Is there a problem with that?

neolit123 · 2025-09-22T10:40:03Z

@neolit123

please keep the commits squashed to 1.

Usually we don't enforce this in CAPI and just use the squash merge method with tide. Is there a problem with that?

nukes gpg signatures, but most commiters don't use them and therefore they will not require you later to revert their unsigned commits.

cmd/clusterctl/cmd/describe_cluster.go

neolit123

seems there is some sort of a bug on the github ui.
Files changed are 10 but there are only 2 listed in the diff.

anyone else seeing this?

irapandey · 2025-10-06T12:22:51Z

any chance you are looking at diff from one commit?

neolit123 · 2025-10-06T12:24:45Z

internal/util/tree/tree.go

+}
+
+// CreateObjectTreeV1Beta1 creates a new tablewriter.Table for the object tree.
+// Returns a new tablewriter.Table for the object tree.


if the two separate functions are kept, best to properly add the Godoc comment here and explain how they differ.

yeah - I have been playing around with options as the diff is merely a space worth of padding

I suppose something like this might be useful

// CreateObjectTree creates a new tablewriter.Table for the object tree. // Returns a new tablewriter.Table for the object tree. func CreateObjectTree(w io.Writer) *tablewriter.Table { return createObjectTreeWithPadding(w, " ") } // CreateObjectTreeV1Beta1 creates a new tablewriter.Table for the object tree. // Returns a new tablewriter.Table for the object tree. func CreateObjectTreeV1Beta1(w io.Writer) *tablewriter.Table { return createObjectTreeWithPadding(w, " ") } // createObjectTreeWithPadding creates a new tablewriter.Table for the object tree with the specified right padding. func createObjectTreeWithPadding(w io.Writer, rightPadding string) *tablewriter.Table { cfg := tablewriter.Config{ Row: tw.CellConfig{ Formatting: tw.CellFormatting{AutoWrap: tw.WrapNone}, Alignment: tw.CellAlignment{Global: tw.AlignLeft}, Padding: tw.CellPadding{Global: tw.Padding{Left: "", Right: rightPadding}}, }, Header: tw.CellConfig{ Alignment: tw.CellAlignment{Global: tw.AlignLeft}, }, Behavior: tw.Behavior{TrimSpace: tw.Off}, } // Creates the output table tbl := tablewriter.NewTable(w, tablewriter.WithConfig(cfg), tablewriter.WithRendition(tw.Rendition{ Settings: tw.Settings{ Separators: tw.SeparatorsNone, Lines: tw.LinesNone, }, Borders: tw.BorderNone, })) return tbl }

if all the functions that have a v1beta1 suffix can be merged with the versions without the suffix that would be ideal as long as it's a simple change.

I suppose that can be done - but the PR would keep stretching on - I can open an issue to make those changes
"Merge all v1beta1 functions into generic functions"
I would need to understand why they are separate in the first place

neolit123 · 2025-10-06T12:28:50Z

any chance you are looking at diff from one commit?

seemed like a temporary bug, it showed 2 files changed for all commits. after a few refreshes it's now working.

neolit123 · 2025-10-06T12:30:15Z

/label tide/merge-method-squash

neolit123 · 2025-10-06T12:31:59Z

best for someone else to review as well.

/approve
/hold
/area clusterctl

k8s-ci-robot · 2025-10-06T12:32:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neolit123

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [neolit123]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sbueringer · 2025-10-06T13:02:13Z

/assign @fabriziopandini

k8s-ci-robot · 2025-10-07T11:19:27Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

sbueringer · 2025-10-07T15:32:47Z

@irapandey Can you please rebase? Thank you!

fabriziopandini

Great work!
just a few nits from my side

fabriziopandini · 2025-10-07T20:24:14Z

internal/util/tree/tree_test.go

+			if err := tbl.Render(); err != nil {
+				fmt.Printf("Error rendering table: %v", err)
+				os.Exit(1)
+			}


Should this be

Suggested change

if err := tbl.Render(); err != nil {

fmt.Printf("Error rendering table: %v", err)

os.Exit(1)

}

g.Expect( tbl.Render()).To(Succeed(), "Error rendering table")

(same in Test_TreePrefix)

fabriziopandini · 2025-10-07T20:34:46Z

internal/util/tree/tree.go


+// CreateObjectTree creates a new tablewriter.Table for the object tree.
+// Returns a new tablewriter.Table for the object tree.
+func CreateObjectTree(w io.Writer) *tablewriter.Table {


should this be

Suggested change

func CreateObjectTree(w io.Writer) *tablewriter.Table {

func createObjectTree(w io.Writer) *tablewriter.Table {

Same for createObjectTreeV1Beta1

fabriziopandini · 2025-10-07T20:49:03Z

internal/util/tree/tree.go

+		Header: tw.CellConfig{
+			Alignment: tw.CellAlignment{Global: tw.AlignLeft},
+		},


From a quick test, if we set

Suggested change

Header: tw.CellConfig{

Alignment: tw.CellAlignment{Global: tw.AlignLeft},

},

Formatting: tw.CellFormatting{AutoWrap: tw.WrapNone},

Alignment: tw.CellAlignment{Global: tw.AlignLeft},

Padding: tw.CellPadding{Global: tw.Padding{Left: "", Right: " "}},

We get a more consistent alignment between headers and rows, wdyt?
(same for CreateObjectTreeV1Beta1)

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-area PR is missing an area label labels Sep 18, 2025

k8s-ci-robot requested review from elmiko and fabriziopandini September 18, 2025 13:29

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 18, 2025

irapandey force-pushed the tablewriter_12318 branch 2 times, most recently from d6abc40 to 54cab07 Compare September 19, 2025 10:37

irapandey marked this pull request as ready for review September 19, 2025 10:39

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 19, 2025

neolit123 reviewed Sep 19, 2025

View reviewed changes

internal/util/tree/tree.go Outdated Show resolved Hide resolved

neolit123 reviewed Sep 19, 2025

View reviewed changes

internal/util/tree/tree.go Outdated Show resolved Hide resolved

neolit123 reviewed Sep 19, 2025

View reviewed changes

internal/util/tree/tree.go Show resolved Hide resolved

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 19, 2025

neolit123 reviewed Sep 22, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 23, 2025

Bumping tablewriter to v1.0.9 - latest

3376a83

irapandey added 2 commits September 25, 2025 12:30

fixing lint issues

4b573fc

adding create table

eb5c4d1

irapandey force-pushed the tablewriter_12318 branch from dd9cf0e to a478afc Compare September 25, 2025 07:00

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 25, 2025

irapandey force-pushed the tablewriter_12318 branch from a478afc to 6262377 Compare September 25, 2025 07:07

Fixed linting issues

c0a7775

irapandey force-pushed the tablewriter_12318 branch from 6262377 to c0a7775 Compare September 25, 2025 08:56

add error handling

51acf57

irapandey force-pushed the tablewriter_12318 branch from d4bf079 to 51acf57 Compare October 6, 2025 09:04

neolit123 reviewed Oct 6, 2025

View reviewed changes

cmd/clusterctl/cmd/describe_cluster.go Outdated Show resolved Hide resolved

use pkg/errors

ed6bce6

neolit123 reviewed Oct 6, 2025

View reviewed changes

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Oct 6, 2025

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. area/clusterctl Issues or PRs related to clusterctl and removed do-not-merge/needs-area PR is missing an area label labels Oct 6, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 6, 2025

k8s-ci-robot assigned fabriziopandini Oct 6, 2025

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 7, 2025

fabriziopandini reviewed Oct 7, 2025

View reviewed changes

	func CreateObjectTree(w io.Writer) *tablewriter.Table {
	func createObjectTree(w io.Writer) *tablewriter.Table {

🌱 Bumping tablewriter to v1.0.9 - latest #12781

Are you sure you want to change the base?

🌱 Bumping tablewriter to v1.0.9 - latest #12781

Conversation

irapandey commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Sep 18, 2025

Uh oh!

irapandey commented Sep 18, 2025

Uh oh!

irapandey commented Sep 19, 2025

Uh oh!

irapandey commented Sep 19, 2025

Uh oh!

irapandey commented Sep 19, 2025

Uh oh!

irapandey commented Sep 19, 2025

Uh oh!

neolit123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

neolit123 commented Sep 19, 2025

Uh oh!

irapandey commented Sep 22, 2025

Uh oh!

irapandey commented Sep 22, 2025

Uh oh!

neolit123 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neolit123 Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neolit123 Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer commented Sep 22, 2025

Uh oh!

neolit123 commented Sep 22, 2025

Uh oh!

Uh oh!

neolit123 left a comment

Choose a reason for hiding this comment

Uh oh!

irapandey commented Oct 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irapandey commented Sep 18, 2025 •

edited

Loading

neolit123 Oct 6, 2025 •

edited

Loading

neolit123 Sep 25, 2025 •

edited

Loading