Demo with handcrafted schemas

[marcelldls]

Some difficulties:

• is that each level of definition (Service > Beamline > default ioc-instance) has different required values,
• the nesting of shared.ioc-instance and ioc-instance So I have three different charts which are similar

[marcelldls]

@gilesknap Heres a go at a handcrafted schema to hint developers at a service or services repo level. The following can be included to the header to demo:

# yaml-language-server: $schema=../../Schemas/ioc-instance.json#/$defs/repo
# yaml-language-server: $schema=../../Schemas/ioc-instance.json#/$defs/service

Thoughts?

[gilesknap]

taking a look now

[gilesknap]

This looks promising.

I've not looked in the schema because I find it too scary! :-)

But I've tried it out on bl45p.

I notice that in the service runAsUser and runAsGroup are string in the schema but should be int. These fields are correct for the repo.

Does that mean you had to write the repo and service parts totally separately? Is there a way to avoid that using references of some kind. I think you told me that schema can be written in YAML - if so then you could translate to Yaml and use anchors and aliases to repeat a section of the schema maybe.

I think this is really useful and needs two things in addition to the schema itself:

1. Publish the schema as part of a release so it can be referred to by URL (see how ibek publishes the support.ibek.yaml schema in its CI)
2. Add the above URL into the headers of the yaml in ec-services-template
3. reminder: I would also like to add to the ec-services-template, comments in the values.yaml for the example IOC about how to put sleep in the entrypoint and use the developer image instead of runtime.

[marcelldls]

This looks promising.

I've not looked in the schema because I find it too scary! :-)

But I've tried it out on bl45p.

I notice that in the service runAsUser and runAsGroup are string in the schema but should be int. These fields are correct for the repo.

Does that mean you had to write the repo and service parts totally separately? Is there a way to avoid that using references of some kind. I think you told me that schema can be written in YAML - if so then you could translate to Yaml and use anchors and aliases to repeat a section of the schema maybe.

I think this is really useful and needs two things in addition to the schema itself:

1. Publish the schema as part of a release so it can be referred to by URL (see how ibek publishes the support.ibek.yaml schema in its CI)

2. Add the above URL into the headers of the yaml in ec-services-template

3. reminder: I would also like to add to the ec-services-template, comments in the values.yaml for the example IOC about how to put sleep in the entrypoint and use the developer image instead of runtime.

Thanks GIles

Its not toooo bad - only the json format is annoying but fine for this I believe. I have a base schema which defines all the parameters. The service and repo schemas just point at this but define their own "required" values and create the shared/ioc-instance indentations we need. The only trick was getting it all into one file

1. Thank you - I will add the release code
2. Will make a seperate PR for this when its ready. Will have to rebase and fix some parameters to polish this off now that I am more confident in this
3. I have a PR submitted

[marcelldls]

Github action tested at https://github.com/marcelldls/ec-helm-charts

[gilesknap]

@marcelldls looking good. I think maybe you did not mean the last release to be 3.4.4 since previous releases were 3.5.x? But the URL for 3.4.4 works for testing against BL45P, this helped me to see that start was now starttCommand. We need to think about how to roll out this change without causing too much pain due to mandatory changes in the shared values.yaml - if you are game you could look into copier 'migrations'.

I'm still getting some schema errors in bl45p's shared values.yaml Please can you take a look at bl45p and see what is up with:

• globalEnv
• tolerations

I've already pushed the changes to the bl45p in branch values-schema

I really like this idea, it is nice and consistent that we are gaining schema validation for all of the yaml in the beamline repos. Well done.

[gilesknap]

I was going to say as a stretch goal for this that we could add regex to the resource limits and requests as I can never remember the format for those. I failed to find any reference to a regex for them but I assume the K8S plugin has it somewhere in the schemas it uses for manifest validation.

[marcelldls]

@marcelldls looking good. I think maybe you did not mean the last release to be 3.4.4 since previous releases were 3.5.x? But the URL for 3.4.4 works for testing against BL45P, this helped me to see that start was now starttCommand. We need to think about how to roll out this change without causing too much pain due to mandatory changes in the shared values.yaml - if you are game you could look into copier 'migrations'.

I'm still getting some schema errors in bl45p's shared values.yaml Please can you take a look at bl45p and see what is up with:

* globalEnv

* tolerations

I've already pushed the changes to the bl45p in branch values-schema

I really like this idea, it is nice and consistent that we are gaining schema validation for all of the yaml in the beamline repos. Well done.

Sorry the version was just unused tags after forking (no meaning behind them, just a number)

Will check on BL45P

[marcelldls]

I was going to say as a stretch goal for this that we could add regex to the resource limits and requests as I can never remember the format for those. I failed to find any reference to a regex for them but I assume the K8S plugin has it somewhere in the schemas it uses for manifest validation.

Are the defaults not enough of a hint?

A further stretch could be to do validation as a CI job, but maybe not right now

[marcelldls]

I will have a go with https://json-schema.org/understanding-json-schema/reference/regular_expressions

[gilesknap]

Are the defaults not enough of a hint?

Well threre are lots of strings in the format you can write and I'm unsure of the memory ones without referring to: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#meaning-of-memory and I can't even find a reference for the cpu limit definition (except for float or int followed by m - is that all the options?)

BTW I found this article while looking for the above which says don't put resource limits in! https://home.robusta.dev/blog/stop-using-cpu-limits

[gilesknap]

A further stretch could be to do validation as a CI job, but maybe not right now

I was going to say we already check the IOC renders - but that does not use the values.yaml only the ioc.yaml so this would be a nice check to have. Although beamline repo checking is a little arbitrary - you need to see the notification because you can still try to deploy a broken IOC - the CI does not take the tag away from the repo if it fails.

[gilesknap]

I'm not seeing the ec-services-template PR for some reason? - sorry forget that - I can see it but just had notification for some reason.

[marcelldls]

Add resource limits regex as per: https://github.com/kubernetes/kubernetes/blob/ae5543e4c8f99cb1555102a8ebc310aed3c82596/staging/src/k8s.io/apimachinery/pkg/api/resource/quantity.go#L149

[marcelldls]

@gilesknap Fixed against BL45P

[gilesknap]

Nice. I can't say I'm any the wiser as to how to specify cpu limits with the 'overly permissive' regex, but I can hardly complain given the provenance of that regex!!

Happy to merge this now.

[marcelldls]

Thanks @gilesknap - Lets merge, release and then I will make a PR for the ec-services-template accordingly

[gilesknap]

um - the merge button is doing nothing for me - do you want to give it a try?