-
Notifications
You must be signed in to change notification settings - Fork 10
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
Allow to disable backrest container #132
Allow to disable backrest container #132
Conversation
4866aac
to
41d6394
Compare
3ef66c6
to
fd96920
Compare
Add enabled field for BackRestSidecar spec. By default enabled is true (original behaviour). Setting to false will drop back-rest container deployment.
b29b31d
to
282ba56
Compare
@cscetbon Could you take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one question about the way you set the current repository, otherwise it looks good.
- id: lower-repo | ||
shell: pwsh | ||
run: | | ||
"::set-output name=repository::$($env:GITHUB_REPOSITORY.ToLowerInvariant())" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use github.event.repository.name
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will give an error if the repository name has some uppercase letters (exactly what AKamyshnikova has).
The is a discussion here https://github.com/orgs/community/discussions/10553 and I used one of the suggested options to workaround this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @AKamyshnikova 👏
What's in this PR?
Add enabled field for BackRestSidecar spec.
By default enabled is true (original behaviour).
Setting to false will drop back-rest container deployment.
Why?
If out of box backup/restore tool is used - deploy of back-rest container is not necessary.
Checklist