Improve addons list table design #21288

pavansaikrishna78 · 2025-08-11T13:45:38Z

If status is enabled then the whole row green

command: addons list

command: addons list -d

command: profile list

command: profile list -d

ADDON.LIST.MINIKUBE.docx

k8s-ci-robot · 2025-08-11T13:45:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pavansaikrishna78
Once this PR has been reviewed and has the lgtm label, please assign spowelljr for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

k8s-ci-robot · 2025-08-11T13:45:48Z

Hi @pavansaikrishna78. Thanks for your PR.

I'm waiting for a kubernetes 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.

minikube-bot · 2025-08-11T13:48:03Z

Can one of the admins verify this patch?

medyagh · 2025-08-11T19:24:57Z

pkg/minikube/constants/constants.go

@@ -163,6 +163,12 @@ const (

 	// Mirror CN
 	AliyunMirror = "registry.cn-hangzhou.aliyuncs.com/google_containers"
+
+	//TABLE WRITER COLORS


is there a way to show how it looks like maybe a link or copy to the comment ?
is a library to use?

https://gist.github.com/JBlond/2fea43a3049b38287e5e9cefc87b2124

medyagh · 2025-08-11T19:26:52Z

cmd/minikube/cmd/config/addons_list.go

 		if cc == nil {
 			temp = []string{addonName, maintainer}
 		} else {
 			enabled := addonBundle.IsEnabled(cc)
-			temp = []string{addonName, cc.Name, fmt.Sprintf("%s %s", stringFromStatus(enabled), iconFromStatus(enabled)), maintainer}
+			if enabled{
+				status := fmt.Sprintf("%s%s%s", constants.Enabled, iconFromStatus(enabled), constants.Default)


add a helper func to Format the text

we have a style package
style.Enable(.....)

medyagh · 2025-08-11T19:32:43Z

cmd/minikube/cmd/config/profile_list.go

-				cpIP, strconv.Itoa(cpPort), k8sVersion, p.Status, strconv.Itoa(len(p.Config.Nodes)), c, k})
+			if p.Status == "OK" {
+				data = append(data, []string{
+					fmt.Sprintf("%s%s%s", constants.Enabled, p.Name, constants.Default),


we should have a style func that does
style.Normal( white color)
style.Enabled(
Style.Trouble(

medyagh · 2025-08-11T19:33:46Z

cmd/minikube/cmd/config/profile_list.go

@@ -179,11 +168,77 @@ func profilesToTableData(profiles []*config.Profile) [][]string {
 			k = "*"
 		}
 		if isDetailed {
-			data = append(data, []string{p.Name, p.Config.Driver, p.Config.KubernetesConfig.ContainerRuntime,
-				cpIP, strconv.Itoa(cpPort), k8sVersion, p.Status, strconv.Itoa(len(p.Config.Nodes)), c, k})
+			if p.Status == "OK" {


this should be an iteration loop that calls the and based on the status it will call the Style Function in the package.

nirs

Please split the logical changes to separate commits instead of posing a new commit on every change.

If tests fail fix the failing commit. Don't post another commit fixing the first commit. This makes reviewing the changes much harder. Adding fix commit locally is fine, but before you post a new version you should squash the fix commit into the commit that introduced the problem.

Please remove all the unrelated and unnecessary changes. Focus on only on the design change to keep the change small, easy to review and be able to complete it quickly.

nirs · 2025-08-16T18:51:30Z

pkg/minikube/constants/constants.go

@@ -163,6 +163,12 @@ const (

 	// Mirror CN
 	AliyunMirror = "registry.cn-hangzhou.aliyuncs.com/google_containers"
+
+	//TABLE WRITER COLORS
+	Enabled = "\033[32m"


This is green forground color. I will help to add a comment like:

Enabled = "\033[32m" // green forground

nirs · 2025-08-16T18:53:47Z

pkg/minikube/constants/constants.go

+
+	//TABLE WRITER COLORS
+	Enabled = "\033[32m"
+	Default = "\033[0m"


This is not a color but a reset command, removing all attributes (foreground color, background color, effects). We don't need a constant for default since it does not change the text.

nirs · 2025-08-16T18:54:05Z

pkg/minikube/constants/constants.go

+	//TABLE WRITER COLORS
+	Enabled = "\033[32m"
+	Default = "\033[0m"
+	Disabled = "\033[0m"


Same, since this is using the same value as default.

nirs · 2025-08-16T18:54:34Z

pkg/minikube/constants/constants.go

+	Enabled = "\033[32m"
+	Default = "\033[0m"
+	Disabled = "\033[0m"
+	Error = "\033[31m"  


This is red foreground, a comment will help.

But I don't think this is the way to go. What we need is a new package for color that can be reused by code writing output for tables or other output.

This package can use constants to the define the color of enabled, disabled, or errors, and provide functions using this signature:

Enabled(s string) string

Code rendering UI should use the functions to decorate fragments of text.

To implement this we can use a library like this:
https://github.com/fatih/color?tab=readme-ov-file#insert-into-noncolor-strings-sprintfunc

The advantage of using a library is using a well tested code that works on all platforms.

nirs · 2025-08-16T19:05:12Z

cmd/minikube/cmd/config/profile_list.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2019 The Kubernetes Authors All rights reserved.
+Copyright 2025 The Kubernetes Authors All rights reserved.


We don't need to update the dates, that's pointless effort. The best way is to remove the date form the copyright headers, but this is not in the scope of this PR, so just remove this change.

nirs · 2025-08-16T19:21:14Z

cmd/minikube/cmd/config/profile_list.go

+				applyColor = true
+			default:
+				applyColor = false
+			}


This is too complicated for no reason.

nirs · 2025-08-16T19:23:21Z

cmd/minikube/cmd/config/profile_list.go

+					row[i] = fmt.Sprintf("%s%d%s", colorCode, v, constants.Default)
+				default:
+					row[i] = fmt.Sprintf("%s%v%s", colorCode, v, constants.Default)
+				}


Converting the values to string like we did before was better. If we colorize all value in a raw, we can keep the old code as is and apply the right color function to the entire slice.

nirs · 2025-08-16T19:23:57Z

cmd/minikube/cmd/config/profile_list.go

@@ -208,8 +290,7 @@ func warnInvalidProfiles(invalidProfiles []*config.Profile) {
 func printProfilesJSON() {
 	validProfiles, invalidProfiles, err := listProfiles()
 	updateProfilesStatus(validProfiles)
-
-	var body = map[string]interface{}{}
+	body := map[string]interface{}{}


Why did you change this?

nirs · 2025-08-16T19:29:20Z

cmd/minikube/cmd/config/addons_list.go

@@ -107,7 +108,7 @@ var printAddonsList = func(cc *config.ClusterConfig, printDocs bool) {
 	if cc == nil {
 		tHeader = []string{"Addon Name", "Maintainer"}
 	} else {
-		tHeader = []string{"Addon Name", "Profile", "Status", "Maintainer"}
+		tHeader = []string{"Addon Name", "Enabled", "Maintainer"}


We have 3 logic changes:

Removing the profile column

Replacing Status with Enabled

Coloring the output

Each change should be done in a separate commit. This will make it easier to write the code without errors, test it, and review.

nirs · 2025-08-16T20:25:07Z

cmd/minikube/cmd/config/addons_list.go

+				temp[i] = fmt.Sprintf("%s%s%s", valueColorCode, valueStr, constants.Default)
+			} else {
+				temp[i] = valueStr
+			}


Too complicated.

We need 2 steps:

Create list of strings for the table writer

Color the strings

For creating list of strings we can modify the previous code a little bit:

enabled := addonBundle.IsEnabled(cc) row = []string{addonName, cc.Name, iconFromStatus(enabled), maintainer}

iconForStatus() should return empty string if enabled is false. If it does not do this create a function that does this.

Since the list is so simple, we don't really need a loop to color it:

if enabled { row[0] = color.Enabled(row[0]) row[2] = color.Enabled(row[2]) }

But since wan to color longer lists (like profile list), we can add a function to color rows:

type colorFunc func(string) string func ColorRow(row []string, colored colorFunc) { for i := range row { text := row[i] if text == "" || isEmoji(text) { continue } row[i] = colored(text) } }

With this we can do:

switch status { case Broken: color.ColorRow(row, color.Broken) case Error: color.ColorRow(row, color.Error) case Enabled: color.ColorRow(row, color.Enabled) }

addons list and profile list and making pass the test

medyagh

lets plz not do changes not related to this PR
and also check nir's suggetsions how to make the code more simple

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 11, 2025

k8s-ci-robot requested review from nirs and prezha August 11, 2025 13:45

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 11, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 11, 2025

medyagh reviewed Aug 11, 2025

View reviewed changes

nirs reviewed Aug 16, 2025

View reviewed changes

pavansaikrishna78 force-pushed the addon-list branch 2 times, most recently from 3f61c2c to 6bc5fee Compare August 18, 2025 15:11

pavansaikrishna78 force-pushed the addon-list branch from 565498b to cd36cf8 Compare August 18, 2025 15:57

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 18, 2025

addon list for table write package and profile list

789116d

addons list and profile list and making pass the test

pavansaikrishna78 force-pushed the addon-list branch from cd36cf8 to 789116d Compare August 18, 2025 16:23

pavansaikrishna78 force-pushed the addon-list branch from 6aab811 to 789116d Compare August 19, 2025 12:52

pavansaikrishna78 requested a review from nirs August 25, 2025 17:53

medyagh requested changes Aug 25, 2025

View reviewed changes

pavansaikrishna78 and others added 2 commits September 2, 2025 15:55

Merge branch 'kubernetes:master' into addon-list

6dabb12

adding color to addons list and profile list

05e4192

Improve addons list table design #21288

Are you sure you want to change the base?

Improve addons list table design #21288

Conversation

pavansaikrishna78 commented Aug 11, 2025

Uh oh!

k8s-ci-robot commented Aug 11, 2025

Uh oh!

k8s-ci-robot commented Aug 11, 2025

Uh oh!

minikube-bot commented Aug 11, 2025

Uh oh!

medyagh Aug 11, 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!

medyagh Aug 11, 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!

nirs 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!

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!

medyagh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

medyagh Aug 11, 2025 •

edited

Loading

medyagh Aug 11, 2025 •

edited

Loading