Optimize: move experimental cmd to alpha stage #1577
Conversation
cmd/sealer/cmd/alpha/alpha.go
Outdated
| func NewCmdAlpha() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "alpha", | ||
| Short: "Sealer experimental sub-commands", |
There was a problem hiding this comment.
Please add a long field to add more description of sealer alpha command. This is very useful for end users. Thanks. And do not make sealer uppercase. sealer is a reserved word, then it has no uppercase mode, just like etcd. sealer and etcd both have a lowercase first letter.
There was a problem hiding this comment.
alpha command has sub command which has their own long description. i am ok with adding long description on alpha command, but seems like there is very little to describe.
There was a problem hiding this comment.
😄， we should explain to help end-users to understand what is alpha command for. Please take the following paragraph into consideration.
alpha command of sealer is used to provide functionality incubation from immature to mature. Each function will experience a growing procedure. Alpha command policy calls on end users to experience alpha functionality as early as possible, and actively feedback the experience results to sealer community, and finally cooperate to promote function from incubation to graduation. Please file an issue at https://github.com/sealerio/sealer/issues when you have any feedback on alpha commands.
cmd/sealer/cmd/alpha/cert.go
Outdated
|
|
||
| var altNames []string | ||
|
|
||
| var longCertCmdDescription = `Add domain or ip in certs: you had better backup old certs first. this command will update cluster API server cert, you need to restart your API server manually after using sealer cert. then, you can using cmd "openssl x509 -noout -text -in apiserver.crt" to check the cert details. |
There was a problem hiding this comment.
Please make sure that first word of every sentence should be uppercase. It is minor mistake. But I think we should pay more attention to this minor cases.
cmd/sealer/cmd/alpha/cert.go
Outdated
| Example: exampleForCertCmd, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| if len(altNames) == 0 { | ||
| return fmt.Errorf("ip address or DNS domain needed for cert Subject Alternative Names") |
There was a problem hiding this comment.
s/ip/IP/g, IP is a reserved word. While I think quite lot of people use ip, which is lowercase. And lowercase is incorrect.
cmd/sealer/cmd/alpha/cert.go
Outdated
| }, | ||
| } | ||
|
|
||
| certCmd.Flags().StringSliceVar(&altNames, "alt-names", []string{}, "add domain or ip in certs, sealer.cool or 10.103.97.2") |
There was a problem hiding this comment.
Do we have a default domain name? If we have, we should add it explicitly in the flag description.
There was a problem hiding this comment.
yes, we have a default domain in the cert process builtin, i will add it explicitly in the flag description.
cmd/sealer/cmd/alpha/exec.go
Outdated
| ipList = append(ipList, cluster.GetIPSByRole(role)...) | ||
| } | ||
| if len(ipList) == 0 { | ||
| return fmt.Errorf("failed to get target ipList by roles label %s", roles) |
There was a problem hiding this comment.
should add the detailed failure reason:
return fmt.Errorf("failed to get target ipList: no IP gotten by role(%s)", roles)
|
|
Codecov Report
@@ Coverage Diff @@
## main #1577 +/- ##
=======================================
Coverage 11.19% 11.19%
=======================================
Files 88 88
Lines 8067 8067
=======================================
Hits 903 903
Misses 7052 7052
Partials 112 112 Continue to review full report at Codecov.
|
cmd/sealer/cmd/alpha/cert.go
Outdated
| }, | ||
| } | ||
|
|
||
| certCmd.Flags().StringSliceVar(&altNames, "alt-names", []string{}, "add DNS domain or IP in certs, if it already in the cert subject alternative names list, nothing will changed") |
There was a problem hiding this comment.
add DNS domain or IP in certs. If it is already in the cert subject alternative names list, nothing will be changed.
cmd/sealer/cmd/alpha/prune.go
Outdated
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| var exampleForPruneCmd = `The following command will prune sealer data directory such as, image db ,image layer, build tmp. |
There was a problem hiding this comment.
The following command will prune sealer data directory, such as: image db, image layer and build tmp.
|
LGTM unless the above suggestion updated. |
Describe what this PR does / why we need it
move below experimental cmd to alpha stage to incubate.
sealer prunesealer gensealer debugsealer execsealer certsealer mergesealer upgradeDoes this pull request fix one issue?
fix #1522
Describe how you did it
Describe how to verify it
Special notes for reviews