Add issue-template by donggyupark · Pull Request #7 · tmax-cloud/hypercloud-sds

donggyupark · 2020-08-21T07:30:45Z

issue-template command collects logs and information for problem analysis

shellwedance

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/errors in util, and every error-logging handlers call this util function

shellwedance · 2020-09-16T05:17:47Z

hcsctl/cmd/issue-template.go

+)
+
+var issueTemplateCmd = &cobra.Command{
+	Use:   "issue-template",


It is more coherent to make subcommand as a verb.

I recommend create-issue-info or create-issue-template instead of issue-template.

I agree but what about gather? This command is more about gathering rook-ceph related log files and system status.

shellwedance · 2020-09-16T06:25:31Z

hcsctl/pkg/rook/issue-template.go

+
+	info, err := os.Stat(source)
+	if err != nil {
+		return nil


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.

shellwedance · 2020-09-16T06:29:34Z

hcsctl/pkg/rook/issue-template.go

+			}
+
+			if err2 := tarball.WriteHeader(header); err2 != nil {
+				return err


It should return err2, not err

shellwedance · 2020-09-16T06:33:35Z

hcsctl/pkg/rook/issue-template.go

+	if err != nil {
+		glog.Error(err.Error())
+	}
+


Delete the compressed directory (rookInfoDirName) after compressing

SangHyeok-Kim95

lgtm

TODO:

Consider the case where a user created a pool differently from the hypersds's default pool setting.

SangHyeok-Kim95 · 2020-09-17T02:43:35Z

hcsctl/pkg/rook/issue-template.go

+	cephMountInfoDirName string = "cephMount"
+	kubeCommand          string = "kubeCommand"
+	cephCommand          string = "cephCommand"
+	commonCommand        string = "commonCommand"


I recommend shellCommand instead of commonCommand.

SangHyeok-Kim95 · 2020-09-17T02:48:46Z

hcsctl/pkg/rook/issue-template.go

+		return err
+	}
+
+	glog.Info("Get cluster info")


I recommend more specific logs.
Get cluster Info -> Get Ceph Cluster infor

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.

SangHyeok-Kim95 · 2020-09-17T02:58:43Z

hcsctl/pkg/rook/issue-template.go

+	}
+
+	cmdList := [][]string{}
+	cmdList = append(cmdList, []string{"ceph_status", "ceph", "status"},


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)

hookak · 2020-09-17T05:41:03Z

lgtm. It would be nice to briefly explain what logs are collected in commit message.

hbinkim

.github/workflows/hcsctl.yml e2e section should be updated to execute this new command.

hbinkim · 2020-09-21T11:25:11Z

hcsctl/cmd/issue-template.go

+)
+
+var issueTemplateCmd = &cobra.Command{
+	Use:   "issue-template",


I agree but what about gather? This command is more about gathering rook-ceph related log files and system status.

hbinkim · 2020-09-21T11:47:16Z

hcsctl/cmd/issue-template.go

+
+var issueTemplateCmd = &cobra.Command{
+	Use:   "issue-template",
+	Short: "issue-template을 생성합니다.",


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 파일을 생성합니다. 이슈 등록 시 이를 첨부해주세요.

hbinkim · 2020-09-21T12:10:00Z

hcsctl/pkg/rook/issue-template.go

+		return err
+	}
+
+	glog.Info("Get cluster info")


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 requested review from SangHyeok-Kim95, hookak and shellwedance August 21, 2020 07:30

donggyupark assigned hbinkim Aug 21, 2020

donggyupark force-pushed the issue-template branch from 0bd455d to 385c745 Compare August 21, 2020 08:14

Add issue-template

40f0a58

issue-template command collects logs and information for problem analysis

hbinkim force-pushed the issue-template branch from 385c745 to 40f0a58 Compare September 4, 2020 06:30

fix importing package name

8fa341f

shellwedance reviewed Sep 16, 2020

View reviewed changes

SangHyeok-Kim95 reviewed Sep 17, 2020

View reviewed changes

hbinkim reviewed Sep 21, 2020

View reviewed changes

donggyupark force-pushed the master branch 2 times, most recently from d5006df to 792aa2c Compare October 5, 2022 02:31

Conversation

donggyupark commented Aug 21, 2020

Uh oh!

shellwedance left a comment • 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!

SangHyeok-Kim95 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!

hookak commented Sep 17, 2020

Uh oh!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shellwedance left a comment •

edited

Loading