Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #184 +/- ##
==========================================
- Coverage 92.28% 92.22% -0.07%
==========================================
Files 40 40
Lines 2009 2070 +61
==========================================
+ Hits 1854 1909 +55
- Misses 155 161 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@GDYendell , I think I prefer to generalize this to have structure like you did in odin and I did in ec-helm-charts:
Even though we only have one chart here. Waddyathink? |
gilesknap
left a comment
There was a problem hiding this comment.
Ready for review.
There was a problem hiding this comment.
I am a bit confused by the two helm charts. Is the intention that there is a base chart that doesn't assume what transport you are using and then an EPICS specific one? Because currently the base chart has EPICS things in it. If not, what are the two charts for?
This has also made me wonder how easy it is going to be to deploy multiple transports, because each is going to need their own ports exposed etc.
Fair point. The ioc helm chart is not really a helm chart, it is just there to generate schema that we use in services/XXX/values.yaml. That values.yaml has in it: fastcs-instance:
<fastcs schema compliant yaml goes here>I could make that schema quite easily with a template and jinja, or just with sed. But I decided to go with the same schema generating tool we used for the main fastcs chart values - and that requires a helm chart. I'm not that happy with it as it looks a little confusing and I used the convention that charts with suffix |
My take is that we just publish ports for all transports in the one helm chart - so far I have PVA and CA in there. Or we could pass config to the service to expose ports as needed, that just means we specify the list of transports in 2 places. BTW. At DLS we are still using hostNetwork so this service is not used except in demos and training. |
|
@GDYendell I agreed with your renaming of fastcs-instance to fastcs and made the change. Now I'm having doubts. fastcs-example publishes a container which is essentially a generic IOC for the simulation temp controller. You make instances of that using a services repository. So it still has the same generic container with instances adding config. With that in mind are we OK with the helm chart name 'fastcs'? |
We discussed this again and I have been convinced that the schemas should be called fastcs and fastcs-service. There is no longer a second chart, only a basic values.yaml for the new schema generator to work from. |
Now there is only one helm chart called fastcs (and two schemas called fastcs and fastcs-service) |
fastcs-exampleusing the published helm chart `0.9.2-alpha.3https://github.com/gilesknap/t01-services/tree/add-fastcs-example/services/bl01t-ea-fastcs-01
Resolves #177