From 172f08b328724bdb132fb152dbd74e15dc71b2da Mon Sep 17 00:00:00 2001 From: luofucong Date: Fri, 19 Dec 2025 19:58:54 +0800 Subject: [PATCH] 1. remove not worked metasrv option: `--use-memory-store` 2. fix golangci-lint 3. upgrade go version 4. use bare metal in e2e test --- .github/workflows/ci.yaml | 24 +----- .golangci.yaml | 110 +++++----------------------- cmd/gtctl/cluster_create.go | 4 +- go.mod | 2 +- pkg/artifacts/manager.go | 8 +- pkg/artifacts/manager_test.go | 11 +-- pkg/cluster/baremetal/cluster.go | 13 +--- pkg/cluster/baremetal/create.go | 2 +- pkg/components/metasrv.go | 10 +-- pkg/config/baremetal.go | 2 +- pkg/helm/loader_test.go | 7 +- pkg/kube/client.go | 2 +- pkg/metadata/manager_test.go | 3 +- pkg/utils/file/file.go | 18 ++++- pkg/utils/file/file_test.go | 4 +- tests/e2e/greptimedbcluster_test.go | 56 +++----------- 16 files changed, 69 insertions(+), 207 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ef97c96c..997f6042 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -3,7 +3,7 @@ name: CI on: [ push, pull_request ] env: - GO_VERSION: "1.21" + GO_VERSION: "1.25" permissions: contents: read @@ -25,31 +25,11 @@ jobs: with: go-version: ${{ env.GO_VERSION }} - name: golangci-lint - uses: golangci/golangci-lint-action@v3 + uses: golangci/golangci-lint-action@v9 with: - version: v1.54 # '-v' flag is required to show the output of golangci-lint. args: -v - unit-test: - name: Unit test coverage - runs-on: ubuntu-latest - needs: [ lint ] - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 - with: - go-version: ${{ env.GO_VERSION }} - - name: Unit test - run: make coverage - - name: Upload coverage to Codecov - uses: codecov/codecov-action@v4 - with: - token: ${{ secrets.CODECOV_TOKEN }} - fail_ci_if_error: true - files: ./coverage.xml - name: codecov-gtctl - verbose: true e2e: name: End to End tests runs-on: ubuntu-latest diff --git a/.golangci.yaml b/.golangci.yaml index 2ecdd9fb..95018510 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,95 +1,23 @@ +version: "2" + # Options for analysis running. run: - # Timeout for analysis, e.g. 30s, 5m. - # Default: 1m - timeout: 10m - - # The default concurrency value is the number of available CPU. - concurrency: 4 - - # Which dirs to skip: issues from them won't be reported. - # Can use regexp here: `generated.*`, regexp is applied on full path, - # including the path prefix if one is set. - # Default value is empty list, - # but default dirs are skipped independently of this option's value (see skip-dirs-use-default). - # "/" will be replaced by current OS file path separator to properly work on Windows. - skip-dirs: - - bin - - docs - - examples - - hack - -# output configuration options. -output: - # Format: colored-line-number|line-number|json|colored-tab|tab|checkstyle|code-climate|junit-xml|github-actions|teamcity - # - # Multiple can be specified by separating them by comma, output can be provided - # for each of them by separating format name and path by colon symbol. - # Output path can be either `stdout`, `stderr` or path to the file to write to. - # Example: "checkstyle:report.xml,json:stdout,colored-line-number" - # - # Default: colored-line-number - format: colored-line-number - - # Print lines of code with issue. - # Default: true - print-issued-lines: true - - # Print linter name in the end of issue text. - # Default: true - print-linter-lines: true + # Timeout for total work, e.g. 30s, 5m, 5m30s. + # If the value is lower or equal to 0, the timeout is disabled. + # Default: 0 (disabled) + timeout: 5m linters: - # Disable all linters. - disable-all: true - - # Enable specific linter - # https://golangci-lint.run/usage/linters/#enabled-by-default - enable: - # Errcheck is a program for checking for unchecked errors in Go code. These unchecked errors can be critical bugs in some cases. - - errcheck - - # Linter for Go source code that specializes in simplifying code. - - gosimple - - # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string. - - govet - - # Detects when assignments to existing variables are not used. - - ineffassign - - # It's a set of rules from staticcheck. It's not the same thing as the staticcheck binary. - # The author of staticcheck doesn't support or approve the use of staticcheck as a library inside golangci-lint. - - staticcheck - - # Check import statements are formatted according to the 'goimport' command. Reformat imports in autofix mode. - - goimports - - # Checks whether HTTP response body is closed successfully. - - bodyclose - - # Provides diagnostics that check for bugs, performance and style issues. - # Extensible without recompilation through dynamic rules. - # Dynamic rules are written declaratively with AST patterns, filters, report message and optional suggestion. - - gocritic - - # Gofmt checks whether code was gofmt-ed. By default, this tool runs with -s option to check for code simplification. - - gofmt - - # Finds repeated strings that could be replaced by a constant. - - goconst - - # Finds commonly misspelled English words in comments. - - misspell - - # Finds naked returns in functions greater than a specified function length. - - nakedret - -linters-settings: - goimports: - # A comma-separated list of prefixes, which, if set, checks import paths - # with the given prefixes are grouped after 3rd-party packages. - # Default: "" - local-prefixes: github.com/GreptimeTeam/gtctl - linters-settings: - min-occurrences: 3 + # Default set of linters. + # The value can be: + # - `standard`: https://golangci-lint.run/docs/linters/#enabled-by-default + # - `all`: enables all linters by default. + # - `none`: disables all linters by default. + # - `fast`: enables only linters considered as "fast" (`golangci-lint help linters --json | jq '[ .[] | select(.fast==true) ] | map(.name)'`). + # Default: standard + default: standard + +formatters: + # Enable specific formatter. + # Default: [] (uses standard Go formatting) + enable: [] diff --git a/cmd/gtctl/cluster_create.go b/cmd/gtctl/cluster_create.go index a8cfa69d..7b18567b 100644 --- a/cmd/gtctl/cluster_create.go +++ b/cmd/gtctl/cluster_create.go @@ -62,7 +62,6 @@ type clusterCreateCliOptions struct { Config string GreptimeBinVersion string EnableCache bool - UseMemoryMeta bool // Common options. Timeout int @@ -110,7 +109,6 @@ func NewCreateClusterCommand(l logger.Logger) *cobra.Command { cmd.Flags().StringVar(&options.GreptimeDBClusterValuesFile, "greptimedb-cluster-values-file", "", "The values file for greptimedb cluster.") cmd.Flags().StringVar(&options.EtcdClusterValuesFile, "etcd-cluster-values-file", "", "The values file for etcd cluster.") cmd.Flags().StringVar(&options.GreptimeDBOperatorValuesFile, "greptimedb-operator-values-file", "", "The values file for greptimedb operator.") - cmd.Flags().BoolVar(&options.UseMemoryMeta, "use-memory-meta", false, "Bootstrap the whole cluster without installing etcd for testing purposes through using the memory storage of metasrv in bare-metal mode.") return cmd } @@ -184,7 +182,7 @@ func NewCluster(args []string, options *clusterCreateCliOptions, l logger.Logger l.V(0).Infof("Creating GreptimeDB cluster '%s' on bare-metal", logger.Bold(clusterName)) var opts []baremetal.Option - opts = append(opts, baremetal.WithEnableCache(options.EnableCache), baremetal.WithMetastore(options.UseMemoryMeta)) + opts = append(opts, baremetal.WithEnableCache(options.EnableCache)) if len(options.GreptimeBinVersion) > 0 { opts = append(opts, baremetal.WithGreptimeVersion(options.GreptimeBinVersion)) } diff --git a/go.mod b/go.mod index c3d6dee6..dc1d835e 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/GreptimeTeam/gtctl -go 1.18 +go 1.25 require ( github.com/GreptimeTeam/greptimedb-operator v0.1.0-alpha.9 diff --git a/pkg/artifacts/manager.go b/pkg/artifacts/manager.go index 642791c0..e9ea943e 100644 --- a/pkg/artifacts/manager.go +++ b/pkg/artifacts/manager.go @@ -246,7 +246,7 @@ func (m *manager) downloadFromHTTP(ctx context.Context, httpURL string, dest str if resp.StatusCode != http.StatusOK { return fmt.Errorf("download failed, status code: %d", resp.StatusCode) } - defer resp.Body.Close() + defer fileutils.MustClose(resp.Body) data, err := io.ReadAll(resp.Body) if err != nil { @@ -306,7 +306,7 @@ func (m *manager) chartIndexFile(ctx context.Context, indexURL string) (*repo.In if err != nil { return nil, err } - defer rsp.Body.Close() + defer fileutils.MustClose(rsp.Body) data, err := io.ReadAll(rsp.Body) if err != nil { @@ -431,7 +431,7 @@ func (m *manager) installBinaries(downloadFile, installDir string) error { if err != nil { return err } - defer os.RemoveAll(tempDir) + defer fileutils.RemoveAll(tempDir) if err := fileutils.Uncompress(downloadFile, tempDir); err != nil { return err @@ -525,7 +525,7 @@ func (m *manager) getVersionInfoFromS3(typ ArtifactType, name string, nightly bo if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("get latest info from '%s' failed, status code: %d", latestVersionInfoURL, resp.StatusCode) } - defer resp.Body.Close() + defer fileutils.MustClose(resp.Body) data, err := io.ReadAll(resp.Body) if err != nil { diff --git a/pkg/artifacts/manager_test.go b/pkg/artifacts/manager_test.go index 3184eeb3..02943f9a 100644 --- a/pkg/artifacts/manager_test.go +++ b/pkg/artifacts/manager_test.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/kind/pkg/log" "github.com/GreptimeTeam/gtctl/pkg/logger" + "github.com/GreptimeTeam/gtctl/pkg/utils/file" ) func TestDownloadCharts(t *testing.T) { @@ -33,7 +34,7 @@ func TestDownloadCharts(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(tempDir) + defer file.RemoveAll(tempDir) m, err := NewManager(logger.New(os.Stdout, log.Level(4), logger.WithColored())) if err != nil { @@ -79,7 +80,7 @@ func TestDownloadChartsFromCNRegion(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(tempDir) + defer file.RemoveAll(tempDir) m, err := NewManager(logger.New(os.Stdout, log.Level(4), logger.WithColored())) if err != nil { @@ -125,7 +126,7 @@ func TestDownloadBinariesFromCNRegion(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(tempDir) + defer file.RemoveAll(tempDir) m, err := NewManager(logger.New(os.Stdout, log.Level(4), logger.WithColored())) if err != nil { @@ -171,7 +172,7 @@ func TestDownloadBinaries(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(tempDir) + defer file.RemoveAll(tempDir) m, err := NewManager(logger.New(os.Stdout, log.Level(4), logger.WithColored())) if err != nil { @@ -218,7 +219,7 @@ func TestArtifactsCache(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(tempDir) + defer file.RemoveAll(tempDir) m, err := NewManager(logger.New(os.Stdout, log.Level(4), logger.WithColored())) if err != nil { diff --git a/pkg/cluster/baremetal/cluster.go b/pkg/cluster/baremetal/cluster.go index b1098be2..e6f473a5 100644 --- a/pkg/cluster/baremetal/cluster.go +++ b/pkg/cluster/baremetal/cluster.go @@ -34,7 +34,6 @@ type Cluster struct { config *config.BareMetalClusterConfig createNoDirs bool enableCache bool - useMemoryMeta bool am artifacts.Manager mm metadata.Manager @@ -55,9 +54,9 @@ type ClusterComponents struct { } func NewClusterComponents(config *config.BareMetalClusterComponentsConfig, workingDirs components.WorkingDirs, - wg *sync.WaitGroup, logger logger.Logger, useMemoryMeta bool) *ClusterComponents { + wg *sync.WaitGroup, logger logger.Logger) *ClusterComponents { return &ClusterComponents{ - MetaSrv: components.NewMetaSrv(config.MetaSrv, workingDirs, wg, logger, useMemoryMeta), + MetaSrv: components.NewMetaSrv(config.MetaSrv, workingDirs, wg, logger), Datanode: components.NewDataNode(config.Datanode, config.MetaSrv.ServerAddr, workingDirs, wg, logger), Frontend: components.NewFrontend(config.Frontend, config.MetaSrv.ServerAddr, workingDirs, wg, logger), Etcd: components.NewEtcd(workingDirs, wg, logger), @@ -85,12 +84,6 @@ func WithEnableCache(enableCache bool) Option { } } -func WithMetastore(useMemoryMeta bool) Option { - return func(c *Cluster) { - c.useMemoryMeta = useMemoryMeta - } -} - func WithCreateNoDirs() Option { return func(c *Cluster) { c.createNoDirs = true @@ -143,7 +136,7 @@ func NewCluster(l logger.Logger, clusterName string, opts ...Option) (cluster.Op DataDir: csd.DataDir, LogsDir: csd.LogsDir, PidsDir: csd.PidsDir, - }, &c.wg, c.logger, c.useMemoryMeta) + }, &c.wg, c.logger) return c, nil } diff --git a/pkg/cluster/baremetal/create.go b/pkg/cluster/baremetal/create.go index f1f39c3f..65169859 100644 --- a/pkg/cluster/baremetal/create.go +++ b/pkg/cluster/baremetal/create.go @@ -52,7 +52,7 @@ func (c *Cluster) Create(ctx context.Context, options *opt.CreateOptions) error return nil } - if !c.useMemoryMeta { + if c.config.Etcd != nil { if err := withSpinner("Etcd Cluster", c.createEtcdCluster); err != nil { return err } diff --git a/pkg/components/metasrv.go b/pkg/components/metasrv.go index cdefff1a..81dc0da6 100644 --- a/pkg/components/metasrv.go +++ b/pkg/components/metasrv.go @@ -22,7 +22,6 @@ import ( "net" "net/http" "path" - "strconv" "sync" "time" @@ -37,19 +36,17 @@ type metaSrv struct { workingDirs WorkingDirs wg *sync.WaitGroup logger logger.Logger - useMemoryMeta bool allocatedDirs } func NewMetaSrv(config *config.MetaSrv, workingDirs WorkingDirs, - wg *sync.WaitGroup, logger logger.Logger, useMemoryMeta bool) ClusterComponent { + wg *sync.WaitGroup, logger logger.Logger) ClusterComponent { return &metaSrv{ config: config, workingDirs: workingDirs, wg: wg, logger: logger, - useMemoryMeta: useMemoryMeta, } } @@ -128,11 +125,6 @@ func (m *metaSrv) BuildArgs(params ...interface{}) []string { args = GenerateAddrArg("--http-addr", m.config.HTTPAddr, nodeID, args) args = GenerateAddrArg("--bind-addr", bindAddr, nodeID, args) - if m.useMemoryMeta { - useMemoryMeta := strconv.FormatBool(m.useMemoryMeta) - args = GenerateAddrArg("--use-memory-store", useMemoryMeta, nodeID, args) - } - if len(m.config.Config) > 0 { args = append(args, fmt.Sprintf("-c=%s", m.config.Config)) } diff --git a/pkg/config/baremetal.go b/pkg/config/baremetal.go index 8795747c..91f3c7e2 100644 --- a/pkg/config/baremetal.go +++ b/pkg/config/baremetal.go @@ -38,7 +38,7 @@ type BareMetalClusterMetadata struct { // Each field of BareMetalClusterConfig can also have its own exported method `Validate`. type BareMetalClusterConfig struct { Cluster *BareMetalClusterComponentsConfig `yaml:"cluster" validate:"required"` - Etcd *Etcd `yaml:"etcd" validate:"required"` + Etcd *Etcd `yaml:"etcd"` } type BareMetalClusterComponentsConfig struct { diff --git a/pkg/helm/loader_test.go b/pkg/helm/loader_test.go index 36eb332c..4fc04ae8 100644 --- a/pkg/helm/loader_test.go +++ b/pkg/helm/loader_test.go @@ -26,6 +26,7 @@ import ( "github.com/GreptimeTeam/gtctl/pkg/artifacts" opt "github.com/GreptimeTeam/gtctl/pkg/cluster" "github.com/GreptimeTeam/gtctl/pkg/logger" + "github.com/GreptimeTeam/gtctl/pkg/utils/file" ) const ( @@ -37,7 +38,7 @@ func TestLoadAndRenderChart(t *testing.T) { if err != nil { t.Errorf("failed to create render: %v", err) } - defer cleanMetadataDir() + defer file.RemoveAll(testMetadataDir) opts := &LoadOptions{ ReleaseName: "gtctl-ut", @@ -73,7 +74,3 @@ func TestLoadAndRenderChart(t *testing.T) { t.Errorf("expected %s, got %s", string(wantedManifests), string(manifests)) } } - -func cleanMetadataDir() { - os.RemoveAll(testMetadataDir) -} diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 30dedd6b..6aeeec1a 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -109,7 +109,7 @@ func NewClient(kubeconfig string) (*Client, error) { kubeVersion, err := kubeClient.ServerVersion() if err != nil { - return nil, fmt.Errorf("failed to get kubernetes server version: %v\n", err) + return nil, fmt.Errorf("failed to get kubernetes server version: %v", err) } helm.KubeVersion = kubeVersion.String() diff --git a/pkg/metadata/manager_test.go b/pkg/metadata/manager_test.go index 234a4ab4..66616c66 100644 --- a/pkg/metadata/manager_test.go +++ b/pkg/metadata/manager_test.go @@ -26,6 +26,7 @@ import ( "github.com/GreptimeTeam/gtctl/pkg/artifacts" "github.com/GreptimeTeam/gtctl/pkg/config" + "github.com/GreptimeTeam/gtctl/pkg/utils/file" ) func TestMetadataManager(t *testing.T) { @@ -33,7 +34,7 @@ func TestMetadataManager(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(tempDir) + defer file.RemoveAll(tempDir) m, err := New(tempDir) if err != nil { diff --git a/pkg/utils/file/file.go b/pkg/utils/file/file.go index 4a934d5c..f844132f 100644 --- a/pkg/utils/file/file.go +++ b/pkg/utils/file/file.go @@ -73,13 +73,13 @@ func CopyFile(src, dst string) error { if err != nil { return err } - defer r.Close() + defer MustClose(r) w, err := os.Create(dst) if err != nil { return err } - defer w.Close() + defer MustClose(w) _, err = io.Copy(w, r) if err != nil { @@ -115,7 +115,7 @@ func unzip(file, dst string) error { if err != nil { return err } - defer archive.Close() + defer MustClose(archive) for _, f := range archive.File { filePath := filepath.Join(dst, f.Name) @@ -210,3 +210,15 @@ func untar(file, dst string) error { return nil } + +func MustClose(c io.Closer) { + if err := c.Close(); err != nil { + _ = fmt.Errorf("failed to close: %v", err) + } +} + +func RemoveAll(path string) { + if err := os.RemoveAll(path); err != nil { + _ = fmt.Errorf("failed to remove path %s: %v", path, err) + } +} diff --git a/pkg/utils/file/file_test.go b/pkg/utils/file/file_test.go index e33ee936..c576e453 100644 --- a/pkg/utils/file/file_test.go +++ b/pkg/utils/file/file_test.go @@ -39,9 +39,7 @@ func TestUncompress(t *testing.T) { } // Clean up output dir. - defer func() { - os.RemoveAll(outputDir) - }() + defer RemoveAll(outputDir) for _, test := range tests { if err := Uncompress(test.path, test.dst); err != nil { diff --git a/tests/e2e/greptimedbcluster_test.go b/tests/e2e/greptimedbcluster_test.go index 30b35f2d..56d17823 100644 --- a/tests/e2e/greptimedbcluster_test.go +++ b/tests/e2e/greptimedbcluster_test.go @@ -28,7 +28,6 @@ import ( . "github.com/onsi/gomega" "github.com/go-sql-driver/mysql" - "k8s.io/klog/v2" ) const ( @@ -66,19 +65,17 @@ type TestData struct { var _ = Describe("Basic test of greptimedb cluster", func() { It("Bootstrap cluster", func() { var err error - err = createCluster() - Expect(err).NotTo(HaveOccurred(), "failed to create cluster") + go func() { + if err := createCluster(); err != nil { + panic(err) + } + }() + // Wait for GreptimeDB cluster to be ready. + time.Sleep(3 * time.Minute) err = getCluster() Expect(err).NotTo(HaveOccurred(), "failed to get cluster") - err = listCluster() - Expect(err).NotTo(HaveOccurred(), "failed to list cluster") - - go func() { - forwardRequest() - }() - By("Connecting GreptimeDB") var db *sql.DB var conn *sql.Conn @@ -135,16 +132,11 @@ var _ = Describe("Basic test of greptimedb cluster", func() { data = append(data, d) } Expect(len(data) == testRowIDNum).Should(BeTrue(), "get the wrong data from db") - - err = deleteCluster() - Expect(err).NotTo(HaveOccurred(), "failed to delete cluster") }) }) func createCluster() error { - cmd := exec.Command("../../bin/gtctl", "cluster", "create", "mydb", "--timeout", "300") - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + cmd := exec.Command("../../bin/gtctl", "cluster", "create", "mydb", "--bare-metal", "--timeout", "300") if err := cmd.Run(); err != nil { return err } @@ -152,17 +144,7 @@ func createCluster() error { } func getCluster() error { - cmd := exec.Command("../../bin/gtctl", "cluster", "get", "mydb") - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { - return err - } - return nil -} - -func listCluster() error { - cmd := exec.Command("../../bin/gtctl", "cluster", "list") + cmd := exec.Command("../../bin/gtctl", "cluster", "get", "mydb", "--bare-metal") cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { @@ -170,23 +152,3 @@ func listCluster() error { } return nil } - -func deleteCluster() error { - cmd := exec.Command("../../bin/gtctl", "cluster", "delete", "mydb", "--tear-down-etcd") - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { - return err - } - return nil -} - -func forwardRequest() { - for { - cmd := exec.Command("kubectl", "port-forward", "svc/mydb-frontend", "4002:4002") - if err := cmd.Run(); err != nil { - klog.Errorf("Failed to port forward: %v", err) - return - } - } -}