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

Flexibility to provide backup engine on the helm chart #5850

Closed
wants to merge 8 commits into from
Closed

Flexibility to provide backup engine on the helm chart #5850

wants to merge 8 commits into from

Conversation

yindia
Copy link
Contributor

@yindia yindia commented Feb 24, 2020

Fixed #5847

Yuvraj added 3 commits February 24, 2020 20:26
Signed-off-by: Yuvraj <yuvraj.yad001@gmail.com>
Signed-off-by: Yuvraj <yuvraj.yad001@gmail.com>
Signed-off-by: Yuvraj <yuvraj.yad001@gmail.com>
@sougou sougou requested a review from deepthi February 24, 2020 15:57
@yindia
Copy link
Contributor Author

yindia commented Feb 24, 2020

@deepthi @derekperkins I am reading the documentation for backup(https://vitess.io/docs/user-guides/backup-and-restore/). Should we provide all flag configurable via helm or just add backup_engine_implementation in the helm.

@deepthi
Copy link
Member

deepthi commented Feb 24, 2020

@deepthi @derekperkins I am reading the documentation for backup(https://vitess.io/docs/user-guides/backup-and-restore/). Should we provide all flag configurable via helm or just add backup_engine_implementation in the helm.

I think we should allow all xtrabackup flags to be configurable via helm.

Signed-off-by: Yuvraj <yuvraj.yad001@gmail.com>
@yindia
Copy link
Contributor Author

yindia commented Feb 24, 2020

@deepthi ready for first round of code review

Yuvraj added 2 commits February 25, 2020 02:05
Signed-off-by: Yuvraj <yuvraj.yad001@gmail.com>
Signed-off-by: Yuvraj <yuvraj.yad001@gmail.com>
@yindia yindia changed the title Fix 5847 Flexibility to provide backup engine on the helm chart Feb 24, 2020
@deepthi
Copy link
Member

deepthi commented Feb 28, 2020

I'll defer to @derekperkins for the actual review and approval. Don't know much about helm.

@deepthi
Copy link
Member

deepthi commented Mar 23, 2020

@derekperkins ping

@derekperkins
Copy link
Member

Sorry for the delay. I had one question about indentation on the yaml. Also, if you could add an example to the readme, that'd be great. https://github.com/vitessio/vitess/tree/master/helm/vitess We'll go ahead and merge it after that.

Signed-off-by: Yuvraj <yuvraj.yad001@gmail.com>
Signed-off-by: Yuvraj <yuvraj.yad001@gmail.com>
@sougou
Copy link
Contributor

sougou commented Apr 23, 2020

What's the status on this PR?

@yindia
Copy link
Contributor Author

yindia commented Apr 23, 2020

It is ready for the review

@deepthi
Copy link
Member

deepthi commented Apr 23, 2020

It is ready for the review

@derekperkins ping

@deepthi
Copy link
Member

deepthi commented May 11, 2020

Trying to re-run failed tests.

@deepthi deepthi closed this May 11, 2020
@deepthi deepthi reopened this May 11, 2020
@deepthi
Copy link
Member

deepthi commented May 12, 2020

hi @evalsocket can you confirm that the helm example works fine with these changes? Then I can approve and merge.

@sougou
Copy link
Contributor

sougou commented Jul 27, 2020

Now that we're deprecating helm #6439, I think we can close this PR. LMK if anyone has other thoughts.

@deepthi
Copy link
Member

deepthi commented Aug 24, 2020

Now that we're deprecating helm #6439, I think we can close this PR. LMK if anyone has other thoughts.

Closing now.

@deepthi deepthi closed this Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flexibility to provide backup engine on the helm chart
4 participants