Bring etcd support for bucket DNS federation #5501
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #5501 +/- ##
==========================================
- Coverage 59.41% 58.79% -0.63%
==========================================
Files 217 218 +1
Lines 31056 31452 +396
==========================================
+ Hits 18453 18492 +39
- Misses 11010 11344 +334
- Partials 1593 1616 +23
Continue to review full report at Codecov.
|
harshavardhana
force-pushed
the
etcd
branch
10 times, most recently
from
d6053ee to
ed50de9
Compare
harshavardhana
changed the title
|
@harshavardhana I tried testing this PR using the docker-compose below and observe that minio instances fail to start with the error logs below. version: '2.1'
services:
etcd1:
image: quay.io/coreos/etcd:v2.3.8
ports:
- "4001:4001"
- "2380:2380"
- "2379:2379"
command: -name etcd0 -advertise-client-urls http://etcd1:2379 -listen-client-urls http://0.0.0.0:2379 -initial-advertise-peer-urls http://etcd1:2380 -listen-peer-urls http://0.0.0.0:2380 -initial-cluster-token etcd-cluster-1 -initial-cluster etcd0=http://etcd1:2380 -initial-cluster-state new
minio1:
image: minio:with-etcd
volumes:
- /home/kp/work/bucket-dns/data1:/data
ports:
- "9001:9000"
environment:
MINIO_ACCESS_KEY: minio
MINIO_SECRET_KEY: minio123
MINIO_ETCD_ENDPOINTS: http://etcd1:2379
MINIO_DOMAIN_IP: "etcd1"
MINIO_DOMAIN_NAME: "komboothi.io"
entrypoint: >
/bin/sh -c "
curl https://raw.githubusercontent.com/nitisht/minio-compose/master/wait-for.sh -o wait-for.sh;
chmod +x wait-for.sh;
./wait-for.sh etcd1:2379 -- /usr/bin/docker-entrypoint.sh minio server /data;
"
minio2:
image: minio:with-etcd
volumes:
- /home/kp/work/bucket-dns/data2:/data
ports:
- "9002:9000"
environment:
MINIO_ACCESS_KEY: minio
MINIO_SECRET_KEY: minio123
MINIO_ETCD_ENDPOINTS: http://etcd1:2379
MINIO_DOMAIN_IP: "etcd1"
MINIO_DOMAIN_NAME: "komboothi.io"
entrypoint: >
/bin/sh -c "
curl https://raw.githubusercontent.com/nitisht/minio-compose/master/wait-for.sh -o wait-for.sh;
chmod +x wait-for.sh;
./wait-for.sh etcd1:2379 -- /usr/bin/docker-entrypoint.sh minio server /data;
"
|
krisis
previously requested changes
Feb 15, 2018
cmd/common-main.go
Outdated
| if err := loadConfig(); err != nil { | ||
| if etcdc.IsKeyNotFound(err) { | ||
| fatalIf(newConfig(), "Unable to initialize minio config for the first time.") | ||
| log.Println("Created minio configuration file successfully at", globalEtcdClient.Endpoints()) |
There was a problem hiding this comment.
process exits on fatalIf. log.Println after that doesn't have any effect.
There was a problem hiding this comment.
process exits on fatalIf. log.Println after that doesn't have any effect.
Only on error @krisis if not it prints the log.Println
| writeErrorResponse(w, toAPIErrorCode(err), r.URL) | ||
| return | ||
|
|
||
| // If etcd, dns federation configured list buckets from etcd. |
There was a problem hiding this comment.
The comment could be more descriptive like,
// if etcd is configured, list buckets from its key-value store. This would list only those buckets that were created after configuring etcd-coredns with minio instances.
There was a problem hiding this comment.
We need to fix this behavior @krisis so your comment might not be appropriate.
| ## Federation (Bucket Lookup) | ||
| Bucket lookup federation requires two dependencies | ||
|
|
||
| - etcd (for config, bucket SRV records) |
There was a problem hiding this comment.
we should expand on what information is being persisted in etcd.
There was a problem hiding this comment.
what would be better to list?
docs/federation/lookup/README.md
Outdated
| minio server http://rack{5...8}.host{5...8}.domain.com/mnt/export{1...32} | ||
| ``` | ||
|
|
||
| In this configuration you can see `MINIO_ETCD_ENDPOINTS` points to the etcd backend which manages our |
docs/federation/lookup/README.md
Outdated
| # Federation | ||
| There are primarily two types of federation | ||
|
|
||
| - Bucket lookup |
There was a problem hiding this comment.
We should explain (at least briefly) what we mean when we say Bucket lookup and Bucket sharded.
cmd/server-main.go
Outdated
| BUCKET-DNS: | ||
| MINIO_DOMAIN: To enable virtual-host-style requests. | ||
| MINIO_DOMAIN_IP: To enable virtual-host-style requests. |
There was a problem hiding this comment.
A note on sample values or allowed values would be useful.
There was a problem hiding this comment.
Will document them separately in a doc. since this is just informative.
harshavardhana
force-pushed
the
etcd
branch
2 times, most recently
from
3e3576c to
b88a802
Compare
harshavardhana
force-pushed
the
etcd
branch
2 times, most recently
from
7eb1249 to
10ff6b6
Compare
|
#5999 fixed here.. |
harshavardhana
force-pushed
the
etcd
branch
3 times, most recently
from
1b756ca to
74748a4
Compare
|
@vadmeste @krishnasrinivas @krisis any approvals or wrong things in the PR? |
krisis
reviewed
Jun 5, 2018
cmd/common-main.go
Outdated
| var err error | ||
| globalEtcdClient, err = etcd.New(etcd.Config{ | ||
| Endpoints: etcdEndpoints, | ||
| Transport: &http.Transport{ |
There was a problem hiding this comment.
we could use the existing http.Transport
cmd/common-main.go
Outdated
| logger.FatalIf(err, "Unable to initialize etcd with %s", etcdEndpoints) | ||
| } | ||
|
|
||
| globalDomainName, ok = os.LookupEnv("MINIO_DOMAIN") |
There was a problem hiding this comment.
suggestion: This can be simplified.
globalDomainName, globalIsEnvDomainName = os.LookupEnv("MINIO_DOMAIN")
| // IMPORTANT NOTE: When updating this struct make sure that | ||
| // serverConfig.ConfigDiff() is updated as necessary. | ||
| type serverConfigV23 struct { | ||
| quick.Config `json:"-"` // ignore interfaces |
There was a problem hiding this comment.
this would move to serverConfigV24 on rebase
| _, dok := os.LookupEnv("MINIO_DOMAIN") | ||
| _, eok := os.LookupEnv("MINIO_ETCD_ENDPOINTS") | ||
| _, iok := os.LookupEnv("MINIO_PUBLIC_IPS") | ||
| if dok && eok && !iok { |
There was a problem hiding this comment.
why do we depend on dok and eok to update domain IPs from endpoint list?
There was a problem hiding this comment.
globalDomainIPs is only needed for federation that's why.
| func setBucketForwardingHandler(h http.Handler) http.Handler { | ||
| fwd := handlers.NewForwarder(&handlers.Forwarder{ | ||
| PassHost: true, | ||
| RoundTripper: NewCustomHTTPTransport(), |
There was a problem hiding this comment.
do we need a new http transport? Could we reuse an existing transport?
There was a problem hiding this comment.
it is already using existing transport, it just initializes a new one while registering the handler and reuses in all future operations.
krisis
reviewed
Jun 5, 2018
cmd/object-handlers.go
Outdated
| return | ||
| } | ||
| // Close writer explicitly to indicate data has been written | ||
| defer srcInfo.Writer.Close() |
harshavardhana
force-pushed
the
etcd
branch
5 times, most recently
from
67223a4 to
03b99bf
Compare
- Supports centralized `config.json` - Supports centralized `bucket` service records for client lookups - implement a new proxy forwarder
Buckets already present on a Minio server before it joins a bucket federated deployment will now be added to etcd during startup. In case of a bucket name collision, admin is informed via Minio server console message. Added configuration migration for configuration stored in etcd backend. Also, environment variables are updated and ListBucket path style request is no longer forwarded.
harshavardhana
force-pushed
the
etcd
branch
2 times, most recently
from
1067107 to
0f3f57d
Compare
krisis
previously approved these changes
Jun 8, 2018
This PR adds CopyObject support for objects residing in buckets in different Minio instances (where Minio instances are part of a federated setup). Also, added support for multiple Minio domain IPs. This is required for distributed deployments, where one deployment may have multiple nodes, each with a different public IP.
Also use hosts passed to Minio startup command to populate IP addresses if MINIO_PUBLIC_IPS is not set.
Mint Automation
5501-58ebea4/mint-fs.sh.log:5501-58ebea4/mint-gateway-azure.sh.log:5501-58ebea4/mint-xl.sh.log:5501-58ebea4/mint-dist-xl.sh.log:5501-58ebea4/mint-gateway-s3.sh.log: |
krisis
approved these changes
Jun 8, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Bring etcd support for bucket DNS federation
Motivation and Context
config.jsonbucketservice recordsfor client lookups
How Has This Been Tested?
Manually, using multiple servers
Types of changes
Checklist:
mintPR # here: )