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

vtctld: flag to proxy vttablet URLs #6058

Merged
merged 2 commits into from
Apr 13, 2020
Merged
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
45 changes: 43 additions & 2 deletions go/vt/vtctld/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"vitess.io/vitess/go/vt/mysqlctl"
logutilpb "vitess.io/vitess/go/vt/proto/logutil"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/proto/vttime"
)

var (
Expand All @@ -57,6 +58,23 @@ const (
jsonContentType = "application/json; charset=utf-8"
)

// TabletWithURL wraps topo.Tablet and adds a URL property.
type TabletWithURL struct {
Alias *topodatapb.TabletAlias `json:"alias,omitempty"`
Hostname string `json:"hostname,omitempty"`
PortMap map[string]int32 `json:"port_map,omitempty"`
Keyspace string `json:"keyspace,omitempty"`
Shard string `json:"shard,omitempty"`
KeyRange *topodatapb.KeyRange `json:"key_range,omitempty"`
Type topodatapb.TabletType `json:"type,omitempty"`
DbNameOverride string `json:"db_name_override,omitempty"`
Tags map[string]string `json:"tags,omitempty"`
MysqlHostname string `json:"mysql_hostname,omitempty"`
MysqlPort int32 `json:"mysql_port,omitempty"`
MasterTermStartTime *vttime.Time `json:"master_term_start_time,omitempty"`
URL string `json:"url,omitempty"`
}

func httpErrorf(w http.ResponseWriter, r *http.Request, format string, args ...interface{}) {
errMsg := fmt.Sprintf(format, args...)
log.Errorf("HTTP error on %v: %v, request: %#v", r.URL.Path, errMsg, r)
Expand Down Expand Up @@ -90,6 +108,7 @@ func handleCollection(collection string, getFunc func(*http.Request) (interface{

// JSON encode response.
data, err := vtctl.MarshalJSON(obj)
log.Flush()
if err != nil {
return fmt.Errorf("cannot marshal data: %v", err)
}
Expand Down Expand Up @@ -372,8 +391,30 @@ func initAPI(ctx context.Context, ts *topo.Server, actions *ActionRepository, re
if err != nil {
return nil, err
}
// Pass the embedded proto directly or jsonpb will panic.
return t.Tablet, err

tab := &TabletWithURL{
Alias: t.Alias,
Hostname: t.Hostname,
PortMap: t.PortMap,
Keyspace: t.Keyspace,
Shard: t.Shard,
KeyRange: t.KeyRange,
Type: t.Type,
DbNameOverride: t.DbNameOverride,
Tags: t.Tags,
MysqlHostname: t.MysqlHostname,
MysqlPort: t.MysqlPort,
MasterTermStartTime: t.MasterTermStartTime,
}
tabletPath = fmt.Sprintf("http://%s:%d", t.Tablet.Hostname, t.PortMap["vt"])
if *proxyTablets {
tabletID := fmt.Sprintf("%s-%d", t.Tablet.Alias.Cell, t.Tablet.Alias.Uid)
addRemote(tabletID, tabletPath)
tab.URL = fmt.Sprintf("/vttablet/%s-%d", t.Tablet.Alias.Cell, t.Tablet.Alias.Uid)
} else {
tab.URL = tabletPath
}
return tab, nil
})

// Healthcheck real time status per (cell, keyspace, tablet type, metric).
Expand Down
8 changes: 1 addition & 7 deletions go/vt/vtctld/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,20 +193,14 @@ func TestAPI(t *testing.T) {
{"GET", "tablets/?shard=ks1%2F80-&cell=cell1", "", `[]`},
{"GET", "tablets/cell1-100", "", `{
"alias": {"cell": "cell1", "uid": 100},
"hostname": "",
"port_map": {"vt": 100},
"keyspace": "ks1",
"shard": "-80",
"key_range": {
"start": null,
"end": "gA=="
},
"type": 2,
"db_name_override": "",
"tags": {},
"mysql_hostname":"",
"mysql_port":0,
"master_term_start_time":null
"url":"http://:100"
}`},
{"GET", "tablets/nonexistent-999", "", "404 page not found"},
{"POST", "tablets/cell1-100?action=TestTabletAction", "", `{
Expand Down
68 changes: 34 additions & 34 deletions go/vt/vtctld/rice-box.go

Large diffs are not rendered by default.

81 changes: 81 additions & 0 deletions go/vt/vtctld/vttablet_proxy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
Copyright 2020 The Vitess Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package vtctld

import (
"bytes"
"flag"
"fmt"
"io/ioutil"
"net/http"
"net/http/httputil"
"net/url"
"strings"
"sync"

"vitess.io/vitess/go/vt/log"
)

var (
proxyTablets = flag.Bool("proxy_tablets", false, "Setting this true will make vtctld proxy the tablet status instead of redirecting to them")

proxyMu sync.Mutex
remotes = make(map[string]*httputil.ReverseProxy)
)

func addRemote(tabletID, path string) {
u, err := url.Parse(path)
if err != nil {
log.Errorf("Error parsing URL %v: %v", path, err)
return
}

proxyMu.Lock()
defer proxyMu.Unlock()
if _, ok := remotes[tabletID]; ok {
return
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean the proxy will stop working for a given tablet after its address changes?

}

rp := &httputil.ReverseProxy{}
rp.Director = func(req *http.Request) {
splits := strings.SplitN(req.URL.Path, "/", 4)
if len(splits) < 4 {
return
}
req.URL.Scheme = u.Scheme
req.URL.Host = u.Host
req.URL.Path = "/" + splits[3]
}

prefixPath := fmt.Sprintf("/vttablet/%s/", tabletID)

rp.ModifyResponse = func(r *http.Response) error {
b, _ := ioutil.ReadAll(r.Body)
b = bytes.ReplaceAll(b, []byte(`href="/`), []byte(fmt.Sprintf(`href="%s`, prefixPath)))
b = bytes.ReplaceAll(b, []byte(`href=/`), []byte(fmt.Sprintf(`href=%s`, prefixPath)))
r.Body = ioutil.NopCloser(bytes.NewBuffer(b))
r.Header["Content-Length"] = []string{fmt.Sprint(len(b))}
// Don't forget redirects
loc := r.Header["Location"]
for i, v := range loc {
loc[i] = strings.Replace(v, "/", prefixPath, 1)
}
return nil
}
http.Handle(prefixPath, rp)
Copy link
Member

Choose a reason for hiding this comment

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

It would be more natural to respond to tablets coming/going and changing addresses if there were only one handler, which looks at a cached lookup table of tablet aliases to addresses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be more natural to respond to tablets coming/going and changing addresses if there were only one handler, which looks at a cached lookup table of tablet aliases to addresses.

This is what I started out with. The problem with the unified approach is that there is no easy way to associate a response to a request, which is needed for rewriting URLs to match the tablet id. I looked at making the vttablets send an extra header identifying the tablet id. But the one-proxy-per-tablet felt much simpler.

What I'll instead do is replace the per-tablet proxy every time. I looked at the code. It's just metdata (no goroutines etc). So, it should be fine.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see. Couldn't you still have a single handler though? If reverse proxy objects are cheap to make, you can just make a new one for each request you handle and use it once. Then you don't have handlers slowly leaking in cases when tablet IDs go away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the page that renders the tablets is the one that causes the map entry to be created.

Thinking about it again, there's no real need for that page to be controlling the redirects. We could directly GetTablet at the time of request, create the redirect and forward it. Let me update the other PR to reflect this.

remotes[tabletID] = rp
}
2 changes: 1 addition & 1 deletion web/vtctld2/app/main.3a2141f06032f2bc1229.bundle.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/vtctld2/src/app/dashboard/shard.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ <h1 class="vt-title">{{keyspaceName}}/{{shardName}}</h1>
</p-column>
<p-column header="Status">
<template let-tab="rowData">
<a href="{{getUrl(tab)}}">
<a href="{{tab.url)}}">
<button md-button>Status</button>
</a>
</template>
Expand Down