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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a8280e6
fix ui doesn't work when running backend without 'feature-version'
baurine Feb 10, 2022
cf1d6e4
refine
baurine Feb 11, 2022
86bfa76
unify PDVersion and FeatureVersion
baurine Feb 11, 2022
dde104b
refine
baurine Feb 11, 2022
88239ed
check pd version when starting
baurine Feb 11, 2022
1138739
rename FEATURE_VERSION to PD_VERSION
baurine Feb 11, 2022
09b04ad
Merge remote-tracking branch 'origin/master' into fix-ui-not-work
baurine Feb 11, 2022
6c51e0b
fix lint
baurine Feb 11, 2022
24ea2e8
Merge branch 'master' into fix-ui-not-work
breezewish Feb 22, 2022
948703b
Merge branch 'master' into fix-ui-not-work
baurine Feb 23, 2022
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
49 changes: 32 additions & 17 deletions pkg/apiserver/info/info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,28 +78,12 @@ type InfoResponse struct { // nolint
// @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 {
_ = c.Error(err)
return
}
constraint, err := semver.NewConstraint(">= v5.4.0")
ngmState, err := s.checkNgmState()
if err != nil {
_ = c.Error(err)
return
}

ngmState := utils.NgmStateNotSupported
if constraint.Check(v) {
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,
Expand All @@ -110,6 +94,37 @@ func (s *Service) infoHandler(c *gin.Context) {
c.JSON(http.StatusOK, resp)
}

func (s *Service) checkNgmState() (utils.NgmState, error) {
ngmState := utils.NgmStateNotSupported

// The FeatureVersion value is "N/A" when running `make && bin/tidb-dashboard`
// The FeatureVersion value is "Unknown" when running `go run cmd/tidb-dashboard/main.go`
featureSupported :=
Copy link
Member

Choose a reason for hiding this comment

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

It's interesting, why do we have these two different values? Is it possible to be simply "", which is the well-known default value for a missing string?

Copy link
Collaborator Author

@baurine baurine Feb 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The config.FeatureVersion is assigned to version.PDVersion default.

func Default() *Config {
	return &Config{
		DataDir:            "/tmp/dashboard-data",
		TempDir:            "",
		PDEndPoint:         "http://127.0.0.1:2379",
		PublicPathPrefix:   defaultPublicPathPrefix,
		ClusterTLSConfig:   nil,
		TiDBTLSConfig:      nil,
		EnableTelemetry:    true,
		EnableExperimental: false,
		FeatureVersion:     version.PDVersion,
	}
}

and the version.PDVeresion default value is "Unknown".

// Version information. It will be overwritten by LDFLAGS.
var (
	InternalVersion = "Unknown"
	Standalone      = "Unknown" // Unknown, Yes or No
	PDVersion       = "Unknown"
	BuildTime       = "Unknown"
	BuildGitHash    = "Unknown"
)

Its value is overwritten to "N/A" by LDFLAGS when building in dev mode.

LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.PDVersion=N/A"

Let me try to assign FEATURE_VERSION to "" if PDVersion is "N/A" and "Unknown".

Copy link
Member

Choose a reason for hiding this comment

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

Can we unify the PDVersion and the FEATURE_VERSION? Maybe we can specify the pd version during development.

Copy link
Member

Choose a reason for hiding this comment

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

How about not assigning version.PDVersion to Default()? Will it cause any problems?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about not assigning version.PDVersion to Default()? Will it cause any problems?

I think it should be the same effect. we eventually need to use the version.PDVersion, before or later.

s.params.Config.FeatureVersion == "N/A" ||
s.params.Config.FeatureVersion == "Unknown"
if !featureSupported {
// drop "-alpha-xxx" suffix
versionWithoutSuffix := strings.Split(s.params.Config.FeatureVersion, "-")[0]
v, err := semver.NewVersion(versionWithoutSuffix)
if err != nil {
return ngmState, err
}
constraint, _ := semver.NewConstraint(">= v5.4.0")
baurine marked this conversation as resolved.
Show resolved Hide resolved
featureSupported = constraint.Check(v)
}
if !featureSupported {
return ngmState, nil
}

ngmState = utils.NgmStateNotStarted
addr, err := topology.FetchNgMonitoringTopology(s.lifecycleCtx, s.params.EtcdClient)
if err == nil && addr != "" {
ngmState = utils.NgmStateStarted
}

return ngmState, nil
}

type WhoAmIResponse struct {
DisplayName string `json:"display_name"`
IsShareable bool `json:"is_shareable"`
Expand Down
6 changes: 6 additions & 0 deletions util/featureflag/featureflag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ func (f *FeatureFlag) VersionGuard() gin.HandlerFunc {
// pdVersion, standaloneVersion examples: "v5.2.2", "v5.3.0", "v5.4.0-alpha-xxx", "5.3.0" (semver can handle `v` prefix by itself)
// constraints examples: "~5.2.2", ">= 5.3.0", see semver docs to get more information.
func (f *FeatureFlag) isSupportedIn(targetVersion string) bool {
// The targetVersion value is "N/A" when running `make && bin/tidb-dashboard`
// The targetVersion value is "Unknown" when running `go run cmd/tidb-dashboard/main.go`
if targetVersion == "N/A" || targetVersion == "Unknown" {
return true
}

// drop "-alpha-xxx" suffix
versionWithoutSuffix := strings.Split(targetVersion, "-")[0]
v, err := semver.NewVersion(versionWithoutSuffix)
Expand Down