Package List (Parameters | Tasks | Plans) #1223

kensipe · 2019-12-24T05:33:49Z

What this PR does / why we need it:

Consolidating "list" as a verb under "package". Instead of kudo package params list, this PR changes to kudo package list parameters and adds kudo package list tasks and kudo package list plans.

This CLI utility functions are useful when:

you are a operator dev and building an operator locally
you want to "discover" what an operator can do (locally or from a repository)

Tasks

go run cmd/kubectl-kudo/main.go package list tasks cassandra
Name	Kind 	Resources                                                                                                                                                                            
node	Apply	[service.yaml cassandra-yaml.yaml cassandra-env-sh.yaml jvm-options.yaml stateful-set.yaml cassandra-exporter-config-yml.yaml service-monitor-yaml.yaml node-readiness-probe-sh.yaml]

go run cmd/kubectl-kudo/main.go package list tasks ../operators/repository/cowsay/operator/
Name  	Kind 	Resources        
app   	Apply	[deployment.yaml]
genwww	Pipe 	pipe-pod.yaml

go run cmd/kubectl-kudo/main.go package list tasks zookeeper
Name              	Kind  	Resources                                                     
infra             	Apply 	[bootstrap.sh.yaml healthcheck.sh.yaml services.yaml pdb.yaml]
app               	Apply 	[statefulset.yaml]                                            
validation        	Apply 	[validation.yaml]                                             
validation-cleanup	Delete	[validation.yaml]                                             
not-allowed       	Apply 	[]

** Plans **

go run cmd/kubectl-kudo/main.go package list plans zookeeper
zookeeper is a repository package from { name:community, url:https://kudo-repository.storage.googleapis.com }
plans
├── deploy (serial)
│   ├── [phase]  zookeeper (parallel)
│   │   └── [step]  deploy
│   │       ├── infra
│   │       └── app
│   └── [phase]  validation (serial)
│       ├── [step]  validation
│       │   └── validation
│       └── [step]  cleanup
│           └── validation-cleanup
└── validation (serial)
    └── [phase]  connection (serial)
        ├── [step]  connection
        │   └── validation
        └── [step]  cleanup
            └── validation-cleanup

or

go run cmd/kubectl-kudo/main.go package list plans zookeeper -t
plans
├── deploy (serial)
│   ├── [phase]  zookeeper (parallel)
│   │   └── [step]  deploy
│   │       ├── [apply]  infra
│   │       │   ├── bootstrap.sh.yaml
│   │       │   ├── healthcheck.sh.yaml
│   │       │   ├── services.yaml
│   │       │   └── pdb.yaml
│   │       └── [apply]  app
│   │           └── statefulset.yaml
│   └── [phase]  validation (serial)
│       ├── [step]  validation
│       │   └── [apply]  validation
│       │       └── validation.yaml
│       └── [step]  cleanup
│           └── [delete]  validation-cleanup
│               └── validation.yaml
└── validation (serial)
    └── [phase]  connection (serial)
        ├── [step]  connection
        │   └── [apply]  validation
        │       └── validation.yaml
        └── [step]  cleanup
            └── [delete]  validation-cleanup
                └── validation.yaml

go run cmd/kubectl-kudo/main.go package list plans ../operators/repository/cowsay/operator/
Name  	Phase	Strategy	Step    	Task    
deploy	main 	serial  	genfiles	[genwww]
      	     	        	app     	[app]

** Parameters **

go run cmd/kubectl-kudo/main.go package list parameters zookeeper
Name         	Default	Required
CLIENT_PORT  	2181   	true    
CPUS         	250m   	true    
CPUS_LIMIT   	1000m  	true    
DISK_SIZE    	5Gi    	true    
ELECTION_PORT	3888   	true    
MEMORY       	1024Mi 	true    
MEM_LIMIT    	2048Mi 	true    
NODE_COUNT   	3      	true    
SERVER_PORT  	2888   	true    
STORAGE_CLASS	       	false

Fixes #

… to add a number of other list types for operators Signed-off-by: Ken Sipe <kensipe@gmail.com>

Signed-off-by: Ken Sipe <kensipe@gmail.com>

kensipe · 2019-12-24T05:34:47Z

needs tests otherwise good to go

alenkacz

Love the rename of the command 👏 much more consistent now. We should not forget to mention it as breaking change though.

I have couple of issues with this:

I don't really understand the use case for the tasks command. As operator I cannot invoke tasks, as developer, I am editing the yaml so why would I list it through CLI when I can just directly open the yaml?
I think the plans list should probably use tree view (just a suggestion)? I think using something like "github.com/xlab/treeprint" as in plan_status would be much nicer. Or we could just list plans without phases?
I think the PR should reference an issue that should be in this iteration with meaningful description. If I remember correctly you were the one proposing everyone follows this :-)
we need tests

plus the comments I had inline :-)

pkg/kudoctl/cmd/package_list.go

pkg/kudoctl/cmd/package_list_plans.go

pkg/kudoctl/cmd/package_list_tasks.go

Signed-off-by: Ken Sipe <kensipe@gmail.com>

kensipe · 2019-12-31T16:49:29Z

@alenkacz pushed updates for basic changes... I love the treeprint idea a lot. I will work on that next.

regarding task list and desire to have it... I'm thinking of 2 users:

operator dev (that is new but maintaining or updating an operator
kudo user (admin)

The question is: what does this opaque operator named x (mysql) do? what does it consist of?
For user 1. I want to create a new plan and would like to know what tasks exist and if any are not used.
for user 2. I see this plan with phases and steps that link to this task... but what does that task link to.

It is a reasonable argument that all you will get is a list of tasks with a list of template files.... but hopefully the naming of these things will be valuable... or perhaps we add an output for a file for viewing.

kensipe · 2019-12-31T17:57:12Z

LOVE the tree view! thanks @alenkacz !

Signed-off-by: Ken Sipe <kensipe@gmail.com>

zen-dog

Love the tree view and cmd consistency! Left a few nits, but nothing critical.

pkg/kudoctl/cmd/package_list.go

pkg/kudoctl/cmd/package_list_params.go

pkg/kudoctl/cmd/package_list_plans.go

pkg/kudoctl/cmd/package_list_tasks.go

zen-dog · 2020-01-06T10:28:25Z

pkg/kudoctl/cmd/testdata/plans-list.golden

@@ -0,0 +1,11 @@
+plans


I like golden files but this seems to be too small to be one. wdyt about putting it into a multiline var in the test above? Same for the other two golden files below.

I actually like this one a lot. it is a great output example that could be referenced via documentation (when using asciidoc). It is also easy for a developer to "see" what the output is intended to be.

Hm, it's not exactly YAML files so that they absolutely must be referenced to stay consistent with the codebase. And a developer can "see" them even better if they're in the same test file, where they're used. But I don't feel too strongly about it so if you insist 🤷‍♂

👍 for referencing these files in our documentation -- is there already a docs PR for this feature?

Signed-off-by: Ken Sipe <kensipe@gmail.com>

zen-dog

🚢

concerns addressed and it isn't clear if she will return

nfnt

Thanks, this is a great addition, awesome to have these consistent set of subcommands to investigate operators.

My comments are mostly nits or small corrections, overall this looks great!

nfnt · 2020-01-07T13:33:05Z

pkg/kudoctl/cmd/testdata/plans-list.golden

@@ -0,0 +1,11 @@
+plans


👍 for referencing these files in our documentation -- is there already a docs PR for this feature?

pkg/kudoctl/cmd/package_list.go

pkg/kudoctl/cmd/package_list_params.go

nfnt · 2020-01-07T13:52:50Z

pkg/kudoctl/cmd/package_list_plans.go

+}
+
+func sortedPlanNames(pf *packages.Files) []string {
+	planNames := make([]string, 0, len(pf.Operator.Plans))


sort.Strings also calls sort.Sort which is using interface{}. So
sort.Sort(funk.Keys(pf.Operator.Plans)) would do the same.

nfnt · 2020-01-07T13:58:24Z

pkg/kudoctl/cmd/package_list_plans.go

+	}
+
+	f := cmd.Flags()
+	f.StringVar(&lc.RepoName, "repo", "", "Name of repository configuration to use. (default defined by context)")


Not yours, more a general question: Given that repo and version are variables shared by all these subcommands, is it possible in cobra to "inherit" them here so that they don't have to be defined again for every subcommand?

nfnt · 2020-01-07T14:03:29Z

pkg/kudoctl/cmd/package_list_plans_test.go

+
+	gp := filepath.Join("testdata", file+".golden")
+
+	if *updateGolden {


Probably not yours, but why would we update files in a test? Especially if it's the file we're testing against?

it is to easy the burden on "known" output changes... it's a pretty standard technique

Co-Authored-By: Jan Schlicht <jan@d2iq.com>

nfnt

LGTM!

Signed-off-by: Ken Sipe <kensipe@gmail.com>

Make lint happy again. This was result of a race between merging #1223 and #1245

Co-authored-by: Jan Schlicht <jan.schlicht+gh@gmail.com> Signed-off-by: Ken Sipe <kensipe@gmail.com> Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>

Make lint happy again. This was result of a race between merging #1223 and #1245 Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>

kensipe added 2 commits December 23, 2019 17:30

moving params list to list params to be consistent with verb noun and…

be392ad

… to add a number of other list types for operators Signed-off-by: Ken Sipe <kensipe@gmail.com>

list params moved, list plans and list tasks added

07f9015

Signed-off-by: Ken Sipe <kensipe@gmail.com>

kensipe requested review from alenkacz, gerred, nfnt and zen-dog as code owners December 24, 2019 05:33

improved logic around list plans table layout

0682488

Signed-off-by: Ken Sipe <kensipe@gmail.com>

alenkacz added the release-highlight label Dec 30, 2019

alenkacz previously requested changes Dec 30, 2019

View reviewed changes

pkg/kudoctl/cmd/package_list.go Outdated Show resolved Hide resolved

pkg/kudoctl/cmd/package_list_plans.go Outdated Show resolved Hide resolved

pkg/kudoctl/cmd/package_list_plans.go Outdated Show resolved Hide resolved

pkg/kudoctl/cmd/package_list_tasks.go Outdated Show resolved Hide resolved

updates based on pr feedback

099c368

Signed-off-by: Ken Sipe <kensipe@gmail.com>

added treeview for plans

93d0463

Signed-off-by: Ken Sipe <kensipe@gmail.com>

kensipe mentioned this pull request Dec 31, 2019

Report "Package list parameters" as a Breaking Change #1238

Closed

added tests

a6fe7d0

Signed-off-by: Ken Sipe <kensipe@gmail.com>

kensipe requested a review from alenkacz December 31, 2019 18:21

we have to sort or we get undeterministic results

a05056d

Signed-off-by: Ken Sipe <kensipe@gmail.com>

zen-dog requested changes Jan 6, 2020

View reviewed changes

updates based on PR feedback

3050da9

Signed-off-by: Ken Sipe <kensipe@gmail.com>

zen-dog approved these changes Jan 6, 2020

View reviewed changes

gerred added the needs review label Jan 6, 2020

ANeumann82 approved these changes Jan 7, 2020

View reviewed changes

nfnt requested changes Jan 7, 2020

View reviewed changes

Apply suggestions from code review

2834d52

Co-Authored-By: Jan Schlicht <jan@d2iq.com>

nfnt approved these changes Jan 7, 2020

View reviewed changes

kensipe added 3 commits January 7, 2020 10:25

Merge branch 'master' into ken/package-lists

6e829c2

update based on pr review

521e7ad

Signed-off-by: Ken Sipe <kensipe@gmail.com>

Merge branch 'master' into ken/package-lists

f8a5069

kensipe merged commit 52ecf33 into master Jan 7, 2020

kensipe deleted the ken/package-lists branch January 7, 2020 19:58

porridge mentioned this pull request Jan 8, 2020

Check the type assertion on funk.Keys return value. #1256

Merged

porridge added a commit that referenced this pull request Jan 8, 2020

Check the type assertion on funk.Keys return value. (#1256)

6dcbee3

Make lint happy again. This was result of a race between merging #1223 and #1245

ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020

Package List (Parameters | Tasks | Plans) (#1223)

a4daf3f

Co-authored-by: Jan Schlicht <jan.schlicht+gh@gmail.com> Signed-off-by: Ken Sipe <kensipe@gmail.com> Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>

ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020

Check the type assertion on funk.Keys return value. (#1256)

ee84900

Make lint happy again. This was result of a race between merging #1223 and #1245 Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>

porridge added release/highlight This PR is a highlight for the next release and removed release-highlight labels Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Package List (Parameters | Tasks | Plans) #1223

Package List (Parameters | Tasks | Plans) #1223

kensipe commented Dec 24, 2019 •

edited

Loading

kensipe commented Dec 24, 2019

alenkacz left a comment •

edited

Loading

kensipe commented Dec 31, 2019

kensipe commented Dec 31, 2019

zen-dog left a comment

zen-dog Jan 6, 2020

kensipe Jan 6, 2020

zen-dog Jan 6, 2020

nfnt Jan 7, 2020

zen-dog left a comment

nfnt left a comment

nfnt Jan 7, 2020

nfnt Jan 7, 2020

nfnt Jan 7, 2020

nfnt Jan 7, 2020

kensipe Jan 7, 2020

nfnt left a comment


		gp := filepath.Join("testdata", file+".golden")

		if *updateGolden {

Package List (Parameters | Tasks | Plans) #1223

Package List (Parameters | Tasks | Plans) #1223

Conversation

kensipe commented Dec 24, 2019 • edited Loading

kensipe commented Dec 24, 2019

alenkacz left a comment • edited Loading

Choose a reason for hiding this comment

kensipe commented Dec 31, 2019

kensipe commented Dec 31, 2019

zen-dog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zen-dog left a comment

Choose a reason for hiding this comment

nfnt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nfnt left a comment

Choose a reason for hiding this comment

kensipe commented Dec 24, 2019 •

edited

Loading

alenkacz left a comment •

edited

Loading