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

sidecar: Allow sidecar to have write (no delete) access: Avoid deleting on failed upload. #3744

Closed
bwplotka opened this issue Jan 25, 2021 · 7 comments

Comments

@bwplotka
Copy link
Member

bwplotka commented Jan 25, 2021

Ideally, we don't want to give delete access to sidecar. And we don't need to delete partial uploads anymore as they are handled well on compactor/store gw sides.

Discussion https://cloud-native.slack.com/archives/CL25937SP/p1611572804008400

AC:

  • We can run a sidecar without Deletion Permission.
@SuperQ
Copy link
Contributor

SuperQ commented Jan 25, 2021

Yes, right now we need to give Object Admin level access to the sidecar, rather than just Create and Read.

@mayankshah1607
Copy link

Hi @bwplotka
I'd love to take up this issue and make my first contribution here!

@mayankshah1607
Copy link

Just a very quick question - how many times should the upload be retried, and for how long?

@mayankshah1607
Copy link

mayankshah1607 commented Feb 1, 2021

I have opened a PR - #3756
Not entirely sure if this would be the right approach, would love to have some feedback :)

@bwplotka bwplotka changed the title sidecar: Avoid deleting on failed upload; retry instead sidecar: Avoid deleting on failed upload Mar 10, 2021
@bwplotka bwplotka changed the title sidecar: Avoid deleting on failed upload sidecar: Allow sidecar to have write (no delete) access: Avoid deleting on failed upload. Mar 10, 2021
@bwplotka
Copy link
Member Author

Looks like somehow we have two PRs for the same thing (: Which should we review? Sorry for the lag in reviewing.

Let's maybe first have PR that removes deleting (only for sidecar), that would be the first step, However, both PRs @Biswajitghosh98 and @mayankshah1607 are about retries which are quite controversial.

Sorry, I put retries in the title - I rather meant delete the deletion action for the sidecar and the sidecar will retry automatically on crash. Let's start there.

Improving retries is not as easy as we think, created separate issue for this: #3907

@stale
Copy link

stale bot commented Jun 2, 2021

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

@stale stale bot added the stale label Jun 2, 2021
@stale
Copy link

stale bot commented Jun 16, 2021

Closing for now as promised, let us know if you need this to be reopened! 🤗

@stale stale bot closed this as completed Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants