Review helm files structure

[mathieu-benoit]

Purpose

Review the helm files structure to be compliant with the recommendation, especially where the name of subfolder of each chart should be equal to the name field in the Chart.yaml file - https://docs.helm.sh/developing_charts/#the-chart-yaml-file

Otherwise, we are not able to do helm package, without this we got this error:

Error: directory name (helm) and Chart.yaml name (mydrive-poi) must match

Does this introduce a breaking change?

[X] Yes
[ ] No

Pull Request Type

What kind of change does this Pull Request introduce?

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[X] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

• Get the code

git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install

• Test the code

What to Check

Verify that the following are valid:

• Provisioning a new Attendee environment from there: https://github.com/Azure-Samples/openhack-devops-proctor/tree/master/provision-team
• helm install from apis/poi/charts/mydrive-poi still works
• helm install from apis/trips/charts/mydrive-trips still works
• helm install from apis/user-java/charts/mydrive-user-java still works
• helm install from apis/userprofile/charts/mydrive-user still works

Other Information

[dtzar]

@mathieu-benoit I had a chat with @dcaro and we don't think this significant of a folder change/name/structure is worth moving forward and testing with the direction you're going. However, if you were to refactor this to a single helm chart for all of the APIs which also fixes the helm package problem we do believe it would be worthwhile.

As it stands now, people can use helm package if they simply rename the helm chart.yaml name to helm OR rename the folder. In our experience, most people don't use the helm package option although it is a valid approach.

[mathieu-benoit]

Hi @dcaro, this PR with its associated PR Azure-Samples/openhack-devops-proctor#254 are ready for your review. Again, the goal here is to have the proper folder/files structure to be able to do helm package without any errors or extra works to make it working. Thanks!

[dtzar]

@mathieu-benoit Were you going to consolidate to a single helm chart (per the previous comment) or no?

[mathieu-benoit]

Hi @dtzar, I was not sure why you are matching both requests/needs here and also mainly for these 3 reasons: 1/ more impact on the changes/refactoring/tests 2/ not following purely speaking the microservice approach where we should have one chart per service (?) 3/ It could be a future improvement to have a generic chart like your are thinking?
Again the first intent here is to fix an error (with helm package) and also following the recommended practice by Helm.

[dcaro]

@mathieu-benoit thanks for the work on this PR, I agree with you that we should keep one helm package per micro-service.
Did you fully test the deployment using the provisioning scripts?

[dtzar]

Best practice is to have a single helm chart if they all use the same structure (which all of them do). This keeps the APIs deployed in a consistent manner and allows easier updates/maintenance to the helm charts. All that said, yes technically there is no problem keeping separate helm charts. Our thinking was that to do all the effort you've done to rename/move and then test, it would make sense to just consolidate to a single chart for all the APIs and then test.

[dcaro]

@mathieu-benoit I just ran an automated deployment end to end and this worked. I will merge on Friday to not interfere with an ongoing event this week.

[dcaro]

LGTM