mhausenblas / burry.sh

Cloud Native Infrastructure BackUp & RecoveRY
Apache License 2.0
260 stars 38 forks source link

Target parameter is misleading #20

Closed palmerabollo closed 5 years ago

palmerabollo commented 5 years ago

The README says that --target can take any of these values: local, s3, minio or tty. So I assume that you must set "minio" in case you want to use it as target.

However, in the example "Back up etcd to Minio" you use --target s3:

$ ./burry --endpoint etcd.mesos:1026 --isvc etcd --credentials play.minio.io:9000,ACCESS_KEY_ID=Q3AM3UQ867SPQQA43P2F,SECRET_ACCESS_KEY=zuf+tfteSlswRu7BJ86wekitnifILbZam1KYY3TG --target s3

Is that ok? I'm asking this because I'm not able to make burry work with minio using --target minio and I don't know if this might be the cause. I'm getting a "server gave HTTP response to HTTPS" error:

time="2018-11-08T22:30:57Z" level=info msg="Selected operation: BACKUP" func=main
2018/11/08 22:30:57 Connected to 100.70.205.228:2181
2018/11/08 22:30:57 Authenticated: id=101037703899381862, timeout=4000
2018/11/08 22:30:57 Re-submitting `0` credentials after reconnect
time="2018-11-08T22:30:57Z" level=info msg="Rewriting root" func=store
time="2018-11-08T22:31:00Z" level=fatal msg="Get https://127.0.0.1:9000/my-backups-bucket/?location=: http: server gave HTTP response to HTTPS client" func=toremoteS3

I'm using burry 0.4.0 and minio RELEASE.2018-11-06T01-01-02Z just in case it helps. Thanks.

palmerabollo commented 5 years ago

I see that useSSL is hardcoded https://github.com/mhausenblas/burry.sh/blob/master/remotes.go#L96, would it be possible to make it configurable?

mhausenblas commented 5 years ago

@palmerabollo sure it's possible. Care to send in a PR? :)

palmerabollo commented 5 years ago

@mhausenblas sure. Do you know if it's enough to add a new USESSL key to extractS3Config https://github.com/mhausenblas/burry.sh/blob/master/manifest.go#L85 and then change useSSL := s3Config.useSSL? https://github.com/mhausenblas/burry.sh/blob/master/remotes.go#L96

mhausenblas commented 5 years ago

Not sure @palmerabollo … I suppose one would need to try and run some tests to verify

palmerabollo commented 5 years ago

Fixed in https://github.com/mhausenblas/burry.sh/pull/23 It would be great if you could release a new version with the fixes. Thank you.