Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix ui doesn't work when running backend without 'feature-version' option #1178

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ jobs:
matrix:
# test latest features and compatibility of lower version
include:
- feature_version: 6.0.0
- pd_version: 6.0.0
tidb_version: nightly
- feature_version: 5.0.0
- pd_version: 5.0.0
tidb_version: v5.0.0
steps:
- name: Checkout code
Expand Down Expand Up @@ -145,13 +145,13 @@ jobs:
make run &
env:
UI: 1
FEATURE_VERSION: ${{ matrix.feature_version }}
PD_VERSION: ${{ matrix.pd_version }}
- name: Run E2E Features Test
run: make test_e2e
env:
SERVER_URL: http://127.0.0.1:12333/dashboard/
CI: true
FEATURE_VERSION: ${{ matrix.feature_version }}
PD_VERSION: ${{ matrix.pd_version }}
TIDB_VERSION: ${{ matrix.tidb_version }}
- name: Archive Test Results
if: always()
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/upload-e2e-snapshots.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ jobs:
matrix:
# test latest features and compatibility of lower version
include:
- feature_version: 6.0.0
- pd_version: 6.0.0
tidb_version: nightly
- feature_version: 5.0.0
- pd_version: 5.0.0
tidb_version: v5.0.0
steps:
- name: Checkout code
Expand Down Expand Up @@ -88,13 +88,13 @@ jobs:
make run &
env:
UI: 1
FEATURE_VERSION: ${{ matrix.feature_version }}
PD_VERSION: ${{ matrix.pd_version }}
- name: Run E2E Features Test
run: make e2e_test
env:
SERVER_URL: http://127.0.0.1:12333/dashboard/
CI: true
FEATURE_VERSION: ${{ matrix.feature_version }}
PD_VERSION: ${{ matrix.pd_version }}
TIDB_VERSION: ${{ matrix.tidb_version }}
CYPRESS_ALLOW_SCREENSHOT: true
- name: Archive Test Results
Expand All @@ -106,5 +106,5 @@ jobs:
- name: Upload snapshots artifact
uses: actions/upload-artifact@v2
with:
name: e2e-snapshots-${{ matrix.feature_version }}
name: e2e-snapshots-${{ matrix.pd_version }}
path: ${{ github.workspace }}/ui/cypress/snapshots/**/*
12 changes: 6 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@ BUILD_TAGS ?=

LDFLAGS ?=

FEATURE_VERSION ?= 6.0.0
PD_VERSION ?= 6.0.0

TIDB_VERSION ?= latest

ifeq ($(UI),1)
BUILD_TAGS += ui_server
endif

LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.InternalVersion=$(shell grep -v '^\#' ./release-version)"
LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.Standalone=Yes"
LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.PDVersion=N/A"
LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.PDVersion=$(PD_VERSION)"
LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.BuildTime=$(shell date -u '+%Y-%m-%d %I:%M:%S')"
LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.BuildGitHash=$(shell git rev-parse HEAD)"

TIDB_VERSION ?= latest

default: server

.PHONY: clean
Expand Down Expand Up @@ -73,11 +73,11 @@ endif

.PHONY: run
run:
bin/tidb-dashboard --debug --experimental --feature-version "$(FEATURE_VERSION)" --host 0.0.0.0
bin/tidb-dashboard --debug --experimental --host 0.0.0.0

test_e2e_compat_features:
cd ui &&\
yarn run:e2e-test:compat-features --env FEATURE_VERSION=$(FEATURE_VERSION) TIDB_VERSION=$(TIDB_VERSION)
yarn run:e2e-test:compat-features --env PD_VERSION=$(PD_VERSION) TIDB_VERSION=$(TIDB_VERSION)

test_e2e_common_features:
cd ui &&\
Expand Down
6 changes: 5 additions & 1 deletion cmd/tidb-dashboard/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"sync"
"syscall"

"github.com/Masterminds/semver"
"github.com/pingcap/log"
flag "github.com/spf13/pflag"
"go.etcd.io/etcd/pkg/transport"
Expand Down Expand Up @@ -53,6 +54,10 @@ type DashboardCLIConfig struct {

// NewCLIConfig generates the configuration of the dashboard in standalone mode.
func NewCLIConfig() *DashboardCLIConfig {
if _, err := semver.NewVersion(version.PDVersion); err != nil {
log.Fatal("Invalid sematic PD Version", zap.Error(err), zap.String("pd_version", version.PDVersion))
}
Comment on lines +57 to +59
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the PDVersion when starting, expose the error as early as possible.


cfg := &DashboardCLIConfig{}
cfg.CoreConfig = config.Default()

Expand All @@ -65,7 +70,6 @@ func NewCLIConfig() *DashboardCLIConfig {
flag.StringVar(&cfg.CoreConfig.PDEndPoint, "pd", cfg.CoreConfig.PDEndPoint, "PD endpoint address that Dashboard Server connects to")
flag.BoolVar(&cfg.CoreConfig.EnableTelemetry, "telemetry", cfg.CoreConfig.EnableTelemetry, "allow telemetry")
flag.BoolVar(&cfg.CoreConfig.EnableExperimental, "experimental", cfg.CoreConfig.EnableExperimental, "allow experimental features")
flag.StringVar(&cfg.CoreConfig.FeatureVersion, "feature-version", cfg.CoreConfig.FeatureVersion, "target TiDB version for standalone mode")

showVersion := flag.BoolP("version", "v", false, "print version information and exit")

Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@

s.app = fx.New(
fx.Logger(utils.NewFxPrinter()),
fx.Supply(featureflag.NewRegistry(s.config.FeatureVersion)),
fx.Supply(featureflag.NewRegistry(version.PDVersion)),

Check warning on line 149 in pkg/apiserver/apiserver.go

View check run for this annotation

Codecov / codecov/patch

pkg/apiserver/apiserver.go#L149

Added line #L149 was not covered by tests
Modules,
fx.Provide(
s.provideLocals,
Expand Down
10 changes: 5 additions & 5 deletions pkg/apiserver/conprof/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ type ServiceParams struct {
}

type Service struct {
FeatureFlagConprof *featureflag.FeatureFlag
params ServiceParams
featureConprof *featureflag.FeatureFlag

params ServiceParams
lifecycleCtx context.Context
}

func newService(lc fx.Lifecycle, p ServiceParams) *Service {
s := &Service{
FeatureFlagConprof: p.FeatureFlags.Register("conprof", ">= 5.3.0"),
params: p,
params: p,
featureConprof: p.FeatureFlags.Register("conprof", ">= 5.3.0"),
}

lc.Append(fx.Hook{
Expand All @@ -54,7 +54,7 @@ func newService(lc fx.Lifecycle, p ServiceParams) *Service {
func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) {
endpoint := r.Group("/continuous_profiling")

endpoint.Use(s.FeatureFlagConprof.VersionGuard())
endpoint.Use(s.featureConprof.VersionGuard())
{
endpoint.GET("/config", auth.MWAuthRequired(), s.params.NgmProxy.Route("/config"))
endpoint.POST("/config", auth.MWAuthRequired(), auth.MWRequireWritePriv(), s.params.NgmProxy.Route("/config"))
Expand Down
44 changes: 20 additions & 24 deletions pkg/apiserver/info/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
"sort"
"strings"

"github.com/Masterminds/semver"
"github.com/gin-gonic/gin"
"github.com/thoas/go-funk"
"go.etcd.io/etcd/clientv3"
Expand All @@ -35,12 +34,17 @@
}

type Service struct {
params ServiceParams
params ServiceParams
featureNgm *featureflag.FeatureFlag

lifecycleCtx context.Context
}

func NewService(lc fx.Lifecycle, p ServiceParams) *Service {
s := &Service{params: p}
s := &Service{
params: p,
featureNgm: p.FeatureFlags.Register("ngm", ">= 5.4.0"),
}

lc.Append(fx.Hook{
OnStart: func(ctx context.Context) error {
Expand Down Expand Up @@ -78,37 +82,29 @@
// @Security JwtAuth
// @Failure 401 {object} rest.ErrorResponse
func (s *Service) infoHandler(c *gin.Context) {
// Checking ngm deployments
// drop "-alpha-xxx" suffix
versionWithoutSuffix := strings.Split(s.params.Config.FeatureVersion, "-")[0]
v, err := semver.NewVersion(versionWithoutSuffix)
if err != nil {
rest.Error(c, err)
return
}
constraint, err := semver.NewConstraint(">= v5.4.0")
if err != nil {
rest.Error(c, err)
return
resp := InfoResponse{
Version: version.GetInfo(),
EnableTelemetry: s.params.Config.EnableTelemetry,
EnableExperimental: s.params.Config.EnableExperimental,
SupportedFeatures: s.params.FeatureFlags.SupportedFeatures(),
NgmState: s.checkNgmState(),

Check warning on line 90 in pkg/apiserver/info/info.go

View check run for this annotation

Codecov / codecov/patch

pkg/apiserver/info/info.go#L85-L90

Added lines #L85 - L90 were not covered by tests
}
c.JSON(http.StatusOK, resp)

Check warning on line 92 in pkg/apiserver/info/info.go

View check run for this annotation

Codecov / codecov/patch

pkg/apiserver/info/info.go#L92

Added line #L92 was not covered by tests
}

// Checking ngm deployments.
func (s *Service) checkNgmState() utils.NgmState {

Check warning on line 96 in pkg/apiserver/info/info.go

View check run for this annotation

Codecov / codecov/patch

pkg/apiserver/info/info.go#L96

Added line #L96 was not covered by tests
ngmState := utils.NgmStateNotSupported
if constraint.Check(v) {

if s.featureNgm.IsSupported() {

Check warning on line 99 in pkg/apiserver/info/info.go

View check run for this annotation

Codecov / codecov/patch

pkg/apiserver/info/info.go#L98-L99

Added lines #L98 - L99 were not covered by tests
ngmState = utils.NgmStateNotStarted
addr, err := topology.FetchNgMonitoringTopology(s.lifecycleCtx, s.params.EtcdClient)
if err == nil && addr != "" {
ngmState = utils.NgmStateStarted
}
}

resp := InfoResponse{
Version: version.GetInfo(),
EnableTelemetry: s.params.Config.EnableTelemetry,
EnableExperimental: s.params.Config.EnableExperimental,
SupportedFeatures: s.params.FeatureFlags.SupportedFeatures(),
NgmState: ngmState,
}
c.JSON(http.StatusOK, resp)
return ngmState

Check warning on line 107 in pkg/apiserver/info/info.go

View check run for this annotation

Codecov / codecov/patch

pkg/apiserver/info/info.go#L107

Added line #L107 was not covered by tests
}

type WhoAmIResponse struct {
Expand Down
9 changes: 4 additions & 5 deletions pkg/apiserver/topsql/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,19 @@ type ServiceParams struct {
}

type Service struct {
FeatureTopSQL *featureflag.FeatureFlag

params ServiceParams
params ServiceParams
featureTopSQL *featureflag.FeatureFlag
}

func newService(p ServiceParams, ff *featureflag.Registry) *Service {
return &Service{params: p, FeatureTopSQL: ff.Register("topsql", ">= 5.4.0")}
return &Service{params: p, featureTopSQL: ff.Register("topsql", ">= 5.4.0")}
}

func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) {
endpoint := r.Group("/topsql")
endpoint.Use(
auth.MWAuthRequired(),
s.FeatureTopSQL.VersionGuard(),
s.featureTopSQL.VersionGuard(),
utils.MWConnectTiDB(s.params.TiDBClient),
)
{
Expand Down
8 changes: 4 additions & 4 deletions pkg/apiserver/user/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var (
)

type AuthService struct {
FeatureFlagNonRootLogin *featureflag.FeatureFlag
FeatureNonRootLogin *featureflag.FeatureFlag

middleware *jwt.GinJWTMiddleware
authenticators map[utils.AuthType]Authenticator
Expand Down Expand Up @@ -91,9 +91,9 @@ func NewAuthService(featureFlags *featureflag.Registry) *AuthService {
}

service := &AuthService{
FeatureFlagNonRootLogin: featureFlags.Register("nonRootLogin", ">= 5.3.0"),
middleware: nil,
authenticators: map[utils.AuthType]Authenticator{},
FeatureNonRootLogin: featureFlags.Register("nonRootLogin", ">= 5.3.0"),
middleware: nil,
authenticators: map[utils.AuthType]Authenticator{},
}

middleware, err := jwt.New(&jwt.GinJWTMiddleware{
Expand Down
4 changes: 0 additions & 4 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"crypto/tls"
"net/url"
"strings"

"github.com/pingcap/tidb-dashboard/pkg/utils/version"
)

const (
Expand All @@ -29,7 +27,6 @@ type Config struct {

EnableTelemetry bool
EnableExperimental bool
FeatureVersion string // assign the target TiDB version when running TiDB Dashboard as standalone mode
}

func Default() *Config {
Expand All @@ -42,7 +39,6 @@ func Default() *Config {
TiDBTLSConfig: nil,
EnableTelemetry: true,
EnableExperimental: false,
FeatureVersion: version.PDVersion,
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/user/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestUserSuite(t *testing.T) {
}

func (s *testUserSuite) supportNonRootLogin() bool {
return s.authService.FeatureFlagNonRootLogin.IsSupported()
return s.authService.FeatureNonRootLogin.IsSupported()
}

func (s *testUserSuite) SetupSuite() {
Expand Down
2 changes: 1 addition & 1 deletion ui/cypress.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"screenshotOnRunFailure": false,
"video": false,
"env": {
"FEATURE_VERSION": "6.0.0",
"PD_VERSION": "6.0.0",
"TIDB_VERSION": "nightly"
},
"experimentalSessionSupport": true,
Expand Down
12 changes: 6 additions & 6 deletions ui/cypress/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The Cypress has been added to package.json, so just run `yarn` to install it. We

### Open Test Runner to Run Test Locally

#### Test E2E with FEATURE_VERSION >= 5.3.0
#### Test E2E with PD_VERSION >= 5.3.0

```shell
# start frontend server
Expand All @@ -23,15 +23,15 @@ make dev && make run
cd ui && yarn open:cypress
```

#### Test E2E with FEATURE_VERSION < 5.3.0
#### Test E2E with PD_VERSION < 5.3.0

```shell
# start frontend server
cd ui && yarn start
# start backend server
make dev && make run FEATURE_VERSION=5.0.0
make dev && make run PD_VERSION=5.0.0
# open cypress test runner
cd ui && yarn open:cypress --env FEATURE_VERSION=5.0.0
cd ui && yarn open:cypress --env PD_VERSION=5.0.0
```

Run test by choosing test file under `/integration` on cypress test runner, cypress will open a broswer to run e2e test.
Expand All @@ -42,7 +42,7 @@ Run test by choosing test file under `/integration` on cypress test runner, cypr
# start frontend server
make ui
# start backend server
UI=1 make && make run FEATURE_VERSION=${FEATURE_VERSION}
UI=1 make && make run PD_VERSION=${PD_VERSION}
# run e2e_compat_features and e2e_common_features tests
make test_e2e FEATURE_VERSION=${FEATURE_VERSION}
make test_e2e PD_VERSION=${PD_VERSION}
```
Loading