Add issue-template #7
Conversation
donggyupark
force-pushed
the
issue-template
branch
from
0bd455d to
385c745
Compare
issue-template command collects logs and information for problem analysis
hbinkim
force-pushed
the
issue-template
branch
from
385c745 to
40f0a58
Compare
There was a problem hiding this comment.
Except comments below, All looks good to me.
TODO:
- Extract snapshots info in issue-template
- From HyperSDS 1.4, we support snapshot on both CephFS and RBD
- Unifying error message format
- ex. Create error function using
github.com/pkg/errorsin util, and every error-logging handlers call this util function
- ex. Create error function using
| ) | ||
|
|
||
| var issueTemplateCmd = &cobra.Command{ | ||
| Use: "issue-template", |
There was a problem hiding this comment.
It is more coherent to make subcommand as a verb.
I recommend create-issue-info or create-issue-template instead of issue-template.
There was a problem hiding this comment.
I agree but what about gather? This command is more about gathering rook-ceph related log files and system status.
|
|
||
| info, err := os.Stat(source) | ||
| if err != nil { | ||
| return nil |
There was a problem hiding this comment.
why does it return nil? zip function is called after creating source directory (rookInfoDirName), so if any error occurs here, it should return that error to caller.
| } | ||
|
|
||
| if err2 := tarball.WriteHeader(header); err2 != nil { | ||
| return err |
There was a problem hiding this comment.
It should return err2, not err
| if err != nil { | ||
| glog.Error(err.Error()) | ||
| } | ||
|
|
There was a problem hiding this comment.
Delete the compressed directory (rookInfoDirName) after compressing
| cephMountInfoDirName string = "cephMount" | ||
| kubeCommand string = "kubeCommand" | ||
| cephCommand string = "cephCommand" | ||
| commonCommand string = "commonCommand" |
There was a problem hiding this comment.
I recommend shellCommand instead of commonCommand.
| return err | ||
| } | ||
|
|
||
| glog.Info("Get cluster info") |
There was a problem hiding this comment.
I recommend more specific logs.
Get cluster Info -> Get Ceph Cluster infor
There was a problem hiding this comment.
I guess 'kubeadm_info' command is also executed in getClusterInfo, so "cluster" is not limited to ceph cluster. Maybe it's more clear to separate getCephClusterInfo and getKubeClusterInfo.
| } | ||
|
|
||
| cmdList := [][]string{} | ||
| cmdList = append(cmdList, []string{"ceph_status", "ceph", "status"}, |
There was a problem hiding this comment.
It would be nice to consider the case where the number of pools and the names of the pools are created differently from the hypersds's default pool settings. ( not just replicapool or myfs)
|
lgtm. It would be nice to briefly explain what logs are collected in commit message. |
| ) | ||
|
|
||
| var issueTemplateCmd = &cobra.Command{ | ||
| Use: "issue-template", |
There was a problem hiding this comment.
I agree but what about gather? This command is more about gathering rook-ceph related log files and system status.
|
|
||
| var issueTemplateCmd = &cobra.Command{ | ||
| Use: "issue-template", | ||
| Short: "issue-template을 생성합니다.", |
There was a problem hiding this comment.
I think it's better to include an explanation that this command only gather rook-ceph related information (for now). Maybe we can consider to add flag or rename the command to specify what it is for when we need to gather other information in the future.
rook 관련 로그파일과 클러스터 상태 정보를 수집하여 tar 파일을 생성합니다. 이슈 등록 시 이를 첨부해주세요.
| return err | ||
| } | ||
|
|
||
| glog.Info("Get cluster info") |
There was a problem hiding this comment.
I guess 'kubeadm_info' command is also executed in getClusterInfo, so "cluster" is not limited to ceph cluster. Maybe it's more clear to separate getCephClusterInfo and getKubeClusterInfo.
donggyupark
force-pushed
the
master
branch
2 times, most recently
from
d5006df to
792aa2c
Compare
issue-template command collects logs and information for problem analysis