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

add listen_host for TiDB/TiKV/PD to specified the listen host #495

Merged
merged 10 commits into from
Jun 15, 2020

Conversation

crazycs520
Copy link
Contributor

@crazycs520 crazycs520 commented Jun 11, 2020

Signed-off-by: crazycs520 crazycs520@gmail.com

What problem does this PR solve?

Before this PR, TiDB/TiKV will always listen to the 0.0.0.0 host, this PR adds listen_host flag for PD/TiDB/TiKV to specify the listening address.

If user doesn't specified the listen_host, PD/TiDB/TiKV will listen on 0.0.0.0 defaultly.

What is changed and how it works?

Check List

Tests

  • Manual test

Signed-off-by: crazycs520 <crazycs520@gmail.com>
pkg/cluster/meta/logic.go Outdated Show resolved Hide resolved
pkg/cluster/meta/topology.go Outdated Show resolved Hide resolved
pkg/cluster/template/scripts/tidb.go Outdated Show resolved Hide resolved
Signed-off-by: crazycs520 <crazycs520@gmail.com>
@crazycs520 crazycs520 changed the title add listen_address for TiDB/TiKV to specified the listen address add listen_address for TiDB/TiKV/PD to specified the listen address Jun 11, 2020
Signed-off-by: crazycs520 <crazycs520@gmail.com>
Signed-off-by: crazycs520 <crazycs520@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2020

Codecov Report

Merging #495 into master will decrease coverage by 0.78%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #495      +/-   ##
==========================================
- Coverage   39.84%   39.06%   -0.79%     
==========================================
  Files         201      205       +4     
  Lines       14847    15407     +560     
==========================================
+ Hits         5916     6018     +102     
- Misses       8072     8512     +440     
- Partials      859      877      +18     
Flag Coverage Δ
#coverage 39.06% <ø> (-0.79%) ⬇️
Impacted Files Coverage Δ
...m/pingcap/tiup/components/playground/playground.go 19.67% <0.00%> (-8.94%) ⬇️
go/src/github.com/pingcap/tiup/cmd/mirror.go 41.56% <0.00%> (-5.27%) ⬇️
...c/github.com/pingcap/tiup/pkg/utils/http_client.go 71.42% <0.00%> (-4.77%) ⬇️
go/src/github.com/pingcap/tiup/cmd/root.go 60.43% <0.00%> (-3.85%) ⬇️
...ingcap/tiup/components/playground/instance/tidb.go 86.95% <0.00%> (-3.75%) ⬇️
...ingcap/tiup/components/playground/instance/tikv.go 77.04% <0.00%> (-2.27%) ⬇️
...om/pingcap/tiup/components/cluster/command/root.go 43.03% <0.00%> (-2.24%) ⬇️
...cap/tiup/components/playground/instance/tiflash.go 62.79% <0.00%> (-0.99%) ⬇️
...ingcap/tiup/components/cluster/command/scale_in.go 10.34% <0.00%> (-0.77%) ⬇️
...om/pingcap/tiup/components/cluster/command/stop.go 21.21% <0.00%> (-0.67%) ⬇️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 782b9fa...2c9dfb9. Read the comment docs.

@@ -127,6 +127,7 @@ func AllComponentNames() (roles []string) {
// TiDBSpec represents the TiDB topology specification in topology.yaml
type TiDBSpec struct {
Host string `yaml:"host"`
ListenAddress string `yaml:"listen_address,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think listen_address is a bad name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

em... Any suggestion?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe call it listen_host? (since Address should contain the port part)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Signed-off-by: crazycs520 <crazycs520@gmail.com>
@crazycs520 crazycs520 changed the title add listen_address for TiDB/TiKV/PD to specified the listen address add listen_host for TiDB/TiKV/PD to specified the listen address Jun 15, 2020
@crazycs520 crazycs520 changed the title add listen_host for TiDB/TiKV/PD to specified the listen address add listen_host for TiDB/TiKV/PD to specified the listen host Jun 15, 2020
Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

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

Verified that it works! Could you also open a PR for tiup playground as well?

@lonng lonng merged commit a33f097 into pingcap:master Jun 15, 2020
@lonng
Copy link
Contributor

lonng commented Jun 15, 2020

@crazycs520 Please update the documentation/examples for listen_host field, thanks.

@@ -24,6 +24,7 @@ exec env GODEBUG=madvdontneed=1 bin/tidb-server \
{{- end}}
-P {{.Port}} \
--status="{{.StatusPort}}" \
--host="{{.ListenAddress}}" \

Choose a reason for hiding this comment

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

Please add --status-host="{{.ListenAddress}}" , too

Copy link
Contributor

Choose a reason for hiding this comment

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

@AstroProfundis AstroProfundis mentioned this pull request Jul 7, 2020
fln added a commit to fln/tiup that referenced this pull request Nov 26, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in pingcap#495 and is already used for
the --client-urls flag.
fln added a commit to fln/tiup that referenced this pull request Nov 26, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in pingcap#495 and is already used for
the --client-urls flag.

This should resolve pingcap#337 and partially implement pingcap#691.
fln added a commit to fln/tiup that referenced this pull request Nov 29, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in pingcap#495 and is already used for
the --client-urls flag.

This should resolve pingcap#337 and partially implement pingcap#691.
fln added a commit to fln/tiup that referenced this pull request Nov 30, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in pingcap#495 and is already used for
the --client-urls flag.

This should resolve pingcap#337 and partially implement pingcap#691.
fln added a commit to fln/tiup that referenced this pull request Nov 30, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in pingcap#495 and is already used for
the --client-urls flag.

This should resolve pingcap#337 and partially implement pingcap#691.
fln added a commit to fln/tiup that referenced this pull request Nov 30, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in pingcap#495 and is already used for
the --client-urls flag.

This should resolve pingcap#337 and partially implement pingcap#691.
fln added a commit to fln/tiup that referenced this pull request Nov 30, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in pingcap#495 and is already used for
the --client-urls flag.

This should resolve pingcap#337 and partially implement pingcap#691.
fln added a commit to fln/tiup that referenced this pull request Nov 30, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in pingcap#495 and is already used for
the --client-urls flag.

This should resolve pingcap#337 and partially implement pingcap#691.
fln added a commit to fln/tiup that referenced this pull request Nov 30, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in pingcap#495 and is already used for
the --client-urls flag.

This should resolve pingcap#337 and partially implement pingcap#691.
ti-chi-bot added a commit that referenced this pull request Dec 1, 2020
This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in #495 and is already used for
the --client-urls flag.

This should resolve #337 and partially implement #691.

Co-authored-by: SIGSEGV <gnu.crazier@gmail.com>
Co-authored-by: Ti Prow Robot <71242396+ti-community-prow-bot@users.noreply.github.com>
ti-chi-bot pushed a commit that referenced this pull request Dec 8, 2020
* Support host names in TLS certificates

This commit updates TLS certificate generator to detect if IP address or
host name was used as host value. If host name is detected field `DNSNames`
of x509 SAN extenstion is used instead of `IPAddresses`.

* https://en.wikipedia.org/wiki/Subject_Alternative_Name
* https://tools.ietf.org/html/rfc5280#section-4.2.1.6

This contributes towards fixing #337.

* Fix PD --peer-urls flag to use listen_host variable

This commit updates run_pd.sh template to use `listen_host` for
--peer-urls flag instead of `host`.

This allows us to use server hostname instead of IP addresse. Without
this change if PD service is declared using hostname it fails to start,
reporting an error that it can not bind to given interface.

Metadata variable `listen_host` was introduced in #495 and is already used for
the --client-urls flag.

This should resolve #337 and partially implement #691.

* Switch to host names in integration tests

This PR updates refactors integration tests to use host names instead of
IP addresses. It resolves #337.

All IP literals in the integration tests are replaced with host names
`n1` to `n5`.

This allows us to make integration test topology files immutable and
remove extra topology file templating step.

Extra topology file `full_scale_in_tidb_2nd.yaml` is introduced for
second tidb scaling operation to avoid topology file mutation during the
test.

Using host names in the integration tests will help us to maintain
compatibility with services declared using host names. There should be
no extra maintenance to support using IP addresses.

Co-authored-by: SIGSEGV <gnu.crazier@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants