coreos / etcd-operator

etcd operator creates/configures/manages etcd clusters atop Kubernetes
https://coreos.com/blog/introducing-the-etcd-operator.html
Apache License 2.0
1.75k stars 741 forks source link

Azure blob storage: use correct list prefix #2071

Closed kapouille closed 5 years ago

kapouille commented 5 years ago

Azure bloc storage implementation didn't use the right prefix resulting on no backup cleanup being performed.

etcd-bot commented 5 years ago

Can one of the admins verify this patch?

etcd-bot commented 5 years ago

Can one of the admins verify this patch?

etcd-bot commented 5 years ago

Can one of the admins verify this patch?

hexfusion commented 5 years ago

@etcd-bot ok to test

hexfusion commented 5 years ago

@kapouille thanks for the PR. As we do not have testing for Azure setup can you provide a bit more details on how this resolves the situation?

@rjtsdl I see you are with Oracle now but as a maintainer of Azure ABS feature could you please comment/review.

kapouille commented 5 years ago

of course @hexfusion . In order to list the existing backups and see which ones are out of date or not, it used to retrieve the wrong url, including the container name (<my_container>/<path>). That would return an empty list and therefore never clean up older backups as the azure API only expects <path>

kapouille commented 5 years ago

(apologies about the force-push, really should have done that work in a branch)

kapouille commented 5 years ago

Putting on hold, noticed an odd behaviour where it is keeping the oldest backups rather than the most recent. Investigating.

kapouille commented 5 years ago

Could not repro issue appears to be a one-off due to changing file naming conventions.

hasbro17 commented 5 years ago

@kapouille Sorry this has been open for a while. This seems good to me. Can you please rebase and also add a line in the Fixed section of the CHANGELOG for this.

hasbro17 commented 5 years ago

@kapouille pinging again to see if you have time to update the PR. If not then I can close this out and resubmit the bugfix.

kapouille commented 5 years ago

Apologies, will get on that tomorrow.

kapouille commented 5 years ago

I believe this is now done.