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

US-389994 Initial helm charts SRS backing service #211

Merged

Conversation

pega-kshev
Copy link
Contributor

Adding helm charts for Search and Reporting backing service.
TODO: add test files and README.md content for backingservices chart.

@pega-kshev
Copy link
Contributor Author

Hi @dcasavant the PR is ready for review, kindly also include other reviewers aswell. Thank you.

@dcasavant dcasavant requested review from MadhuriArugula, a team and dcasavant December 2, 2020 17:16
@dcasavant
Copy link
Member

@taz-mon this will need to be added to the runbooks for 8.6

@dcasavant dcasavant added the enhancement New feature or request label Dec 2, 2020
Copy link
Contributor

@taz-pega-work taz-pega-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for including me in your review. I have a few suggestions to the client-facing documents.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
charts/backingservices/Chart.yaml Outdated Show resolved Hide resolved
charts/backingservices/README.md Outdated Show resolved Hide resolved
charts/backingservices/README.md Outdated Show resolved Hide resolved
charts/backingservices/README.md Outdated Show resolved Hide resolved
charts/backingservices/README.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@pega-kshev pega-kshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ready for review

README.md Outdated Show resolved Hide resolved
charts/backingservices/charts/srs/.helmignore Show resolved Hide resolved
charts/backingservices/charts/srs/templates/_helpers.tpl Outdated Show resolved Hide resolved
charts/backingservices/charts/srs/templates/_netpols.tpl Outdated Show resolved Hide resolved
charts/backingservices/charts/srs/values.yaml Outdated Show resolved Hide resolved
charts/backingservices/values.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@taz-pega-work taz-pega-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc changes look good!

Copy link
Contributor

@taz-pega-work taz-pega-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documentation looks correct

@MadhuriArugula
Copy link
Collaborator

@pega-kshev Travis run seems to be failing because of yamllint issue. Can you please look into it.

@pega-kshev
Copy link
Contributor Author

@MadhuriArugula thanks for pointing out, I have corrected it now.

@MadhuriArugula MadhuriArugula requested a review from a team December 22, 2020 11:33
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@kishorv10 kishorv10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pega-kshev a general comment, here backingservices is considered as a chart and srs as subchart, backingservices itself does not have any templates associated. Do we have a usecase to perceive backingservices as a chart. Instead can this be a just folder, srs is the main chart in it. (likewise any other backing services in future). As you are aware, helm subcharts technique does not handle dependencies well.
@dcasavant What are your views on this comment?

@pega-kshev
Copy link
Contributor Author

@pega-kshev a general comment, here backingservices is considered as a chart and srs as subchart, backingservices itself does not have any templates associated. Do we have a usecase to perceive backingservices as a chart. Instead can this be a just folder, srs is the main chart in it. (likewise any other backing services in future). As you are aware, helm subcharts technique does not handle dependencies well.
@dcasavant What are your views on this comment?

@pega-kshev a general comment, here backingservices is considered as a chart and srs as subchart, backingservices itself does not have any templates associated. Do we have a usecase to perceive backingservices as a chart. Instead can this be a just folder, srs is the main chart in it. (likewise any other backing services in future). As you are aware, helm subcharts technique does not handle dependencies well.
@dcasavant What are your views on this comment?

@kishorv10 I dont have immediate insight into what implications that could have, if we define backingservices as a chart but by defining backing services as a chart and services as subcharts, please correct me if I am wrong, one usecase I could see is to have a single values.yaml to configure multiple backing services, secondly, we have flexibility to implement supporting services under backingservices umbrella which would provide common functionality like a service mesh such as this proposal, which would help inject proxy containers using istio or linkerd into other backing service pods.

image

@NekhilKotha NekhilKotha self-requested a review January 4, 2021 05:16
@pega-kshev
Copy link
Contributor Author

@pega-kshev a general comment, here backingservices is considered as a chart and srs as subchart, backingservices itself does not have any templates associated. Do we have a usecase to perceive backingservices as a chart. Instead can this be a just folder, srs is the main chart in it. (likewise any other backing services in future). As you are aware, helm subcharts technique does not handle dependencies well.
@dcasavant What are your views on this comment?

@dcasavant

@Saurabh-16 Saurabh-16 merged commit 58c6af7 into pegasystems:master Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants